diff mbox series

[ndctl] ndctl, list: emit alarm_enable_<field> and clarify misc list items

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

Commit Message

QI Fuli Aug. 7, 2018, 3:35 p.m. UTC
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(-)

Comments

Verma, Vishal L Aug. 7, 2018, 8:15 p.m. UTC | #1
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);
QI Fuli Aug. 8, 2018, 12:57 a.m. UTC | #2
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 mbox series

Patch

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);