| Message ID | 20180416212703.14805-1-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma <vishal.l.verma@intel.com> wrote: > For boot support efibootmgr needs to be able to lookup up the raw > namespace uuid to match the device-path that EFI emits. By default > 'ndctl list' displays the uuid that is present in the address > abstraction info-block. Add a "raw_uuid" so that tooling can > correlate the default uuid with the base uuid for the namespace. > > Cc: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > util/json.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/util/json.c b/util/json.c > index 8d65525..efad8f7 100644 > --- a/util/json.c > +++ b/util/json.c > @@ -650,6 +650,17 @@ static struct json_object *util_dax_badblocks_to_json(struct ndctl_dax *dax, > bb_count, flags); > } > > +static struct json_object *util_raw_uuid(struct ndctl_namespace *ndns) > +{ > + struct json_object *jobj; > + uuid_t raw_uuid; > + char buf[40]; > + > + ndctl_namespace_get_uuid(ndns, raw_uuid); > + uuid_unparse(raw_uuid, buf); > + return json_object_new_string(buf); > +} > + I don't mind this new helper, but it seems out of place given all the other uuid-to-json-string conversions are open-coded in util_namespace_to_json(). Let's just open-code like all the rest, and maybe follow on with a global cleanup to use a helper in a later patch. However, I'm not sure it will be worth it / a net code reduction.
On Mon, 2018-04-16 at 14:41 -0700, Dan Williams wrote: > On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma <vishal.l.verma@intel.com> > wrote: > > For boot support efibootmgr needs to be able to lookup up the raw > > namespace uuid to match the device-path that EFI emits. By default > > 'ndctl list' displays the uuid that is present in the address > > abstraction info-block. Add a "raw_uuid" so that tooling can > > correlate the default uuid with the base uuid for the namespace. > > > > Cc: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > --- > > util/json.c | 23 +++++++++++++++++++++++ > > 1 file changed, 23 insertions(+) > > > > diff --git a/util/json.c b/util/json.c > > index 8d65525..efad8f7 100644 > > --- a/util/json.c > > +++ b/util/json.c > > @@ -650,6 +650,17 @@ static struct json_object > > *util_dax_badblocks_to_json(struct ndctl_dax *dax, > > bb_count, flags); > > } > > > > +static struct json_object *util_raw_uuid(struct ndctl_namespace *ndns) > > +{ > > + struct json_object *jobj; > > + uuid_t raw_uuid; > > + char buf[40]; > > + > > + ndctl_namespace_get_uuid(ndns, raw_uuid); > > + uuid_unparse(raw_uuid, buf); > > + return json_object_new_string(buf); > > +} > > + > > I don't mind this new helper, but it seems out of place given all the > other uuid-to-json-string conversions are open-coded in > util_namespace_to_json(). Let's just open-code like all the rest, and > maybe follow on with a global cleanup to use a helper in a later > patch. However, I'm not sure it will be worth it / a net code > reduction. Sure - the others were calling ndctl_{btt,pfn,dax}_get_uuid, where as with this, all the instances call ndctl namespace_get_uuid hence I factored it out as a function, but I'm happy to open code it in the three sites.
On Mon, Apr 16, 2018 at 2:47 PM, Verma, Vishal L <vishal.l.verma@intel.com> wrote: > On Mon, 2018-04-16 at 14:41 -0700, Dan Williams wrote: >> On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma <vishal.l.verma@intel.com> >> wrote: >> > For boot support efibootmgr needs to be able to lookup up the raw >> > namespace uuid to match the device-path that EFI emits. By default >> > 'ndctl list' displays the uuid that is present in the address >> > abstraction info-block. Add a "raw_uuid" so that tooling can >> > correlate the default uuid with the base uuid for the namespace. >> > >> > Cc: Dan Williams <dan.j.williams@intel.com> >> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> >> > --- >> > util/json.c | 23 +++++++++++++++++++++++ >> > 1 file changed, 23 insertions(+) >> > >> > diff --git a/util/json.c b/util/json.c >> > index 8d65525..efad8f7 100644 >> > --- a/util/json.c >> > +++ b/util/json.c >> > @@ -650,6 +650,17 @@ static struct json_object >> > *util_dax_badblocks_to_json(struct ndctl_dax *dax, >> > bb_count, flags); >> > } >> > >> > +static struct json_object *util_raw_uuid(struct ndctl_namespace *ndns) >> > +{ >> > + struct json_object *jobj; >> > + uuid_t raw_uuid; >> > + char buf[40]; >> > + >> > + ndctl_namespace_get_uuid(ndns, raw_uuid); >> > + uuid_unparse(raw_uuid, buf); >> > + return json_object_new_string(buf); >> > +} >> > + >> >> I don't mind this new helper, but it seems out of place given all the >> other uuid-to-json-string conversions are open-coded in >> util_namespace_to_json(). Let's just open-code like all the rest, and >> maybe follow on with a global cleanup to use a helper in a later >> patch. However, I'm not sure it will be worth it / a net code >> reduction. > > Sure - the others were calling ndctl_{btt,pfn,dax}_get_uuid, where as with > this, all the instances call ndctl namespace_get_uuid hence I factored it > out as a function, but I'm happy to open code it in the three sites. Ah, I'm slow, now I get it. Fine as is.
On Mon, Apr 16, 2018 at 2:49 PM, Dan Williams <dan.j.williams@intel.com> wrote: > On Mon, Apr 16, 2018 at 2:47 PM, Verma, Vishal L > <vishal.l.verma@intel.com> wrote: >> On Mon, 2018-04-16 at 14:41 -0700, Dan Williams wrote: >>> On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma <vishal.l.verma@intel.com> >>> wrote: >>> > For boot support efibootmgr needs to be able to lookup up the raw >>> > namespace uuid to match the device-path that EFI emits. By default >>> > 'ndctl list' displays the uuid that is present in the address >>> > abstraction info-block. Add a "raw_uuid" so that tooling can >>> > correlate the default uuid with the base uuid for the namespace. >>> > >>> > Cc: Dan Williams <dan.j.williams@intel.com> >>> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> >>> > --- >>> > util/json.c | 23 +++++++++++++++++++++++ >>> > 1 file changed, 23 insertions(+) >>> > >>> > diff --git a/util/json.c b/util/json.c >>> > index 8d65525..efad8f7 100644 >>> > --- a/util/json.c >>> > +++ b/util/json.c >>> > @@ -650,6 +650,17 @@ static struct json_object >>> > *util_dax_badblocks_to_json(struct ndctl_dax *dax, >>> > bb_count, flags); >>> > } >>> > >>> > +static struct json_object *util_raw_uuid(struct ndctl_namespace *ndns) >>> > +{ >>> > + struct json_object *jobj; >>> > + uuid_t raw_uuid; >>> > + char buf[40]; >>> > + >>> > + ndctl_namespace_get_uuid(ndns, raw_uuid); >>> > + uuid_unparse(raw_uuid, buf); >>> > + return json_object_new_string(buf); >>> > +} >>> > + >>> >>> I don't mind this new helper, but it seems out of place given all the >>> other uuid-to-json-string conversions are open-coded in >>> util_namespace_to_json(). Let's just open-code like all the rest, and >>> maybe follow on with a global cleanup to use a helper in a later >>> patch. However, I'm not sure it will be worth it / a net code >>> reduction. >> >> Sure - the others were calling ndctl_{btt,pfn,dax}_get_uuid, where as with >> this, all the instances call ndctl namespace_get_uuid hence I factored it >> out as a function, but I'm happy to open code it in the three sites. > > Ah, I'm slow, now I get it. Fine as is. aka: Reviewed-by: Dan Williams <dan.j.williams@intel.com>
diff --git a/util/json.c b/util/json.c index 8d65525..efad8f7 100644 --- a/util/json.c +++ b/util/json.c @@ -650,6 +650,17 @@ static struct json_object *util_dax_badblocks_to_json(struct ndctl_dax *dax, bb_count, flags); } +static struct json_object *util_raw_uuid(struct ndctl_namespace *ndns) +{ + struct json_object *jobj; + uuid_t raw_uuid; + char buf[40]; + + ndctl_namespace_get_uuid(ndns, raw_uuid); + uuid_unparse(raw_uuid, buf); + return json_object_new_string(buf); +} + struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns, unsigned long flags) { @@ -723,6 +734,10 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns, goto err; json_object_object_add(jndns, "uuid", jobj); + jobj = util_raw_uuid(ndns); + if (!jobj) + goto err; + json_object_object_add(jndns, "raw_uuid", jobj); bdev = ndctl_btt_get_block_device(btt); } else if (pfn) { ndctl_pfn_get_uuid(pfn, uuid); @@ -731,6 +746,10 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns, if (!jobj) goto err; json_object_object_add(jndns, "uuid", jobj); + jobj = util_raw_uuid(ndns); + if (!jobj) + goto err; + json_object_object_add(jndns, "raw_uuid", jobj); bdev = ndctl_pfn_get_block_device(pfn); } else if (dax) { struct daxctl_region *dax_region; @@ -742,6 +761,10 @@ struct json_object *util_namespace_to_json(struct ndctl_namespace *ndns, if (!jobj) goto err; json_object_object_add(jndns, "uuid", jobj); + jobj = util_raw_uuid(ndns); + if (!jobj) + goto err; + json_object_object_add(jndns, "raw_uuid", jobj); if ((flags & UTIL_JSON_DAX) && dax_region) { jobj = util_daxctl_region_to_json(dax_region, NULL, flags);
For boot support efibootmgr needs to be able to lookup up the raw namespace uuid to match the device-path that EFI emits. By default 'ndctl list' displays the uuid that is present in the address abstraction info-block. Add a "raw_uuid" so that tooling can correlate the default uuid with the base uuid for the namespace. Cc: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- util/json.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)