Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 16 Nov 2022 16:20:08 +0100
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     linux-kernel@vger.kernel.org, Fei Li <fei1.li@intel.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH v4 1/1] virt: acrn: Mark the uuid field as unused
Message-ID: <Y3T/qJalJ9H224Mu@kroah.com>
References: <20221116092254.66234-1-andriy.shevchenko@linux.intel.com>
 <Y3TMmDXEbGyDQrLF@kroah.com>
 <Y3Tlu0BjgMk3xNDQ@smile.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y3Tlu0BjgMk3xNDQ@smile.fi.intel.com>
Precedence: bulk

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:
> > > After the commits for userspace (see Link tags below) the uuid field is
> > > not being used in the ACRN code. Update kernel to reflect these changes.
> > > I.e. we do the following:
> > > - adding a comment explaining that it's not used anymore
> > > - replacing the specific type by a raw buffer
> > > - updating the example code accordingly
> > > 
> > > The advertised field confused users and actually never been used. So
> > > the wrong part here is that kernel puts something which userspace never
> > > used and hence this may confuse a reader of this code.
> 
> > > - * @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?

> ...
> 
> > > +	__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.

> > 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?

thanks,

greg k-h