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

[Dan Williams]

On Mon, Apr 16, 2018 at 2:49 PM, Dan Williams  wrote:
> On Mon, Apr 16, 2018 at 2:47 PM, Verma, Vishal L
>  wrote:
>> On Mon, 2018-04-16 at 14:41 -0700, Dan Williams wrote:
>>> On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma 
>>> 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 
>>> > Signed-off-by: Vishal Verma 
>>> > ---
>>> >  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 
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

[Dan Williams]

On Mon, Apr 16, 2018 at 2:47 PM, Verma, Vishal L
 wrote:
> On Mon, 2018-04-16 at 14:41 -0700, Dan Williams wrote:
>> On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma 
>> 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 
>> > Signed-off-by: Vishal Verma 
>> > ---
>> >  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.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

[Verma, Vishal L]

On Mon, 2018-04-16 at 14:41 -0700, Dan Williams wrote:
> On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma 
> 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 
> > Signed-off-by: Vishal Verma 
> > ---
> >  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.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

[Dan Williams]

On Mon, Apr 16, 2018 at 2:27 PM, Vishal Verma  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 
> Signed-off-by: Vishal Verma 
> ---
>  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.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

[Vishal Verma]

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 
Signed-off-by: Vishal Verma 
---
 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);
+}
+
 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);
-- 
2.14.3

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm