-----Original Message----- From: Verma, Vishal L [mailto:vishal.l.verma@intel.com] Sent: Wednesday, August 8, 2018 5:15 AM To: linux-nvdimm(a)lists.01.org; Qi, Fuli/斉 福利 <qi.fuli(a)jp.fujitsu.com&gt; Subject: Re: [ndctl PATCH] ndctl, list: emit alarm_enable_<field> and clarify misc list items On Wed, 2018-08-08 at 00:35 +0900, QI Fuli wrote: > This patch adds alarm_enable_<field> to list. Therefore, users can > know Hi Qi, Thanks, I was meaning to do this work but you beat me to it :) I just have a few nits, see below. But otherwise this looks good. > if the "ndclt inject-smart --<field>-alarm=on/off" works or not. s/ndclt/ndctl/ > > Users may confuse "alarm_enable" with "alarm_flag" and "media_temperature" > with "ctrl_temperature", this patch also used to clarify these items. I'm not sure it is a good idea to change these names since they've been released for quite some time, and users/scripts might be depending on them, which this change will break. Let's just add a new field for alarm_enabled_* for each smart field, but not change any of the existing ones for now.

> > Signed-off-by: QI Fuli <qi.fuli(a)jp.fujitsu.com&gt; > --- > ndctl/util/json-smart.c | 45 > ++++++++++++++++++++++++++++++++++------- > 1 file changed, 38 insertions(+), 7 deletions(-) > > diff --git a/ndctl/util/json-smart.c b/ndctl/util/json-smart.c index > 6a1f294..a0677a6 100644 > --- a/ndctl/util/json-smart.c > +++ b/ndctl/util/json-smart.c > @@ -39,34 +39,61 @@ static void smart_threshold_to_json(struct ndctl_dimm *dimm, > unsigned int temp; > double t; > > + jobj = json_object_new_boolean(true); > + if (jobj) > + json_object_object_add(jhealth, > + "alarm_enable_media_temperature", jobj); Lets reword all instances to alarm_enabled_* Since it is showing the status of the alarm. > temp = ndctl_cmd_smart_threshold_get_temperature(cmd); > t = ndctl_decode_smart_temperature(temp); > jobj = json_object_new_double(t); > if (jobj) > json_object_object_add(jhealth, > - "temperature_threshold", jobj); > + "media_temperature_threshold", jobj); > + } else { > + jobj = json_object_new_boolean(false); > + if (jobj) > + json_object_object_add(jhealth, > + "alarm_enable_media_temperature", jobj); > } > > if (alarm_control & ND_SMART_CTEMP_TRIP) { > unsigned int temp; > double t; > > + jobj = json_object_new_boolean(true); > + if (jobj) > + json_object_object_add(jhealth, > + "alarm_enable_ctrl_temperature", jobj); > temp = ndctl_cmd_smart_threshold_get_ctrl_temperature(cmd); > t = ndctl_decode_smart_temperature(temp); > jobj = json_object_new_double(t); > if (jobj) > json_object_object_add(jhealth, > - "controller_temperature_threshold", jobj); > + "ctrl_temperature_threshold", jobj); > + } else { > + jobj = json_object_new_boolean(false); > + if (jobj) > + json_object_object_add(jhealth, > + "alarm_enable_ctrl_temperature", jobj); > } > > if (alarm_control & ND_SMART_SPARE_TRIP) { > unsigned int spares; > > + jobj = json_object_new_boolean(true); > + if (jobj) > + json_object_object_add(jhealth, > + "alarm_enable_spares", jobj); > spares = ndctl_cmd_smart_threshold_get_spares(cmd); > jobj = json_object_new_int(spares); > if (jobj) > json_object_object_add(jhealth, > "spares_threshold", jobj); > + } else { > + jobj = json_object_new_boolean(false); > + if (jobj) > + json_object_object_add(jhealth, > + "alarm_enable_spares", jobj); > } > > out: > @@ -118,7 +145,8 @@ struct json_object > *util_dimm_health_to_json(struct ndctl_dimm *dimm) > > jobj = json_object_new_double(t); > if (jobj) > - json_object_object_add(jhealth, "temperature_celsius", jobj); > + json_object_object_add(jhealth, > + "media_temperature_celsius", jobj); > } > > if (flags & ND_SMART_CTEMP_VALID) { > @@ -128,7 +156,7 @@ struct json_object *util_dimm_health_to_json(struct ndctl_dimm *dimm) > jobj = json_object_new_double(t); > if (jobj) > json_object_object_add(jhealth, > - "controller_temperature_celsius", jobj); > + "ctrl_temperature_celsius", jobj); > } > > if (flags & ND_SMART_SPARES_VALID) { @@ -147,15 +175,18 @@ struct > json_object *util_dimm_health_to_json(struct ndctl_dimm *dimm) > > jobj = json_object_new_boolean(temp_flag); > if (jobj) > - json_object_object_add(jhealth, "alarm_temperature", jobj); > + json_object_object_add(jhealth, > + "alarm_flag_media_temperature", jobj); > > jobj = json_object_new_boolean(ctrl_temp_flag); > if (jobj) > - json_object_object_add(jhealth, "alarm_controller_temperature", jobj); > + json_object_object_add(jhealth, > + "alarm_flag_ctrl_temperature", jobj); > > jobj = json_object_new_boolean(spares_flag); > if (jobj) > - json_object_object_add(jhealth, "alarm_spares", jobj); > + json_object_object_add(jhealth, > + "alarm_flag_spares", jobj); > } > > smart_threshold_to_json(dimm, jhealth);