| Message ID | 20180807153514.31748-1-qi.fuli@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [ndctl] ndctl, list: emit alarm_enable_<field> and clarify misc list items | expand |
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@jp.fujitsu.com> > --- > 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);
Hi Vishal, Thanks for your comments. > -----Original Message----- > From: Verma, Vishal L [mailto:vishal.l.verma@intel.com] > Sent: Wednesday, August 8, 2018 5:15 AM > To: linux-nvdimm@lists.01.org; Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com> > 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. > Ok, I will make a v2 patch which only includes adding alarm_enable_<field>. Thanks, QI > > > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > --- > > 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);
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); 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);
This patch adds alarm_enable_<field> to list. Therefore, users can know if the "ndclt inject-smart --<field>-alarm=on/off" works or not. Users may confuse "alarm_enable" with "alarm_flag" and "media_temperature" with "ctrl_temperature", this patch also used to clarify these items. Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> --- ndctl/util/json-smart.c | 45 ++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-)