On Thu, Jan 31, 2019 at 10:05 AM Matt Riedemann <mriedemos@gmail.com> wrote:
>
> I'm going to top post and try to summarize where we are on this thread
> since I have it on today's nova meeting agenda under the "stuck reviews"
> section.
>
> * The email started as a proposal to change the proposed image property
> and flavor extra spec from a confusing boolean to an enum.
>
> * The question was raised why even have any other option than a unique
> serial number for all instances based on the instance UUID.
>
> * Stephen asked Daniel Berrange (danpb) about the history of the
> [libvirt]/sysinfo_serial configuration option and it sounds like it was
> mostly added as a way to determine guests running on the same host,
> which can already be determined using the hostId parameter in the REST
> API (hostId is the hashed instance.host + instance.project_id so it's
> not exactly the same since it's unique per host and project, not just
> host). However, the hostId is not exposed to the guest in the metadata
> API / config drive - so that could be a regression for applications that
> used this somehow to calculate affinity within the guest based on the
> serial (note that mnaser has a patch to expose hostId in the metadata
> API / config drive [1]).
>
> * danpb said the system.serial we set today should really be
> chassis.serial but that's only available in libvirt >= 4.1.0 and our
> current minimum required version of libvirt is 1.3.1 so setting
> chassis.serial would have to be conditional on the running version of
> libvirt (this is common in that driver).
>
> * Applications that depend on the serial number within the guest were
> not guaranteed it would be unique or not change because migrating the
> guest to another host would change the serial number anyway (that's the
> point of the blueprint - to keep the serial unchanged for each guest),
> so if we just changed to always using unique serial numbers everywhere
> it should probably be OK (and tolerated/expected by guest applications).
>
> * Clearly we would have a release note if we change this behavior but
> keep in mind that end users are not reading release notes, and none of
> this is documented today anyway outside of the [libvirt]/sysinfo_serial
> config option. So a release note would really only help an operator or
> support personal if they get a ticket due to the change in behavior
> (which we probably wouldn't hear about upstream for 2+ years given how
> slow openstack deployments upgrade).
>
> So where are we? If we want the minimal amount of behavior change as
> possible then we just add the new image property / flavor extra spec /
> config option choice, but that arguably adds technical debt and
> virt-driver specific behavior to the API (again, that's not uncommon
> though).
>
> If we want to simplify, we don't add the image property / flavor extra
> spec. But what do we do about the existing config option?
>
> Do we add the 'unique' choice, make it the default, and then deprecate
> the option to at least signal the change is coming in Train?
>
> Or do we just deprecate the option in Stein and completely ignore it,
> always setting the unique serial number as the instance.uuid (and set
> the host serial in chassis.serial if libvirt>=4.1.0)?
>
> In addition, do we expose hostId in the metadata API / config drive via
> [1] so there is a true alternative *from within the guest* to determine
> guest affinity on the same host? I'm personally OK with [1] if there is
> some user documentation around it (as noted in the review).
>
> If we are not going to add the new image property / extra spec, my
> personal choice would be to:
>
> - Add the 'unique' choice to the [libvirt]/sysinfo_serial config option
> and make it the default for new deployments.
> - Deprecate the sysinfo_serial config option in Stein and remove it in
> Train. This would give at least some window of time for transition
> and/or raising a stink if someone thinks we should leave the old
> per-host behavior.
> - Merge mnaser's patch to expose hostId in the metadata API and config
> drive so users still have a way within the guest to determine that
> affinity for servers in the same project on the same host.

I agree with this for a few reasons

Assuming that a system serial means that it is colocated with another
machine seems just taking advantage of a bug in the first place.  That
is not *documented* behaviour and serials should inherently be unique,
it also exposes information about the host which should not be necessary,
Matt has pointed me to an OSSN about this too:

https://wiki.openstack.org/wiki/OSSN/OSSN-0028

I think we should indeed provide a unique serials (only, ideally) to avoid
having the user shooting themselves in the foot by exposing information
they didn't know they were exposing.

The patch that I supplied was really meant to make that information available
in a controllable way, it also provides a much more secure way of exposing
that information because hostId is actually hashed with the tenant ID which
means that one VM from one tenant can't know that it's hosted on the same
VM as another one by usnig the hostId (and with all of the recent processor
issues, this is a big plus in security).


> What do others think?
>
> [1] https://review.openstack.org/#/c/577933/
>
> On 1/24/2019 9:09 AM, Matt Riedemann wrote:
> > The proposal from the spec for this feature was to add an image property
> > (hw_unique_serial), flavor extra spec (hw:unique_serial) and new
> > "unique" choice to the [libvirt]/sysinfo_serial config option. The image
> > property and extra spec would be booleans but really only True values
> > make sense and False would be more or less ignored. There were no plans
> > to enforce strict checking of a boolean value, e.g. if the image
> > property was True but the flavor extra spec was False, we would not
> > raise an exception for incompatible values, we would just use OR logic
> > and take the image property True value.
> >
> > The boolean usage proposed is a bit confusing, as can be seen from
> > comments in the spec [1] and the proposed code change [2].
> >
> > After thinking about this a bit, I'm now thinking maybe we should just
> > use a single-value enum for the image property and flavor extra spec:
> >
> > image: hw_guest_serial=unique
> > flavor: hw:guest_serial=unique
> >
> > If either are set, then we use a unique serial number for the guest. If
> > neither are set, then the serial number is based on the host
> > configuration as it is today.
> >
> > I think that's more clear usage, do others agree? Alex does. I can't
> > think of any cases where users would want hw_unique_serial=False, so
> > this removes that ability and confusion over whether or not to enforce
> > mismatching booleans.
> >
> > [1]
> > https://review.openstack.org/#/c/612531/2/specs/stein/approved/per-instance-libvirt-sysinfo-serial.rst@43
> >
> > [2]
> > https://review.openstack.org/#/c/619953/7/nova/virt/libvirt/driver.py@4894
>
>
> --
>
> Thanks,
>
> Matt
>


--
Mohammed Naser — vexxhost
-----------------------------------------------------
D. 514-316-8872
D. 800-910-1726 ext. 200
E. mnaser@vexxhost.com
W. http://vexxhost.com