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

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

On Thu, Aug 12, 2021 at 03:34:59PM -0700, Dan Williams wrote:
> On Wed, Aug 11, 2021 at 12:18 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Wed, Aug 11, 2021 at 10:11:56AM -0700, Dan Williams wrote:
> > > On Wed, Aug 11, 2021 at 9:59 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> 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 <andriy.shevchenko@linux.intel.com>
> > > >
> > > > Sorry, I'm in doubt this Rb stays. See below.
> 
> Andy, does this incremental diff restore your reviewed-by? The awkward
> piece of this for me is that it introduces a handful of unnecessary
> memory copies. See some of the new nsl_get_uuid() additions and the
> extra copy in nsl_uuid_equal()

It does, thanks! As for the deeper discussion I think you need to talk to
Christoph. It was his idea to move uuid_t from UAPI to internal kernel type.
And I think it made and still makes sense to be that way.

But if we have already users of uuid_t like you are doing here (without this
patch) then it will be fine I guess. Not my area to advise or decide.

-- 
With Best Regards,
Andy Shevchenko