Re: [libvirt] [PATCH alt] conf: Allow user define their own alias

On 09/29/2017 01:16 PM, Peter Krempa wrote: > On Fri, Sep 29, 2017 at 12:57:29 +0200, Michal Privoznik wrote: >> On 09/29/2017 09:52 AM, Peter Krempa wrote: >>> On Fri, Sep 29, 2017 at 09:06:01 +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 >>>> tag 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'/> >>>> <alias user='myDisk0'/> >>>> <address type='drive' controller='0' bus='0' target='0' unit='0'/> >>>> </disk> >>>> >>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >>>> --- >>>> >>>> An alternative approach to: >>>> >>>> https://www.redhat.com/archives/libvir-list/2017- September/msg00765.html >>>> >>>> Honestly, I prefer this one as it's simpler and we don't have to care about >>>> devices changing their aliases on cold plug. I mean, on cold (un-)plug we'd >>>> have to regenerate the aliases so it might be hard to track certain device. >>>> But with this approach, it's no problem. >>>> >>>> Also, I'm not completely convinced about the name of @user attribute. So I'm >>>> open for suggestions. >>> >>> With this proposed design you need to make sure to document that the >>> user alias is _not_ guaranteed to be unique and also that it can't be >>> used to match the device in any way. >>> >> >> Sure. I'll just add it at the end of the paragraph that describes >> <alias/>. Err, hold on. There's none! But okay, I think I can find a >> place to add it there. >> >> Even though my patch doesn't implement it (simply because I wanted to >> have ACK on the design so I've saved it for follow up patches), I don't >> quite see why we can't match user supplied aliases? > > I think it will introduce more problems than solve. You will need to > make sure that it's unique even across hotplug and add lookup code for > everything. Additionally you can't add that as a mandatory field when > looking up, since some device classes allow lookup only with partial XML > descriptions (for unplug/update). It can't be mandatory, that's the whole point. Sure. Well, in DomainPostParse I can check for uniqueness pretty easily: just create an empty list and start adding all strings provided by user. If the string we're trying to add is already in the list we just error out. Sure, I'll have to iterate over all devices, but that should be pretty easy since we have virDomainDeviceInfoIterate(). All that's needed is to write the callback function (= a few lines of code). Then, on hotplug goto 10. IOW, if user alias is provided just check for its uniqueness. > >> virsh domif-setlink fedora myNet down >> >> This has the great advantage of being ordering agnostic. You don't have >> to check for the alias of the device you want to modify (as aliases can >> change across different startups, right?). True, for that we would have > > Well, you've used a bad example for this. 'domif-setlink' uses target and > mac address, both of which are stable and don't rely on ordering on > startup. Same thing applies to disks. Oh right. domiftune is more like it. > > The matching in virsh in this particular command is done by looking for > the full element and then using that with the UpdateDevice() API, so the > lookup is basically syntax sugar. > >> to make sure that the supplied aliases are unique per domain (which is >> trivial to achieve). But apart from that I don't quite see why we >> shouldn't/can't do it. > > Well, I don't think it's trivial. It's simpler than providing the alias > which would be used with qemu, but you still have a zillion hotplug code > paths which would need to check this. Well, it might be slightly tricky yes. But in general, the virDomain*Find() would try to match the user alias first (if provided) else continue with current behaviour. I'm not quite sure what you mean by zillion code paths. > >>> I think that users which know semantics of the current alias may be very >>> confused by putting user data with different semantics into the same >>> field. >>> >> >> Yep. As I say, I'm not happy with the name either. Any suggestion is >> welcome. So a separate element then? Naming is hard. > > I'm voting for separate element in case there's consensus that this > route is a good idea. > Yeah, it may look a bit better. Any idea on the element name?

Michal -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list