Re: [PATCH 1/1] hw/nvme: namespace parameter for EUI64

[Daniel P . Berrangé]

On Thu, Jun 10, 2021 at 07:31:32AM +0200, Klaus Jensen wrote:
> On Jun  9 22:15, Heinrich Schuchardt wrote:
> > Am 9. Juni 2021 21:57:26 MESZ schrieb Klaus Jensen <its@irrelevant.dk>:
> > > On Jun  9 20:13, Heinrich Schuchardt wrote:
> > > > Am 9. Juni 2021 16:39:20 MESZ schrieb "Daniel P. Berrangé"
> > > <berrange@redhat.com>:
> > > > > On Wed, Jun 09, 2021 at 02:33:08PM +0200, Klaus Jensen wrote:
> > > > > > On Jun  9 14:21, Heinrich Schuchardt wrote:
> > > > > > > On 6/9/21 2:14 PM, Klaus Jensen wrote:
> > > > > > > > On Jun  9 13:46, Heinrich Schuchardt wrote:
> > > > > > > > Would it make sense to provide a sensible default for EUI64
> > > such
> > > > > that it
> > > > > > > > is always there? That is, using the same IEEE OUI as we already
> > > > > report
> > > > > > > > in the IEEE field and add an extension identifier by grabbing 5
> > > > > bytes
> > > > > > > > from the UUID?
> > > > > > >
> > > > > > > According to the NVMe 1.4 specification it is allowable to have a
> > > > > NVMe
> > > > > > > device that does not support EUI64. As the EUI64 is used to
> > > define
> > > > > boot
> > > > > > > options in UEFI using a non-zero default may break existing
> > > > > installations.
> > > > > > >
> > > > > > 
> > > > > > Right. We dont wanna do that.
> > > > > 
> > > > > Any change in defaults can (must) be tied to the machine type
> > > versions,
> > > > > so that existing installs are unchanged, but new installs using
> > > latest
> > > > > machine type get the new default.
> > > > 
> > > > The road of least surprise is preferable. All machine should behave
> > > the
> > > > same if there is no real necessity to deviate.
> > > > 
> > > 
> > > I'd rather not introduce a new user-facing knob for this when a very
> > > sensible default can be derived from the uuid and the QEMU IEEE OUI. We
> > > 
> > > already have the uuid parameter that allows users to ensure that the
> > > namespace holds a persistent unique identifier. Adding another
> > > parameter
> > > with the same purpose seems unnecessary. And since we are adding EUI64,
> > > 
> > > we should probably also add NGUID while we are at it.
> > > 
> > > So, instead of adding parameters for EUI64 and NGUID that the user must
> > > 
> > > specify to get this more real-world behavior, I'd prefer to rely on a
> > > couple of boolean compat properties, e.g. 'support-eui64' and
> > > 'support-nguid' that defaults to 'on', but is set to 'off' for pre-6.1
> > > machines.
> > > 
> > > I think this is a nice compromise between making sure that having
> > > sensible EUI64 and NGUID values set is the new default while making
> > > sure
> > > that we do not break existing setup.
> > > 
> > > Would this be an acceptable compromise to you Heinrich?
> > 
> > EUI64 defined on some machine and not on others is totally obscure for
> > users.
> 
> I don't think that is obscure. This is exactly why machine types are
> versioned. It is documented as a feature to ensure working live migration
> between versions, but it is definitely also useful for just making sure that
> no behavior changes between qemu upgrades.
> 
> We have used this feature in the past to change the PCI Vendor/Device ID of
> the device.
> 
> > 
> > The virt machine is typically used and is pre-6.1. As pointed out above
> > you should not change the EUI64 when QEMU is upgraded and invoked with
> > the same parameter set.
> > 
> 
> From an NVMe perspective we are not changing it. We are going from "not
> supported" to "supported". But I acknowledge that there are systems that
> rely on EUI64 being zero - I just don't see why that should refrain us from
> adding EUI64 and NGUID by default in future versions when we can ensure
> compatibility with the versioned machine type (i.e. virt-6.0).

Yes, the whole point of versioned machine types is that they let us fix
bugs and add features to device implementations, while maintaining back
compat. So going from no-EUI64 to  EUI64 by default in a new machine
type version is exactly the kind of thing that is intended to happen.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|