| Message ID | 20170712154441.C680.E1E9C6FF@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 23d0dce3f969 |
| Headers | show |
On Tue, Jul 11, 2017 at 11:44 PM, Yasunori Goto <y-goto@jp.fujitsu.com> wrote: > Hi, > > I made the v3 of output hardware id . > If this patch is ok, please apply. > > -- > Change log > > v3 : - rebased on human readable option patch. > > > v2 : - use json_object_new_int() for each data. > - check MAX value for each data. > - phys_id becomes unsigned short from signed short. > > --- > The current "id" data of dimm (ndctl list -D) shows the information > of DIMM module vendor or serial number. However, I think it is not enough. > > If a NVDIMM becomes broken, users need to know its physical place > rather than vendor or serial number to replace the NVDIMM. > > There are 2 candidate of such information. > a) NFIT Device Handle (_ADR of the NVDIMM device) > This date includes node controller id and socket id. > b) NVDIMM Physical ID (Handle for the SMBIOS Memory Device (Type 17)). > The dmidecode can show this handle with more information like "Locator" > So, user can find the location of NVDIMM. > > At first, I think _ADR is enough information. > > However, when I talked with our firmware team about this requirement, > they said it may not be good data, because the node contoller ID is not > stable on our server due to "partitioning" feature. > (Our server can have plural partition on one box, and may have plural > node 0.) > > So, I make the ndctl shows not only NFIT Device Handle but also > NVDIMM Physical ID. Then, user can find the location with dmidecode. > > > Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com> Looks good to me, applied and pushed out to the 'pending' branch.
diff --git a/ndctl/list.c b/ndctl/list.c index d81d646..7f8db66 100644 --- a/ndctl/list.c +++ b/ndctl/list.c @@ -388,7 +388,7 @@ int cmd_list(int argc, const char **argv, void *ctx) json_object_object_add(jbus, "dimms", jdimms); } - jdimm = util_dimm_to_json(dimm); + jdimm = util_dimm_to_json(dimm, listopts_to_flags()); if (!jdimm) { fail("\n"); continue; diff --git a/util/json.c b/util/json.c index 0878979..80512bd 100644 --- a/util/json.c +++ b/util/json.c @@ -148,10 +148,13 @@ struct json_object *util_bus_to_json(struct ndctl_bus *bus) return NULL; } -struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm) +struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm, + unsigned long flags) { struct json_object *jdimm = json_object_new_object(); const char *id = ndctl_dimm_get_unique_id(dimm); + unsigned int handle = ndctl_dimm_get_handle(dimm); + unsigned short phys_id = ndctl_dimm_get_phys_id(dimm); struct json_object *jobj; if (!jdimm) @@ -169,6 +172,20 @@ struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm) json_object_object_add(jdimm, "id", jobj); } + if (handle < UINT_MAX) { + jobj = util_json_object_hex(handle, flags); + if (!jobj) + goto err; + json_object_object_add(jdimm, "handle", jobj); + } + + if (phys_id < USHRT_MAX) { + jobj = util_json_object_hex(phys_id, flags); + if (!jobj) + goto err; + json_object_object_add(jdimm, "phys_id", jobj); + } + if (!ndctl_dimm_is_enabled(dimm)) { jobj = json_object_new_string("disabled"); if (!jobj) diff --git a/util/json.h b/util/json.h index ec394b6..d885ead 100644 --- a/util/json.h +++ b/util/json.h @@ -26,7 +26,8 @@ enum util_json_flags { struct json_object; void util_display_json_array(FILE *f_out, struct json_object *jarray, int jflag); struct json_object *util_bus_to_json(struct ndctl_bus *bus); -struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm); +struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm, + unsigned long flags); struct json_object *util_mapping_to_json(struct ndctl_mapping *mapping, unsigned long flags); struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns,
Hi, I made the v3 of output hardware id . If this patch is ok, please apply. -- Change log v3 : - rebased on human readable option patch. v2 : - use json_object_new_int() for each data. - check MAX value for each data. - phys_id becomes unsigned short from signed short. --- The current "id" data of dimm (ndctl list -D) shows the information of DIMM module vendor or serial number. However, I think it is not enough. If a NVDIMM becomes broken, users need to know its physical place rather than vendor or serial number to replace the NVDIMM. There are 2 candidate of such information. a) NFIT Device Handle (_ADR of the NVDIMM device) This date includes node controller id and socket id. b) NVDIMM Physical ID (Handle for the SMBIOS Memory Device (Type 17)). The dmidecode can show this handle with more information like "Locator" So, user can find the location of NVDIMM. At first, I think _ADR is enough information. However, when I talked with our firmware team about this requirement, they said it may not be good data, because the node contoller ID is not stable on our server due to "partitioning" feature. (Our server can have plural partition on one box, and may have plural node 0.) So, I make the ndctl shows not only NFIT Device Handle but also NVDIMM Physical ID. Then, user can find the location with dmidecode. Signed-off-by: Yasunori Goto <y-goto@jp.fujitsu.com> --- --- Yasunori Goto