diff mbox

[ndctl] ndctl, util: add a raw_uuid field to namespace listings

Message ID 20180416212703.14805-1-vishal.l.verma@intel.com
State New, archived
Headers show

Commit Message

Verma, Vishal L April 16, 2018, 9:27 p.m. UTC
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(+)

Comments

Dan Williams April 16, 2018, 9:41 p.m. UTC | #1
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.
Verma, Vishal L April 16, 2018, 9:47 p.m. UTC | #2
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.
Dan Williams April 16, 2018, 9:49 p.m. UTC | #3
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.
Dan Williams April 16, 2018, 9:50 p.m. UTC | #4
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 mbox

Patch

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