On Wed, Aug 11, 2021 at 9:59 AM Andy Shevchenko
<[email protected]> wrote:
>
> 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 <[email protected]>
>
> 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?
No, it's not a user ABI, this is an on-disk metadata structure. uuid_t
is approprirate.
>
> > > __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.
Can you clarify the concern? Carving the intent that these 16-bytes
are meant to be treated as UUID in stone is deliberate.
