Re: [PATCH 10/23] libnvdimm/labels: Add uuid helpers

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, Aug 11, 2021 at 11:05:55AM +0300, Andy Shevchenko wrote:
> On Mon, Aug 09, 2021 at 03:28:40PM -0700, Dan Williams wrote:
> > In preparation for CXL labels that move the uuid to a different offset
> > in the label, add nsl_{ref,get,validate}_uuid(). These helpers use the
> > proper uuid_t type. That type definition predated the libnvdimm
> > subsystem, so now is as a good a time as any to convert all the uuid
> > handling in the subsystem to uuid_t to match the helpers.
> > 
> > As for the whitespace changes, all new code is clang-format compliant.
> 
> Thanks, looks good to me!
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Sorry, I'm in doubt this Rb stays. See below.

...

> >  struct btt_sb {
> >  	u8 signature[BTT_SIG_LEN];
> > -	u8 uuid[16];
> > -	u8 parent_uuid[16];
> > +	uuid_t uuid;
> > +	uuid_t parent_uuid;

uuid_t type is internal to the kernel. This seems to be an ABI?

> >  	__le32 flags;
> >  	__le16 version_major;
> >  	__le16 version_minor;

...

> >  struct nd_namespace_label {
> > -	u8 uuid[NSLABEL_UUID_LEN];
> > +	uuid_t uuid;

So seems this.

> >  	u8 name[NSLABEL_NAME_LEN];
> >  	__le32 flags;
> >  	__le16 nlabel;

...

I'm not familiar with FS stuff, but looks to me like unwanted changes.
In such cases you have to use export/import APIs. otherwise you make the type
carved in stone without even knowing that it's part of an ABI or some hardware
/ firmware interfaces.

-- 
With Best Regards,
Andy Shevchenko