RE: [ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify misc list items
Hi Vishal,
Thanks for your comments.
> -Original Message-
> From: Verma, Vishal L [mailto:vishal.l.ve...@intel.com]
> Sent: Wednesday, August 8, 2018 5:15 AM
> To: linux-nvdimm@lists.01.org; Qi, Fuli/斉 福利
> Subject: Re: [ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify
> misc
> list items
>
>
> On Wed, 2018-08-08 at 00:35 +0900, QI Fuli wrote:
> > This patch adds alarm_enable_ 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 ---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_.
Thanks,
QI
> >
> > Signed-off-by: QI Fuli
> > ---
> > 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);
> > +
Re: [ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify misc list items
On Wed, 2018-08-08 at 00:35 +0900, QI Fuli wrote:
> This patch adds alarm_enable_ 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 ---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
> ---
> 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
[ndctl PATCH] ndctl, list: emit alarm_enable_ and clarify misc list items
This patch adds alarm_enable_ to list. Therefore, users can know
if the "ndclt inject-smart ---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
---
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);
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);
