On Thu, Oct 05, 2017 at 11:28:35AM +0200, Michal Privoznik wrote: > On 10/05/2017 11:13 AM, Daniel P. Berrange 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 <mpriv...@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. > >> > >> 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. > > Okay. We can check that. But now, does it make sense to generate the > aliases at define time? I mean, users could provide alias at define > time, and we can fill in the missing ones when starting up the domain. > Just like we're doing now.
I think its beneficial to set them at define time, as it provides a unique identifier that apps / admins can see throughout the lifetime of the guest, avoiding the need to both about setting them manually to a significant extent. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list