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

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(a)redhat.com&gt; >>>>>>>> --- >>>>>>>> >>>>>>>> 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. > > Question is if we are confident enough that: > > a) apps will provide unique aliases (since we'll be putting user input > onto qemu cmd line) > > b) apps will provide only allowed characters in the alias (not every > character can be in id=, can it?) We will have to validate both these points when looking at the XML. > But I think we still have not answered this question: what if we need to > change pattern by which we generate aliases in the future? On one hand, > an alias is just a string so the pattern should not matter. On the other > hand, that's not quite true. For instance, "pci.0" has a very special > meaning. IOW, if we now worry about users cutting off the branch they > are sitting on, this is like giving them flamethrower in fireworks > production hall. 'pci.0' is not an alias - 'pci' is the alias, the '0' is a bus number, so users only provide the first bit which has no special semantics other than needing to comply with a permitted set of characters and be unique. In terms of validation I think we should permit a-Z, 0-9 and -, upto a maximum of say 32 characters in length.