On 01/25/2019 10:03 AM, Stephen Finucane wrote:
On Thu, 2019-01-24 at 09:09 -0600, 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.
Makes sense - we do that for 'hw_cpu_policy', for example (we have 'dedicated' and 'shared', but they're essentially booleans). However, the reason we added 'hw:cpu_policy=shared' (the flavor extra spec) was to provide a way for operators to prevent users requesting pinned CPUs via images metadata if they didn't want them in their cloud. At the risk of rehasing what's in the spec, if there aren't cases where users would want 'hw_unique_serial=False' then what is the upside of ever supporting non-unique serials going forward? Could we not just default to unique serials, avoiding these knobs?
Right, which is precisely what I commented on the spec. :) I see no valid reason to not just always set the sysinfo_serial to the instance UUID and be done with it. -jay