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 <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. > >>> > >>> 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list