Hi Vishal, Thanks for your comments.
> -----Original Message----- > From: Verma, Vishal L [mailto:[email protected]] > Sent: Wednesday, August 8, 2018 5:15 AM > To: [email protected]; Qi, Fuli/斉 福利 <[email protected]> > 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 <[email protected]> > > --- > > 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); _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
