On Fri, Sep 29, 2017 at 3:49 PM, Michal Privoznik <mpriv...@redhat.com> 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 <mpriv...@redhat.com> > >>>> --- > >>>> > >>>> 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"? Best Regards, Roman > > Michal > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list >
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list