On 10/02/2017 05:52 PM, Roman Mohr wrote: > On Fri, Sep 29, 2017 at 3:49 PM, Michal Privoznik <mprivozn@xxxxxxxxxx> > wrote: > >> 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@xxxxxxxxxx> >>>>>> --- >>>>>> >>>>>> 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? >> > > > How about "label" or "userlabel"? Looks like the approach that has the highest chance of merging is UUID stored in a separate element. For instance: <interface type="network"> <source network="default"/> <alias name="net0"/> <!-- if this is the live XML --> <uuid>XXX</uuid> </interface> I still think that we don't have to be that restrictive and just allow users to store any string (including UUID), but whatever. I'll implement this approach. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
