Re: [PATCH v4 1/1] virt: acrn: Mark the uuid field as unused

[Fei Li]

On 2022-11-16 at 18:04:37 +0200, Andy Shevchenko wrote:
> On Wed, Nov 16, 2022 at 04:20:08PM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Nov 16, 2022 at 03:29:31PM +0200, Andy Shevchenko wrote:
> > > On Wed, Nov 16, 2022 at 12:42:16PM +0100, Greg Kroah-Hartman wrote:
> > > > On Wed, Nov 16, 2022 at 11:22:54AM +0200, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > - * @uuid:		UUID of the VM. Pass to hypervisor directly.
> > > > > + * @uuid:		Reserved (used to be UUID of the VM)
> > > > 
> > > > If it's reserved, then don't you need to check for 0?
> > > 
> > > Reserved in a way that it may content something we just don't care about.
> > 
> > "reserved" in the kernel ioctls mean "must be 0 and we will test for it,
> > otherwise this is an empty/useless field that can never be touched again
> > in the future.
> > 
> > Please spell it out in detail as to if you can ever use this later on,
> > and what the kernel will do (if anything) if it is set.
> > 
> > And if "the kernel ignores it" then that means these bytes are now
> > "empty space never to be used again", right?
> 
> Right, I will fix this in v5.
Hi Andy

ACRN does not use uuid recently. But before that, the old ACRN still uses it.
So could we just change the uuid data structure definition here ?

Thanks.
> 
> ...
> 
> > > > > +	__u8	uuid[16];
> > > > 
> > > > You just changed the type here, so what is that going to break in
> > > > userspace that depended on this being of a structure type and now it's
> > > > an array?
> > > 
> > > It's the same. The previous was hidden behind additional type level.
> > 
> > Same size, yes.  Same C structure definition, no.
> 
> It doesn't matter, see below.
> 
> > > > And no other kernel changes needed?  Shouldn't you warn if this field is
> > > > set?
> > > 
> > > No, as pointed out in the commit message kernel never ever used this.
> > 
> > That does not mean that userspace tools never did, right?  You are
> > changing the structure definition, what tool just broke?
> 
> The only tool has been amended like a year ago, so the answer is none.
> The commit message has links to the commits in question that made that
> amendment.
> 
> Maybe I should remove Fixes tags? In such case we will very much know
> that no old tools will be run on the new kernel.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>