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@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.

 docs/schemas/domaincommon.rng                     | 13 +++++++---
 src/conf/device_conf.c                            |  1 +
 src/conf/device_conf.h                            |  1 +
 src/conf/domain_conf.c                            | 20 ++++++++++-----
 tests/genericxml2xmlindata/generic-user-alias.xml | 31 +++++++++++++++++++++++
 tests/genericxml2xmltest.c                        |  2 ++
 6 files changed, 59 insertions(+), 9 deletions(-)
 create mode 100644 tests/genericxml2xmlindata/generic-user-alias.xml

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index bac371ea3..69c121ce9 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5864,9 +5864,16 @@
   </define>
   <define name='alias'>
     <element name='alias'>
-      <attribute name='name'>
-        <ref name='aliasName'/>
-      </attribute>
+      <optional>
+        <attribute name='name'>
+          <ref name='aliasName'/>
+        </attribute>
+      </optional>
+      <optional>
+        <attribute name='user'>
+          <ref name='aliasName'/>
+        </attribute>
+      </optional>
     </element>
     <empty/>
   </define>
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index d69f94fad..ced5db123 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -57,6 +57,7 @@ void
 virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 {
     VIR_FREE(info->alias);
+    VIR_FREE(info->user);
     memset(&info->addr, 0, sizeof(info->addr));
     info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
     VIR_FREE(info->romfile);
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index f87d6f1fc..08a9e57e3 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -135,6 +135,7 @@ typedef struct _virDomainDeviceInfo virDomainDeviceInfo;
 typedef virDomainDeviceInfo *virDomainDeviceInfoPtr;
 struct _virDomainDeviceInfo {
     char *alias;
+    char *user; /* user defined ID for the device */
     int type; /* virDomainDeviceAddressType */
     union {
         virPCIDeviceAddress pci;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 87192eb2d..885825226 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5756,6 +5756,8 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
                           virDomainDeviceInfoPtr info,
                           unsigned int flags)
 {
+    bool formatAlias = info->alias && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE);
+
     if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) {
         virBufferAsprintf(buf, "<boot order='%u'", info->bootIndex);
 
@@ -5764,9 +5766,13 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
 
         virBufferAddLit(buf, "/>\n");
     }
-    if (info->alias &&
-        !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
-        virBufferAsprintf(buf, "<alias name='%s'/>\n", info->alias);
+    if (formatAlias || info->user) {
+        virBufferAddLit(buf, "<alias");
+        if (formatAlias)
+            virBufferAsprintf(buf, " name='%s'", info->alias);
+        if (info->user)
+            virBufferAsprintf(buf, " user='%s'", info->user);
+        virBufferAddLit(buf, "/>\n");
     }
 
     if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) {
@@ -6327,7 +6333,6 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
     while (cur != NULL) {
         if (cur->type == XML_ELEMENT_NODE) {
             if (alias == NULL &&
-                !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
                 virXMLNodeNameEqual(cur, "alias")) {
                 alias = cur;
             } else if (address == NULL &&
@@ -6349,8 +6354,11 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
         cur = cur->next;
     }
 
-    if (alias)
-        info->alias = virXMLPropString(alias, "name");
+    if (alias) {
+        if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
+            info->alias = virXMLPropString(alias, "name");
+        info->user = virXMLPropString(alias, "user");
+    }
 
     if (master) {
         info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
diff --git a/tests/genericxml2xmlindata/generic-user-alias.xml b/tests/genericxml2xmlindata/generic-user-alias.xml
new file mode 100644
index 000000000..025924442
--- /dev/null
+++ b/tests/genericxml2xmlindata/generic-user-alias.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <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>
+    <controller type='usb' index='0'>
+      <alias user='myController0'/>
+    </controller>
+    <controller type='ide' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index 0377a05e9..63a62f6a5 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -130,6 +130,8 @@ mymain(void)
     DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, false,
                  TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
 
+    DO_TEST("user-alias");
+
     virObjectUnref(caps);
     virObjectUnref(xmlopt);
 
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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.

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.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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?

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
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.

> 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.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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).

> 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.

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.

> > 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.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Sep 29, 2017 at 3:49 PM, Michal Privoznik <mprivozn@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 <mprivozn@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

On 10/02/2017 05:52 PM, Roman Mohr wrote:
> On Fri, Sep 29, 2017 at 3:49 PM, Michal Privoznik <mprivozn@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 <mprivozn@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"?

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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Sep 29, 2017 at 09:06:01AM +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'/>

I really do not like this - having two arbitrary string alias names is
just crazy.  If we want to add a second attribute to uniquely identify
devices, then it should be a UUID IMHO, so it at least has some tangible
benefit instead of just duplicating the existing id format


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

On 09/29/2017 10:49 AM, Daniel P. Berrange wrote:
> On Fri, Sep 29, 2017 at 09:06:01AM +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'/>
> 
> I really do not like this - having two arbitrary string alias names is
> just crazy. 

Why is that? We have plenty of elements that do not match to anything at
hypervisor level. Firstly, there's metadata. Secondly, aliases in lxc
driver don't match anything either. And one can argue that the link in
qemu can be broken too. I mean, it's just for now that the aliases we
report happen to be device IDs we put onto the qemu's cmd line. Not to
mention devices that we put there and not report in the domain XML => no
IDs visible there.

> If we want to add a second attribute to uniquely identify
> devices, then it should be a UUID IMHO, so it at least has some tangible
> benefit instead of just duplicating the existing id format

I'm failing to see why UUID is better than any arbitrary string. You
mean we would generate the UUIDs if not supplied by user?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Sep 29, 2017 at 01:04:34PM +0200, Michal Privoznik wrote:
> On 09/29/2017 10:49 AM, Daniel P. Berrange wrote:
> > On Fri, Sep 29, 2017 at 09:06:01AM +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'/>
> > 
> > I really do not like this - having two arbitrary string alias names is
> > just crazy. 
> 
> Why is that? We have plenty of elements that do not match to anything at
> hypervisor level. Firstly, there's metadata. Secondly, aliases in lxc
> driver don't match anything either. And one can argue that the link in
> qemu can be broken too. I mean, it's just for now that the aliases we
> report happen to be device IDs we put onto the qemu's cmd line. Not to
> mention devices that we put there and not report in the domain XML => no
> IDs visible there.

> > If we want to add a second attribute to uniquely identify
> > devices, then it should be a UUID IMHO, so it at least has some tangible
> > benefit instead of just duplicating the existing id format
> 
> I'm failing to see why UUID is better than any arbitrary string. You
> mean we would generate the UUIDs if not supplied by user?

Some hypervisors could map UUIDs to individual devices, so it is a more
generally useful concept.

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

On Fri, Sep 29, 2017 at 12:09:52PM +0100, Daniel P. Berrange wrote:
> On Fri, Sep 29, 2017 at 01:04:34PM +0200, Michal Privoznik wrote:
> > On 09/29/2017 10:49 AM, Daniel P. Berrange wrote:
> > > On Fri, Sep 29, 2017 at 09:06:01AM +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'/>
> > > 
> > > I really do not like this - having two arbitrary string alias names is
> > > just crazy. 
> > 
> > Why is that? We have plenty of elements that do not match to anything at
> > hypervisor level. Firstly, there's metadata. Secondly, aliases in lxc
> > driver don't match anything either. And one can argue that the link in
> > qemu can be broken too. I mean, it's just for now that the aliases we
> > report happen to be device IDs we put onto the qemu's cmd line. Not to
> > mention devices that we put there and not report in the domain XML => no
> > IDs visible there.
> 
> > > If we want to add a second attribute to uniquely identify
> > > devices, then it should be a UUID IMHO, so it at least has some tangible
> > > benefit instead of just duplicating the existing id format
> > 
> > I'm failing to see why UUID is better than any arbitrary string. You
> > mean we would generate the UUIDs if not supplied by user?
> 
> Some hypervisors could map UUIDs to individual devices, so it is a more
> generally useful concept.

Also if we have APIs that accept an 'alias' string, we cannot extend them
to support the user's own 'alias' unless we guarantee the user supplied
alias is different from the alias we give to QEMU.  If we used UUID format,
however, we don't have any ambiguity between a string that's an alias and
a string that's a UUID.

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

On 09/29/2017 01:52 PM, Daniel P. Berrange wrote:
> On Fri, Sep 29, 2017 at 12:09:52PM +0100, Daniel P. Berrange wrote:
>> On Fri, Sep 29, 2017 at 01:04:34PM +0200, Michal Privoznik wrote:
>>> On 09/29/2017 10:49 AM, Daniel P. Berrange wrote:
>>>> On Fri, Sep 29, 2017 at 09:06:01AM +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'/>
>>>>
>>>> I really do not like this - having two arbitrary string alias names is
>>>> just crazy. 
>>>
>>> Why is that? We have plenty of elements that do not match to anything at
>>> hypervisor level. Firstly, there's metadata. Secondly, aliases in lxc
>>> driver don't match anything either. And one can argue that the link in
>>> qemu can be broken too. I mean, it's just for now that the aliases we
>>> report happen to be device IDs we put onto the qemu's cmd line. Not to
>>> mention devices that we put there and not report in the domain XML => no
>>> IDs visible there.
>>
>>>> If we want to add a second attribute to uniquely identify
>>>> devices, then it should be a UUID IMHO, so it at least has some tangible
>>>> benefit instead of just duplicating the existing id format
>>>
>>> I'm failing to see why UUID is better than any arbitrary string. You
>>> mean we would generate the UUIDs if not supplied by user?
>>
>> Some hypervisors could map UUIDs to individual devices, so it is a more
>> generally useful concept.
> 
> Also if we have APIs that accept an 'alias' string, we cannot extend them
> to support the user's own 'alias' unless we guarantee the user supplied
> alias is different from the alias we give to QEMU.  

We can if we document that libvirt generated aliases take precedence
over user ones. That way we can keep the backward compatibility.

> If we used UUID format,
> however, we don't have any ambiguity between a string that's an alias and
> a string that's a UUID.

So IIUC users would be able to specify UUID for devices they want and
for the rest libvirt will generate new one? That'll make XMLs a lot
bigger. If we will not generate UUIDs, but just store ones provided by
user this also makes sense then. Although, dealing with UUIDs is very
user unfriendly, but that ship sailed a long time ago :-P

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Sep 29, 2017 at 03:56:53PM +0200, Michal Privoznik wrote:
> On 09/29/2017 01:52 PM, Daniel P. Berrange wrote:
> > On Fri, Sep 29, 2017 at 12:09:52PM +0100, Daniel P. Berrange wrote:
> >> On Fri, Sep 29, 2017 at 01:04:34PM +0200, Michal Privoznik wrote:
> >>> On 09/29/2017 10:49 AM, Daniel P. Berrange wrote:
> >>>> On Fri, Sep 29, 2017 at 09:06:01AM +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'/>
> >>>>
> >>>> I really do not like this - having two arbitrary string alias names is
> >>>> just crazy. 
> >>>
> >>> Why is that? We have plenty of elements that do not match to anything at
> >>> hypervisor level. Firstly, there's metadata. Secondly, aliases in lxc
> >>> driver don't match anything either. And one can argue that the link in
> >>> qemu can be broken too. I mean, it's just for now that the aliases we
> >>> report happen to be device IDs we put onto the qemu's cmd line. Not to
> >>> mention devices that we put there and not report in the domain XML => no
> >>> IDs visible there.
> >>
> >>>> If we want to add a second attribute to uniquely identify
> >>>> devices, then it should be a UUID IMHO, so it at least has some tangible
> >>>> benefit instead of just duplicating the existing id format
> >>>
> >>> I'm failing to see why UUID is better than any arbitrary string. You
> >>> mean we would generate the UUIDs if not supplied by user?
> >>
> >> Some hypervisors could map UUIDs to individual devices, so it is a more
> >> generally useful concept.
> > 
> > Also if we have APIs that accept an 'alias' string, we cannot extend them
> > to support the user's own 'alias' unless we guarantee the user supplied
> > alias is different from the alias we give to QEMU.  
> 
> We can if we document that libvirt generated aliases take precedence
> over user ones. That way we can keep the backward compatibility.

No, that's fragile. If libvirt were ever to change its alias name for
something there is a risk it might clash with an app name, which previously
did not clash.

> > If we used UUID format,
> > however, we don't have any ambiguity between a string that's an alias and
> > a string that's a UUID.
> 
> So IIUC users would be able to specify UUID for devices they want and
> for the rest libvirt will generate new one? That'll make XMLs a lot
> bigger. If we will not generate UUIDs, but just store ones provided by
> user this also makes sense then. Although, dealing with UUIDs is very
> user unfriendly, but that ship sailed a long time ago :-P

I would not generate UUIDs - only have them if the app asked for them, or
if the hypervisor we're using has assigned them itself.

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

On Fri, Sep 29, 2017 at 09:06:01AM +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@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.

Can you elaborate on you mean by the cold plug problem here ?

Are you saying that if you cold plug a single device, we would regenerate
aliases on every single other device too ?  I would surely think we can
avoid that kind of thing.

To make hotplug easier for applications we could add a V2 hotplug API
which *returns* the fully-expanded device XML.

That way applications would immediately learn the alias of the newly
hotplugged device in a simple manner.

While we were at it, this would give us opportunity to make the v2
hotplug API support hotplugging of multiple devices at once. This is
a feature we've needed for PCI assignment where some devices have to
be co-assigned at the same time.

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

On 10/03/2017 04:19 PM, Daniel P. Berrange wrote:
> On Fri, Sep 29, 2017 at 09:06:01AM +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@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.
> 
> Can you elaborate on you mean by the cold plug problem here ?
> 
> Are you saying that if you cold plug a single device, we would regenerate
> aliases on every single other device too ?  I would surely think we can
> avoid that kind of thing.

Not every other device, but lets consider interfaces for instance. Lets
say that a domain has two interfaces: vnet0 and vnet1. User detaches
vnet0. Should vnet1 be renamed to vnet1? Domain is not running, so it
can. But that puts a lot of burden on mgmt app to fix its internal state
as a result of the rename. Okay, so we don't rename it. The next
interface plugged is going to have name of vnet2. So in the end we will
have discontinuous aliases.

But from greater perspective, since users/mgmt app are not the one who
generate the alias, we would be just transforming problem to a different
one. I mean, even now can users uniquely identify devices - by addresses
(PCI, USB, ...) Having them to find alias we generated for the device
they are interested in is the same problem as finding its address.
Unless they can specify the ID (whether its attribute of <alias/> or
<metadata/> per device, or whatever), they will be chasing libvirt.

Worse, what if we need to change the scheme by which the aliases are
generated? We can't do that for persistent domains as we would be
breaking the link between what mgmt app expects to see and what it
actually sees. You know what I mean.

> 
> To make hotplug easier for applications we could add a V2 hotplug API
> which *returns* the fully-expanded device XML.
> 
> That way applications would immediately learn the alias of the newly
> hotplugged device in a simple manner.
> 
> While we were at it, this would give us opportunity to make the v2
> hotplug API support hotplugging of multiple devices at once. This is
> a feature we've needed for PCI assignment where some devices have to
> be co-assigned at the same time.

Feels a bit out of scope of this feature. Definitely worth having such
API, just not quite sure it fits within this feature. I mean, that'd be
a lot of work.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Tue, Oct 03, 2017 at 05:08:53PM +0200, Michal Privoznik wrote:
> On 10/03/2017 04:19 PM, Daniel P. Berrange wrote:
> > On Fri, Sep 29, 2017 at 09:06:01AM +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@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.
> > 
> > Can you elaborate on you mean by the cold plug problem here ?
> > 
> > Are you saying that if you cold plug a single device, we would regenerate
> > aliases on every single other device too ?  I would surely think we can
> > avoid that kind of thing.
> 
> Not every other device, but lets consider interfaces for instance. Lets
> say that a domain has two interfaces: vnet0 and vnet1. User detaches
> vnet0. Should vnet1 be renamed to vnet1? Domain is not running, so it
> can. But that puts a lot of burden on mgmt app to fix its internal state
> as a result of the rename. Okay, so we don't rename it. The next
> interface plugged is going to have name of vnet2. So in the end we will
> have discontinuous aliases.

Discontinuous aliases is a complete non-issue IMHO. The alias is an
opaque string, so no one should care whether the last digit is a 0
or a 2.  The scenario already happens if you hotplug/hotunplug a
bunch of devices - the VM will have discontinuous aliases for as long
as it carries on running.

> But from greater perspective, since users/mgmt app are not the one who
> generate the alias, we would be just transforming problem to a different
> one. I mean, even now can users uniquely identify devices - by addresses
> (PCI, USB, ...) Having them to find alias we generated for the device
> they are interested in is the same problem as finding its address.
> Unless they can specify the ID (whether its attribute of <alias/> or
> <metadata/> per device, or whatever), they will be chasing libvirt.

Apps would rely on fact that the devices appear in the XML  they get
back in the same order as they originally had in the input XML. The
troublespoot is around devices which magically appear - <controller>
elemenets essentially.

> Worse, what if we need to change the scheme by which the aliases are
> generated? We can't do that for persistent domains as we would be
> breaking the link between what mgmt app expects to see and what it
> actually sees. You know what I mean.

Yes, if we generate aliases at define time we must honour those aliases
for as long as that persistent config exists & never change them behind
the apps back. This would imply we need to persist the aliases into
/etc/libvirt/qemu/ so they're saved across restarts of libvirtd.

Hmm, once we persist them in /etc/libvirt/qemu, we have effectively got
the same scenario as if we just let applications define aliases upfront
in the XML they give us, as its the same code paths.

I wonder if we just have to give up & let apps define aliases themselves.
The thing I disliked about that is that the permitted alias format is
hypervisor specific, but perhaps we just have to document restrictions
per hypervisor.

> > To make hotplug easier for applications we could add a V2 hotplug API
> > which *returns* the fully-expanded device XML.
> > 
> > That way applications would immediately learn the alias of the newly
> > hotplugged device in a simple manner.
> > 
> > While we were at it, this would give us opportunity to make the v2
> > hotplug API support hotplugging of multiple devices at once. This is
> > a feature we've needed for PCI assignment where some devices have to
> > be co-assigned at the same time.
> 
> Feels a bit out of scope of this feature. Definitely worth having such
> API, just not quite sure it fits within this feature. I mean, that'd be
> a lot of work.
> 
> Michal

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