Re: [dm-devel] [PATCH v2] dm-ioctl: return UUID in DM_LIST_DEVICES_CMD result

[Mike Snitzer <snitzer@redhat.com>]

On Thu, Mar 11 2021 at  2:43pm -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Thu, 11 Mar 2021, Mike Snitzer wrote:
> 
> > > Index: linux-2.6/include/uapi/linux/dm-ioctl.h
> > > ===================================================================
> > > --- linux-2.6.orig/include/uapi/linux/dm-ioctl.h	2021-03-09 12:20:23.000000000 +0100
> > > +++ linux-2.6/include/uapi/linux/dm-ioctl.h	2021-03-11 18:42:14.000000000 +0100
> > > @@ -193,8 +193,15 @@ struct dm_name_list {
> > >  	__u32 next;		/* offset to the next record from
> > >  				   the _start_ of this */
> > >  	char name[0];
> > > +
> > > +	/* uint32_t event_nr; */
> > > +	/* uint32_t flags; */
> > > +	/* char uuid[0]; */
> > >  };
> > 
> > If extra padding is being leveraged here (from the __u32 next), why not
> > at least explicitly add the members and then pad out the balance of that
> > __u32?  I'm not liking the usage of phantom struct members.. e.g.
> > the games played with accessing them.
> > 
> > Mike
> 
> What exactly do you mean?
> 
> Do you want to create another structure that holds event_nr, flags and 
> uuid? Or something else?

Just not liking the comments you added in lieu of explicit struct
members.

Can't you remove __u32 next; and replace with named members of
appropriate size? Adding explicit padding to end to get you to 32bit
offset?  I'd need to look closer at the way the code is written, but I
just feel like this patch makes the code even more fiddley.

But I can let it go if you don't see a way forward to make it better..

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel