[libvirt] [PATCH alt] conf: Allow user define their own alias
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
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.
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
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.
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
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
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
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 :|
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
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 :|
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 :|
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
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 :|
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 :|
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
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 :|
participants (4)
-
Daniel P. Berrange -
Michal Privoznik -
Peter Krempa -
Roman Mohr