[libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices

[Daniel P. Berrange]

On Thu, Oct 05, 2017 at 01:31:02PM +0200, Michal Privoznik wrote:
> On 10/05/2017 01:01 PM, Pavel Hrdina wrote:
> > On Thu, Oct 05, 2017 at 10:40:01AM +0100, Daniel P. Berrange wrote:
> >> On Thu, Oct 05, 2017 at 11:27:29AM +0200, Martin Kletzander wrote:
> >>> On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
> >>>> On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> >>>>> On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
> >>>>>> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
> >>>>>>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
> >>>>>>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
> >>>>>>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
> >>>>>>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +0200, Michal Privoznik wrote:
> >>>>>>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1434451
> >>>>>>>>>>>
> >>>>>>>>>>> It comes handy for management application to be able to have a
> >>>>>>>>>>> per-device label so that it can uniquely identify devices it
> >>>>>>>>>>> cares about. The advantage of this approach is that we don't have
> >>>>>>>>>>> to generate aliases at define time (non trivial amount of work
> >>>>>>>>>>> and problems). The only thing we do is parse the user supplied
> >>>>>>>>>>> UUID and format it back. For instance:
> >>>>>>>>>>>
> >>>>>>>>>>>    <disk type='block' device='disk'>
> >>>>>>>>>>>      <driver name='qemu' type='raw'/>
> >>>>>>>>>>>      <source dev='/dev/HostVG/QEMUGuest1'/>
> >>>>>>>>>>>      <target dev='hda' bus='ide'/>
> >>>>>>>>>>>      <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> >>>>>>>>>>>      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> >>>>>>>>>>>    </disk>
> >>>>>>>>>>>
> >>>>>>>>>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> >>>>>>>>>>> ---
> >>>>>>>>>>>
> >>>>>>>>>>> This is just a very basic implementation. If I get a green light on this, I can
> >>>>>>>>>>> implement the feature further, i.e. allow device lookup on the UUID. For
> >>>>>>>>>>> instance:
> >>>>>>>>>>>
> >>>>>>>>>>> virsh domiftune fedora $UUID $bandwidth
> >>>>>>>>>
> >>>>>>>>> <snip>
> >>>>>>>>>
> >>>>>>>>> I'm thinking that part of the problem we're having with agreeing how to
> >>>>>>>>> deal with this RFE is that we're over-analysing semantics, by wondering
> >>>>>>>>> whether its a unique name or UUID, its relation to alias, whether it has
> >>>>>>>>> bearing on APIs.
> >>>>>>>>>
> >>>>>>>>> How about we change tack, and do what we did when we needed application
> >>>>>>>>> specific information at the top level - just declare a general purpose
> >>>>>>>>> <metadata> element and say it is a completely opaque blob. Libvirt will
> >>>>>>>>> *never* do anything with it, other than to preserve it exactly as is.
> >>>>>>>>> No API will ever use the metadata in any way. Libvirt will never try to
> >>>>>>>>> guarantee uniqueness of metadata for each device. It can be JSON or a
> >>>>>>>>> gziped microsoft word document for all we care. Entirely upto the app
> >>>>>>>>> developer to decide what metadata is saved and guarantee uniqueness if
> >>>>>>>>> they so desired.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> That is kind of what I was aiming for.  But in order for it to be cleaner and
> >>>>>>>> easier to use from user as well (and not only mgmt apps) I thought the metadata
> >>>>>>>> would just be one identifier.  If you want to store more metadata for the
> >>>>>>>> device, then you can do all that in the domain metadata and just reference the
> >>>>>>>> particular device using the identifier if mgmt app wants to do that.
> >>>>>>>
> >>>>>>> Yes that is certainly possible. The caveats are that we still need a unique
> >>>>>>> identifier for the device, and the metadata update is not atomic wrt
> >>>>>>> to device hotplug.
> >>>>>>>
> >>>>>>
> >>>>>> Yes, well, our (libvirt) unique identifier is not going anywhere, so
> >>>>>> that's OK, we'll be using what we have been until now.
> >>>>>>
> >>>>>>> The plus side of the global metadata is that we have APIs to update it
> >>>>>>> on the fly already, and its fully namespaced to allow multiple independant
> >>>>>>> data sets to be stored.
> >>>>>>>
> >>>>>>
> >>>>>> Yes, exactly.
> >>>>>>
> >>>>>>> I don't think lack of atomicity is a big deal as you could order it so that
> >>>>>>> you update metadata before doing the hotplug. Then worst case you have a
> >>>>>>> device mentioned in metadata that doesn't exist, which is easy enough to
> >>>>>>> detect.
> >>>>>>>
> >>>>>>
> >>>>>> Right, if you want metadata for device, then you'll just update
> >>>>>> metadata, hotplug device, and if it failed you update the metadata once
> >>>>>> more.
> >>>>>>
> >>>>>> So are we on the same page?  By that I mean agreeing on any sane user-supplied
> >>>>>> identifier that we'll not guarantee uniqueness for, and neither will we use it
> >>>>>> for anything for now?  (We can deal with the issues regarding using it when
> >>>>>> someone wants to actually implement it).
> >>>>>
> >>>>> Per my reply to the earlier patch series, I'm now inclined to say that we
> >>>>> should
> >>>>>
> >>>>>  - Allow the mgmt app to set the aliases upfront
> >>>>>  - Auto-fill missing aliases at XML define time
> >>>>>
> >>>>> it has some downsides, but all the other solutions we've discussed have
> >>>>> their own downsides too. So on balance I think its not worth it to add
> >>>>> a second identifier for each device, when we already have alias.
> >>>
> >>> They are not the same thing.  Our alias is an ID we pass to QEMU and we
> >>> are dependent on it in multiple ways, plus we generate it with some of
> >>> our rules in mind.  Allowing users to modify this would add so many
> >>> possible problems that I would not consider next two releases stable.  I
> >>> would also not be confident enough to tell someone "just use the alias"
> >>> because I see all the places where it's not treated as just another
> >>> configurable parameter.  And there's no problem with that.  But there
> >>> will be if we allow users to set an identifier that's heavily used for
> >>> internal purposes.
> >>
> >> I honestly don't see where the major instability is going to come from ? We
> >> don't have code that cares about the format of the string used in the alias.
> >> What matters is what we keep track of the aliases for any running guests, and
> >> we do that via the state XML, so if libvirt changes the format of aliases it
> >> generates internally it would not break upgrades.
> > 
> > Actually we have some code that depends on the format of aliases.
> > Yes, it can be easily fixed but we may miss something.
> > In src/qemu/qemu_alias.c there is a lot of function that expect certain
> > format of aliases of existing devices in order to generate a new alias
> > for remaining devices and if they fail recognize the format they
> > will report an error instead of assigning new alias.
> > 
> > Another example what could go wrong is that in src/lxc/lxc_process.c
> > while starting new LXC process we simple generate new alias for consoles
> > regardless whether there was already one generated or not.
> 
> Worse. Imagine an user who gives a disk alias "vnet0". Now, when we
> would generate the aliases for the rest of devices, say interfaces, we
> would have to check all other devices and see if the alias we generated
> is not already in use. Possibly by any other device.

This is an easy problem to resolve though - no different to how we deal
with PCI addressing. Just populate a hash of all currently defined aliases
in the XML, then when generating the new alias, just increment the counter
until we find a free entry.


> Although, if we have the prefixed aliases, such clash would not be possible.

Even if mgmt apps need to use a fixed prefix, we still need the global hash
todo uniqueness checking of their aliases.

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