Re: [PATCH 2/3] xfs: use uuid_be to implement the uuid_t type

Re: [PATCH 2/3] xfs: use uuid_be to implement the uuid_t type

[Amir Goldstein]

[ added CC linux-fsdevel ]

On Fri, May 5, 2017 at 1:10 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Fri, May 05, 2017 at 01:06:28PM +0300, Andy Shevchenko wrote:
>> On Fri, 2017-05-05 at 11:56 +0200, Christoph Hellwig wrote:
>> > On Fri, May 05, 2017 at 12:42:23PM +0300, Andy Shevchenko wrote:
>> > > Christoph, I can wait a bit and re-do my patch if we settle down
>> > > data
>> > > types and function name space.
>> >
>> > We should probably just work on a common branch.
>>
>> Works for me. Are you volunteering to provide a such?
>
> Sure, I can set one up in one of my infradead trees.

Christoph, Andy,

Our discussions now span so many threads and different CC lists
(ACPI uuid_le, hoist xfs uuid_t, tmpfs gen s_uuid, ubifs copy s_uuid, ...)
so I picked this relatively young and less overloaded thread.
Since converting filesystems and uuid_be seems to be first order of
business, I only added linux-fsdevel to CC. If you think that other lists
should be included at this point in the discussion, feel free to add them.
Let's try to stick to this thread going forward.

To avoid stepping over each other toes, I propose the following work sequence:

This patch set by Christoph:
- re-define xfs's  uuid_t as uuid_be (this patch)
- more xfs uuid cleanups

V3 of 'hoist xfs uuid helpers' by me:
- hoist uuid_t typedef to uuid.h
- hoist helpers uuid_equal()/uuid_copy() to uuid.h
- create helper uuid_is_null()
- convert xfs to use the new uuid_is_null()

Kill off uuid_be (I can do it):
- Create helper variants uuid_cmp()/uuid_gen()/uuid_to_bin()
- Convery uuid_be users to uuid_t and uuid_ helpers
- Kill off uuid_be and uuid_be_ helpers

Promote uuid_t to VFS and filesystems (I can do it):
- Convert sb->s_uuid to uuid_t
- Convert some sample fs (tmpfs, ext2, ext4, ..) to uuid_t
- Use uuid_copy() and uuid_gen() where due

Kill off generate_random_uuid (Christoph?):
- Not sure what you're intentions are wrt to all users that use
  this helper to initialize a char[16] var?

Maybe repeat the uuid_be->uuid_t process with uuid_le->guid_t (Andy?):
- I don't think any of the above interferes with your work to convert
ACPI to uuid_le?
- You can actually start this work in parallel to uuid_be->uuid_t, because
  it will create trivial merge conflicts that we can easily sort out
on the common
  development branch

Thoughts?

Amir.

Re: [PATCH 2/3] xfs: use uuid_be to implement the uuid_t type

[Christoph Hellwig]

On Wed, May 10, 2017 at 11:39:27AM +0300, Amir Goldstein wrote:
> To avoid stepping over each other toes, I propose the following work sequence:

I agree on the list, but I've started a tree that has a slightly different
sequence:

(1) renamed uuid_be to uuid_t and uuid_le to guid_t, but keep the old
    name for compat for now.
    Including these items from your list:
> - Create helper variants uuid_cmp()/uuid_gen()/uuid_to_bin()

(2): kill uuid_v1 by adding a union to uuid_t that has the v1 meaning
     (and maybe more in the future)

(3) > This patch set by Christoph:
> - re-define xfs's  uuid_t as uuid_be (this patch)
> - more xfs uuid cleanups


(4): > V3 of 'hoist xfs uuid helpers' by me:
> - hoist uuid_t typedef to uuid.h
> - hoist helpers uuid_equal()/uuid_copy() to uuid.h
> - create helper uuid_is_null()
> - convert xfs to use the new uuid_is_null()

(but now moved over the to plain uuid_t type)

(5) > Promote uuid_t to VFS and filesystems (I can do it):
> - Convert sb->s_uuid to uuid_t
> - Convert some sample fs (tmpfs, ext2, ext4, ..) to uuid_t
> - Use uuid_copy() and uuid_gen() where due

And the rest for latter

> Kill off uuid_be (I can do it):
> - Convery uuid_be users to uuid_t and uuid_ helpers
> - Kill off uuid_be and uuid_be_ helpers
> 
> Kill off generate_random_uuid (Christoph?):
> - Not sure what you're intentions are wrt to all users that use
>   this helper to initialize a char[16] var?

They really should be converted to either uuid_t or guid_t, or
just use prandom_bytes if they really aren't either a UUID or GUID

> Maybe repeat the uuid_be->uuid_t process with uuid_le->guid_t (Andy?):
> - I don't think any of the above interferes with your work to convert
> ACPI to uuid_le?
> - You can actually start this work in parallel to uuid_be->uuid_t, because
>   it will create trivial merge conflicts that we can easily sort out
> on the common
>   development branch

With my work I'm right now at item (5) of my list, I'll send it out
later this afternoon.

Re: [PATCH 2/3] xfs: use uuid_be to implement the uuid_t type

[Andy Shevchenko]

On Wed, 2017-05-10 at 14:01 +0200, Christoph Hellwig wrote:
> On Wed, May 10, 2017 at 11:39:27AM +0300, Amir Goldstein wrote:
> > To avoid stepping over each other toes, I propose the following work
> > sequence:
> 
> I agree on the list, but I've started a tree that has a slightly
> different
> sequence:
> 
> (1) renamed uuid_be to uuid_t and uuid_le to guid_t, but keep the old
>     name for compat for now.
>     Including these items from your list:
> > - Create helper variants uuid_cmp()/uuid_gen()/uuid_to_bin()
> 
> (2): kill uuid_v1 by adding a union to uuid_t that has the v1 meaning
>      (and maybe more in the future)
> 
> (3) > This patch set by Christoph:
> > - re-define xfs's  uuid_t as uuid_be (this patch)
> > - more xfs uuid cleanups
> 
> 
> (4): > V3 of 'hoist xfs uuid helpers' by me:
> > - hoist uuid_t typedef to uuid.h
> > - hoist helpers uuid_equal()/uuid_copy() to uuid.h
> > - create helper uuid_is_null()
> > - convert xfs to use the new uuid_is_null()
> 
> (but now moved over the to plain uuid_t type)
> 
> (5) > Promote uuid_t to VFS and filesystems (I can do it):
> > - Convert sb->s_uuid to uuid_t
> > - Convert some sample fs (tmpfs, ext2, ext4, ..) to uuid_t
> > - Use uuid_copy() and uuid_gen() where due
> 
> And the rest for latter
> 
> > Kill off uuid_be (I can do it):
> > - Convery uuid_be users to uuid_t and uuid_ helpers
> > - Kill off uuid_be and uuid_be_ helpers
> > 
> > Kill off generate_random_uuid (Christoph?):
> > - Not sure what you're intentions are wrt to all users that use
> >   this helper to initialize a char[16] var?
> 
> They really should be converted to either uuid_t or guid_t, or
> just use prandom_bytes if they really aren't either a UUID or GUID
> 
> > Maybe repeat the uuid_be->uuid_t process with uuid_le->guid_t
> > (Andy?):
> > - I don't think any of the above interferes with your work to
> > convert
> > ACPI to uuid_le?
> > - You can actually start this work in parallel to uuid_be->uuid_t,
> > because
> >   it will create trivial merge conflicts that we can easily sort out
> > on the common
> >   development branch
> 
> With my work I'm right now at item (5) of my list, I'll send it out
> later this afternoon.


I'm fine with above and it looks like I could rebase my current work
whenever (1) has been done.

Looking forward to repository with changes I can rebase on top of.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 2/3] xfs: use uuid_be to implement the uuid_t type

[Christoph Hellwig]

On Wed, May 10, 2017 at 03:54:55PM +0300, Andy Shevchenko wrote:
> I'm fine with above and it looks like I could rebase my current work
> whenever (1) has been done.
> 
> Looking forward to repository with changes I can rebase on top of.

Here is what I currently have:

http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/uuid-types

It hasn't survived the buildbot yet, so take it with a grain of salt.