<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">- Add the 'unique' choice to the [libvirt]/sysinfo_serial config option <br>and make it the default for new deployments.<br>- Deprecate the sysinfo_serial config option in Stein and remove it in <br>Train. This would give at least some window of time for transition <br>and/or raising a stink if someone thinks we should leave the old <br>per-host behavior.<br>- Merge mnaser's patch to expose hostId in the metadata API and config <br>drive so users still have a way within the guest to determine that <br>affinity for servers in the same project on the same host.  </blockquote><div><br></div><div>So can I assume this is the decision now? I will update the patch according to this and since I have 3 more days before Chinese new year, I will update again if something new happens </div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 31, 2019 at 11:38 PM Mohammed Naser <<a href="mailto:mnaser@vexxhost.com">mnaser@vexxhost.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, Jan 31, 2019 at 10:05 AM Matt Riedemann <<a href="mailto:mriedemos@gmail.com" target="_blank">mriedemos@gmail.com</a>> wrote:<br>
><br>
> I'm going to top post and try to summarize where we are on this thread<br>
> since I have it on today's nova meeting agenda under the "stuck reviews"<br>
> section.<br>
><br>
> * The email started as a proposal to change the proposed image property<br>
> and flavor extra spec from a confusing boolean to an enum.<br>
><br>
> * The question was raised why even have any other option than a unique<br>
> serial number for all instances based on the instance UUID.<br>
><br>
> * Stephen asked Daniel Berrange (danpb) about the history of the<br>
> [libvirt]/sysinfo_serial configuration option and it sounds like it was<br>
> mostly added as a way to determine guests running on the same host,<br>
> which can already be determined using the hostId parameter in the REST<br>
> API (hostId is the hashed instance.host + instance.project_id so it's<br>
> not exactly the same since it's unique per host and project, not just<br>
> host). However, the hostId is not exposed to the guest in the metadata<br>
> API / config drive - so that could be a regression for applications that<br>
> used this somehow to calculate affinity within the guest based on the<br>
> serial (note that mnaser has a patch to expose hostId in the metadata<br>
> API / config drive [1]).<br>
><br>
> * danpb said the system.serial we set today should really be<br>
> chassis.serial but that's only available in libvirt >= 4.1.0 and our<br>
> current minimum required version of libvirt is 1.3.1 so setting<br>
> chassis.serial would have to be conditional on the running version of<br>
> libvirt (this is common in that driver).<br>
><br>
> * Applications that depend on the serial number within the guest were<br>
> not guaranteed it would be unique or not change because migrating the<br>
> guest to another host would change the serial number anyway (that's the<br>
> point of the blueprint - to keep the serial unchanged for each guest),<br>
> so if we just changed to always using unique serial numbers everywhere<br>
> it should probably be OK (and tolerated/expected by guest applications).<br>
><br>
> * Clearly we would have a release note if we change this behavior but<br>
> keep in mind that end users are not reading release notes, and none of<br>
> this is documented today anyway outside of the [libvirt]/sysinfo_serial<br>
> config option. So a release note would really only help an operator or<br>
> support personal if they get a ticket due to the change in behavior<br>
> (which we probably wouldn't hear about upstream for 2+ years given how<br>
> slow openstack deployments upgrade).<br>
><br>
> So where are we? If we want the minimal amount of behavior change as<br>
> possible then we just add the new image property / flavor extra spec /<br>
> config option choice, but that arguably adds technical debt and<br>
> virt-driver specific behavior to the API (again, that's not uncommon<br>
> though).<br>
><br>
> If we want to simplify, we don't add the image property / flavor extra<br>
> spec. But what do we do about the existing config option?<br>
><br>
> Do we add the 'unique' choice, make it the default, and then deprecate<br>
> the option to at least signal the change is coming in Train?<br>
><br>
> Or do we just deprecate the option in Stein and completely ignore it,<br>
> always setting the unique serial number as the instance.uuid (and set<br>
> the host serial in chassis.serial if libvirt>=4.1.0)?<br>
><br>
> In addition, do we expose hostId in the metadata API / config drive via<br>
> [1] so there is a true alternative *from within the guest* to determine<br>
> guest affinity on the same host? I'm personally OK with [1] if there is<br>
> some user documentation around it (as noted in the review).<br>
><br>
> If we are not going to add the new image property / extra spec, my<br>
> personal choice would be to:<br>
><br>
> - Add the 'unique' choice to the [libvirt]/sysinfo_serial config option<br>
> and make it the default for new deployments.<br>
> - Deprecate the sysinfo_serial config option in Stein and remove it in<br>
> Train. This would give at least some window of time for transition<br>
> and/or raising a stink if someone thinks we should leave the old<br>
> per-host behavior.<br>
> - Merge mnaser's patch to expose hostId in the metadata API and config<br>
> drive so users still have a way within the guest to determine that<br>
> affinity for servers in the same project on the same host.<br>
<br>
I agree with this for a few reasons<br>
<br>
Assuming that a system serial means that it is colocated with another<br>
machine seems just taking advantage of a bug in the first place.  That<br>
is not *documented* behaviour and serials should inherently be unique,<br>
it also exposes information about the host which should not be necessary,<br>
Matt has pointed me to an OSSN about this too:<br>
<br>
<a href="https://wiki.openstack.org/wiki/OSSN/OSSN-0028" rel="noreferrer" target="_blank">https://wiki.openstack.org/wiki/OSSN/OSSN-0028</a><br>
<br>
I think we should indeed provide a unique serials (only, ideally) to avoid<br>
having the user shooting themselves in the foot by exposing information<br>
they didn't know they were exposing.<br>
<br>
The patch that I supplied was really meant to make that information available<br>
in a controllable way, it also provides a much more secure way of exposing<br>
that information because hostId is actually hashed with the tenant ID which<br>
means that one VM from one tenant can't know that it's hosted on the same<br>
VM as another one by usnig the hostId (and with all of the recent processor<br>
issues, this is a big plus in security).<br>
<br>
<br>
> What do others think?<br>
><br>
> [1] <a href="https://review.openstack.org/#/c/577933/" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/577933/</a><br>
><br>
> On 1/24/2019 9:09 AM, Matt Riedemann wrote:<br>
> > The proposal from the spec for this feature was to add an image property<br>
> > (hw_unique_serial), flavor extra spec (hw:unique_serial) and new<br>
> > "unique" choice to the [libvirt]/sysinfo_serial config option. The image<br>
> > property and extra spec would be booleans but really only True values<br>
> > make sense and False would be more or less ignored. There were no plans<br>
> > to enforce strict checking of a boolean value, e.g. if the image<br>
> > property was True but the flavor extra spec was False, we would not<br>
> > raise an exception for incompatible values, we would just use OR logic<br>
> > and take the image property True value.<br>
> ><br>
> > The boolean usage proposed is a bit confusing, as can be seen from<br>
> > comments in the spec [1] and the proposed code change [2].<br>
> ><br>
> > After thinking about this a bit, I'm now thinking maybe we should just<br>
> > use a single-value enum for the image property and flavor extra spec:<br>
> ><br>
> > image: hw_guest_serial=unique<br>
> > flavor: hw:guest_serial=unique<br>
> ><br>
> > If either are set, then we use a unique serial number for the guest. If<br>
> > neither are set, then the serial number is based on the host<br>
> > configuration as it is today.<br>
> ><br>
> > I think that's more clear usage, do others agree? Alex does. I can't<br>
> > think of any cases where users would want hw_unique_serial=False, so<br>
> > this removes that ability and confusion over whether or not to enforce<br>
> > mismatching booleans.<br>
> ><br>
> > [1]<br>
> > <a href="https://review.openstack.org/#/c/612531/2/specs/stein/approved/per-instance-libvirt-sysinfo-serial.rst@43" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/612531/2/specs/stein/approved/per-instance-libvirt-sysinfo-serial.rst@43</a><br>
> ><br>
> > [2]<br>
> > <a href="https://review.openstack.org/#/c/619953/7/nova/virt/libvirt/driver.py@4894" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/619953/7/nova/virt/libvirt/driver.py@4894</a><br>
><br>
><br>
> --<br>
><br>
> Thanks,<br>
><br>
> Matt<br>
><br>
<br>
<br>
--<br>
Mohammed Naser — vexxhost<br>
-----------------------------------------------------<br>
D. 514-316-8872<br>
D. 800-910-1726 ext. 200<br>
E. <a href="mailto:mnaser@vexxhost.com" target="_blank">mnaser@vexxhost.com</a><br>
W. <a href="http://vexxhost.com" rel="noreferrer" target="_blank">http://vexxhost.com</a><br>
<br>
</blockquote></div>