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