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. So provided we validate uniqueness & restrict charactersets / length, I don't see what users would do that would cause the instability. > So what if, just to ease the inclusion of this feature, we force very > strict rules on user-supplied aliases and maybe give them a prefix. > Let's say if you want to supply the alias yourself, it has to be in form > of: > > "ua-[a-zA-Z0-9]*" > > otherwise libvirt will generate the alias itself and the only thing we > guarantee is that our aliases will never start with 'ua-'? Yes, having a standard prefix that mgmt apps use would be a reasonable idea - it would protect them from possible future clashes with IDs that libvirt uses for devices that aren't exposed in the XML. 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
