5:58 a.m.
v2 of:
https://www.redhat.com/archives/libvir-list/2017-September/msg01077.html
diff to v1:
- Switched from arbitrary string to UUID
- Moved UUID to separate element
- Added docs
Michal Privoznik (3):
virDomainDeviceInfoParseXML: Separate address parsing into separate
func
virDomainDeviceInfoParseXML: Clear whole @info on failure
conf: Allow users to define UUID for devices
docs/formatdomain.html.in | 21 +++
docs/schemas/domaincommon.rng | 21 ++-
src/conf/device_conf.c | 1 +
src/conf/device_conf.h | 1 +
src/conf/domain_conf.c | 201 ++++++++++++---------
tests/genericxml2xmlindata/generic-device-uuid.xml | 31 ++++
tests/genericxml2xmltest.c | 1 +
7 files changed, 189 insertions(+), 88 deletions(-)
create mode 100644 tests/genericxml2xmlindata/generic-device-uuid.xml
--
2.13.5
5:58 a.m.
New subject: [libvirt] [PATCH v2 1/3] virDomainDeviceInfoParseXML: Separate address parsing into separate func
There's one 'return' in the middle of the function body. It's
very easy to miss and so it makes adding new code harder. Also
the function doesn't follow our style 100%.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 174 ++++++++++++++++++++++++++-----------------------
1 file changed, 93 insertions(+), 81 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 87192eb2d..0bc2e2f94 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6303,6 +6303,93 @@ virDomainDeviceDimmAddressParseXML(xmlNodePtr node,
}
+static int
+virDomainDeviceAddressParseXML(xmlNodePtr address,
+ virDomainDeviceInfoPtr info)
+{
+ int ret = -1;
+ char *type = virXMLPropString(address, "type");
+
+ if (type) {
+ if ((info->type = virDomainDeviceAddressTypeFromString(type)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown address type '%s'"), type);
+ goto cleanup;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("No type specified for device
address"));
+ goto cleanup;
+ }
+
+ switch ((virDomainDeviceAddressType) info->type) {
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+ if (virPCIDeviceAddressParseXML(address, &info->addr.pci) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+ if (virDomainDeviceDriveAddressParseXML(address, &info->addr.drive) <
0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
+ if (virDomainDeviceVirtioSerialAddressParseXML
+ (address, &info->addr.vioserial) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
+ if (virDomainDeviceCcidAddressParseXML(address, &info->addr.ccid) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+ if (virDomainDeviceUSBAddressParseXML(address, &info->addr.usb) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
+ if (virDomainDeviceSpaprVioAddressParseXML(address, &info->addr.spaprvio)
< 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
+ if (virDomainDeviceCCWAddressParseXML
+ (address, &info->addr.ccw) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
+ if (virDomainDeviceISAAddressParseXML(address, &info->addr.isa) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("virtio-s390 bus doesn't have an address"));
+ goto cleanup;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
+ if (virDomainDeviceDimmAddressParseXML(address, &info->addr.dimm) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
+ case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
+ break;
+ }
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(type);
+ return ret;
+}
+
+
/* Parse the XML definition for a device address
* @param node XML nodeset to parse for device address definition
*/
@@ -6319,6 +6406,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
xmlNodePtr boot = NULL;
xmlNodePtr rom = NULL;
char *type = NULL;
+ char *rombar = NULL;
int ret = -1;
virDomainDeviceInfoClear(info);
@@ -6364,102 +6452,26 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
}
if (rom) {
- char *rombar = virXMLPropString(rom, "bar");
- if (rombar &&
+ if ((rombar = virXMLPropString(rom, "bar")) &&
((info->rombar = virTristateSwitchTypeFromString(rombar)) <= 0)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown rom bar value '%s'"), rombar);
- VIR_FREE(rombar);
goto cleanup;
}
- VIR_FREE(rombar);
info->romfile = virXMLPropString(rom, "file");
}
- if (!address)
- return 0;
-
- type = virXMLPropString(address, "type");
-
- if (type) {
- if ((info->type = virDomainDeviceAddressTypeFromString(type)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown address type '%s'"), type);
- goto cleanup;
- }
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("No type specified for device
address"));
- goto cleanup;
- }
-
- switch ((virDomainDeviceAddressType) info->type) {
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
- if (virPCIDeviceAddressParseXML(address, &info->addr.pci) < 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
- if (virDomainDeviceDriveAddressParseXML(address, &info->addr.drive) <
0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL:
- if (virDomainDeviceVirtioSerialAddressParseXML
- (address, &info->addr.vioserial) < 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCID:
- if (virDomainDeviceCcidAddressParseXML(address, &info->addr.ccid) < 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
- if (virDomainDeviceUSBAddressParseXML(address, &info->addr.usb) < 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO:
- if (virDomainDeviceSpaprVioAddressParseXML(address, &info->addr.spaprvio)
< 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW:
- if (virDomainDeviceCCWAddressParseXML
- (address, &info->addr.ccw) < 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO:
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_ISA:
- if (virDomainDeviceISAAddressParseXML(address, &info->addr.isa) < 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390:
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("virtio-s390 bus doesn't have an address"));
+ if (address &&
+ virDomainDeviceAddressParseXML(address, info) < 0)
goto cleanup;
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM:
- if (virDomainDeviceDimmAddressParseXML(address, &info->addr.dimm) < 0)
- goto cleanup;
- break;
-
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE:
- case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST:
- break;
- }
ret = 0;
-
cleanup:
- if (ret == -1)
+ if (ret < 0)
VIR_FREE(info->alias);
VIR_FREE(type);
+ VIR_FREE(rombar);
return ret;
}
--
2.13.5
6:52 a.m.
New subject: [libvirt] [PATCH v2 1/3] virDomainDeviceInfoParseXML: Separate address parsing into separate func
On Tue, Oct 03, 2017 at 12:58:57PM +0200, Michal Privoznik wrote:
There's one 'return' in the middle of the function body.
It's
very easy to miss and so it makes adding new code harder. Also
the function doesn't follow our style 100%.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 174 ++++++++++++++++++++++++++-----------------------
1 file changed, 93 insertions(+), 81 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 87192eb2d..0bc2e2f94 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6364,102 +6452,26 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
}
if (rom) {
- char *rombar = virXMLPropString(rom, "bar");
- if (rombar &&
+ if ((rombar = virXMLPropString(rom, "bar")) &&
If this is what you meant by "follow our coding style 100%", then I
think you are wrong since this (IMHO bad) style is not explicitly
mentioned, only (mis)used once. But I'm not fighting over it since I
know more people prefer less readable code that saves one or two lines =)
ACK
7:01 a.m.
New subject: [libvirt] [PATCH v2 1/3] virDomainDeviceInfoParseXML: Separate address parsing into separate func
On 10/03/2017 01:52 PM, Martin Kletzander wrote:
On Tue, Oct 03, 2017 at 12:58:57PM +0200, Michal Privoznik wrote:
> There's one 'return' in the middle of the function body. It's
> very easy to miss and so it makes adding new code harder. Also
> the function doesn't follow our style 100%.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/conf/domain_conf.c | 174
> ++++++++++++++++++++++++++-----------------------
> 1 file changed, 93 insertions(+), 81 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 87192eb2d..0bc2e2f94 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6364,102 +6452,26 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
> }
>
> if (rom) {
> - char *rombar = virXMLPropString(rom, "bar");
> - if (rombar &&
> + if ((rombar = virXMLPropString(rom, "bar")) &&
If this is what you meant by "follow our coding style 100%", then I
think you are wrong since this (IMHO bad) style is not explicitly
mentioned, only (mis)used once. But I'm not fighting over it since I
know more people prefer less readable code that saves one or two lines =)
No, I was referring to return in the middle of the function. And then to
freeing variables before goto statements, and not freeing them under the
cleanup label. Whatever,
ACK
this is important to me :-)
Thanks,
Michal
5:58 a.m.
New subject: [libvirt] [PATCH v2 2/3] virDomainDeviceInfoParseXML: Clear whole @info on failure
Currently, if parsing of device info fails info->alias is freed.
It doesn't make much sense to leave the rest of the struct
behind.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0bc2e2f94..67095114c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6469,7 +6469,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
ret = 0;
cleanup:
if (ret < 0)
- VIR_FREE(info->alias);
+ virDomainDeviceInfoClear(info);
VIR_FREE(type);
VIR_FREE(rombar);
return ret;
--
2.13.5
6:16 a.m.
New subject: [libvirt] [PATCH v2 2/3] virDomainDeviceInfoParseXML: Clear whole @info on failure
On Tue, Oct 03, 2017 at 12:58:58PM +0200, Michal Privoznik wrote:
Currently, if parsing of device info fails info->alias is freed.
It doesn't make much sense to leave the rest of the struct
behind.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK, trivial
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0bc2e2f94..67095114c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6469,7 +6469,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
ret = 0;
cleanup:
if (ret < 0)
- VIR_FREE(info->alias);
+ virDomainDeviceInfoClear(info);
VIR_FREE(type);
VIR_FREE(rombar);
return ret;
--
2.13.5
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5:58 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
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
UUID 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'/>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
This is just a very basic implementation. If I get a green light on this, I can
implement the feature further, i.e. allow device lookup on the UUID. For
instance:
virsh domiftune fedora $UUID $bandwidth
docs/formatdomain.html.in | 21 +++++++++++++++
docs/schemas/domaincommon.rng | 21 ++++++++++-----
src/conf/device_conf.c | 1 +
src/conf/device_conf.h | 1 +
src/conf/domain_conf.c | 25 +++++++++++++++++
tests/genericxml2xmlindata/generic-device-uuid.xml | 31 ++++++++++++++++++++++
tests/genericxml2xmltest.c | 1 +
7 files changed, 95 insertions(+), 6 deletions(-)
create mode 100644 tests/genericxml2xmlindata/generic-device-uuid.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 5dcf2fedb..b4b2751fd 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3512,6 +3512,27 @@
</dd>
</dl>
+ <p>
+ To help management applications identify devices they care about, devices
+ have an optional <code><uuid/></code> sub-element which
can hold
+ UUID label generated by the management application. For instance:
+ </p>
+
+<pre>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <uuid>2d0ad0dc-3eaa-4295-9d62-3e531197ed7a</uuid>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+</pre>
+
+ <p>
+ The <code><uuid/></code> is available
+ <span class="since">since 3.9.0</span>.
+ </p>
+
<h4><a id="elementsVirtio">Virtio-related
options</a></h4>
<p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index bac371ea3..61d8430bb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5863,12 +5863,21 @@
</data>
</define>
<define name='alias'>
- <element name='alias'>
- <attribute name='name'>
- <ref name='aliasName'/>
- </attribute>
- </element>
- <empty/>
+ <interleave>
+ <optional>
+ <element name='alias'>
+ <attribute name='name'>
+ <ref name='aliasName'/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
+ <optional>
+ <element name='uuid'>
+ <ref name="UUID"/>
+ </element>
+ </optional>
+ </interleave>
</define>
<define name="panic">
<element name="panic">
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index d69f94fad..a732731eb 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->uuid);
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..f80017291 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;
+ unsigned char *uuid; /* user defined UUID for the device, might be NULL */
int type; /* virDomainDeviceAddressType */
union {
virPCIDeviceAddress pci;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 67095114c..2ea26dd9f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5764,11 +5764,18 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+
if (info->alias &&
!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)) {
virBufferAsprintf(buf, "<alias name='%s'/>\n",
info->alias);
}
+ if (info->uuid) {
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(info->uuid, uuidstr);
+ virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuidstr);
+ }
+
if (info->mastertype == VIR_DOMAIN_CONTROLLER_MASTER_USB) {
virBufferAsprintf(buf, "<master startport='%d'/>\n",
info->master.usb.startport);
@@ -6403,10 +6410,12 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
xmlNodePtr address = NULL;
xmlNodePtr master = NULL;
xmlNodePtr alias = NULL;
+ xmlNodePtr uuid = NULL;
xmlNodePtr boot = NULL;
xmlNodePtr rom = NULL;
char *type = NULL;
char *rombar = NULL;
+ char *uuidstr = NULL;
int ret = -1;
virDomainDeviceInfoClear(info);
@@ -6418,6 +6427,9 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
virXMLNodeNameEqual(cur, "alias")) {
alias = cur;
+ } else if (uuid == NULL &&
+ virXMLNodeNameEqual(cur, "uuid")) {
+ uuid = cur;
} else if (address == NULL &&
virXMLNodeNameEqual(cur, "address")) {
address = cur;
@@ -6440,6 +6452,18 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
if (alias)
info->alias = virXMLPropString(alias, "name");
+ if (uuid &&
+ (uuidstr = virXMLNodeContentString(uuid))) {
+ if (VIR_ALLOC_N(info->uuid, VIR_UUID_BUFLEN) < 0)
+ goto cleanup;
+
+ if (virUUIDParse(uuidstr, info->uuid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("malformed uuid element"));
+ goto cleanup;
+ }
+ }
+
if (master) {
info->mastertype = VIR_DOMAIN_CONTROLLER_MASTER_USB;
if (virDomainDeviceUSBMasterParseXML(master, &info->master.usb) < 0)
@@ -6472,6 +6496,7 @@ virDomainDeviceInfoParseXML(xmlNodePtr node,
virDomainDeviceInfoClear(info);
VIR_FREE(type);
VIR_FREE(rombar);
+ VIR_FREE(uuidstr);
return ret;
}
diff --git a/tests/genericxml2xmlindata/generic-device-uuid.xml
b/tests/genericxml2xmlindata/generic-device-uuid.xml
new file mode 100644
index 000000000..6deac5f9c
--- /dev/null
+++ b/tests/genericxml2xmlindata/generic-device-uuid.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'/>
+ <uuid>2d0ad0dc-3eaa-4295-9d62-3e531197ed7a</uuid>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'>
+ <uuid>4d21cdd8-ab68-4964-a879-9b9198f9f097</uuid>
+ </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..e0665aa25 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -130,6 +130,7 @@ mymain(void)
DO_TEST_FULL("chardev-reconnect-invalid-mode", 0, false,
TEST_COMPARE_DOM_XML2XML_RESULT_FAIL_PARSE);
+ DO_TEST("device-uuid");
virObjectUnref(caps);
virObjectUnref(xmlopt);
--
2.13.5
7:11 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Tue, Oct 03, 2017 at 12:58:59PM +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
UUID 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'/>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
This is just a very basic implementation. If I get a green light on this, I can
implement the feature further, i.e. allow device lookup on the UUID. For
instance:
virsh domiftune fedora $UUID $bandwidth
I'll be frank with you, I hate the fact that this can only be UUID and
there are reasons. Even though other will^Wmight not share my view.
I don't want to ACK this patch even though it probably works OK. At the
same time I'm not NACKing it since I might get overruled.
OK, it's my bad I didn't reply on the previous version, but I don't like
this so much I'll reply to it even in this new version.
So let me sum it up:
1) UUIDs are more generally useful concept
Allowing arbitrary (but sane, e.g. [a-zA-Z0-9\-_.]) also inherently
allows UUID is mgmt apps want to use them. This field should never
be used by a hypervisor, only by drivers and if necessary.
2) We cannot extend current APIs to support it, it can clash with
libvirt's alias in the future.
I don't see why not. Searching by IDs (or user aliases, whatever
you call it) does not have to be supported at all. Mgmt apps can
use is only as a part of domain definition. If we want to add such
functionality, then it can be turned on by a flag or flags, e.g.:
- VIR_BLAH_BY_ID
- VIR_BLAH_PREFER_ID
- VIR_BLAH_WHATEVER
But there is no requirement for us to provide this. It should not
be blocking the inclusion of this feature.
3) We have to add a lot of code that handles duplicates, etc.
No. No, we don't. Duplicates can not only be handled by mgmt app
itself, it could also be beneficial. Imagine I have bunch of
devices that share some source or are similar in a way and I just
name them the same. Even if we want to provide a way of specifying
devices for APIs by this user supplied string, we can just unplug
the first one we'll find.
By not requiring that it's an ID, users can take benefit of ti too. And
making sure it's an UUID seems like a waste of computing power.
Sometimes I feel like we are trying to do too much, even stuff that
nobody will effectively use and then we have to support it. The same
with solving corner cases in which we could just error out instead of
wasting manpower on them.
My (way more than) 2 cents.
Have a nice day,
Martin
8:53 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
On Tue, Oct 03, 2017 at 12:58:59PM +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
> UUID 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'/>
> <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
> </disk>
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
>
> This is just a very basic implementation. If I get a green light on this, I can
> implement the feature further, i.e. allow device lookup on the UUID. For
> instance:
>
> virsh domiftune fedora $UUID $bandwidth
<snip>
I'm thinking that part of the problem we're having with agreeing how to
deal with this RFE is that we're over-analysing semantics, by wondering
whether its a unique name or UUID, its relation to alias, whether it has
bearing on APIs.
How about we change tack, and do what we did when we needed application
specific information at the top level - just declare a general purpose
<metadata> element and say it is a completely opaque blob. Libvirt will
*never* do anything with it, other than to preserve it exactly as is.
No API will ever use the metadata in any way. Libvirt will never try to
guarantee uniqueness of metadata for each device. It can be JSON or a
gziped microsoft word document for all we care. Entirely upto the app
developer to decide what metadata is saved and guarantee uniqueness if
they so desired.
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 :|
9:03 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
> On Tue, Oct 03, 2017 at 12:58:59PM +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
> > UUID 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'/>
> > <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
> > </disk>
> >
> > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> > ---
> >
> > This is just a very basic implementation. If I get a green light on this, I can
> > implement the feature further, i.e. allow device lookup on the UUID. For
> > instance:
> >
> > virsh domiftune fedora $UUID $bandwidth
<snip>
I'm thinking that part of the problem we're having with agreeing how to
deal with this RFE is that we're over-analysing semantics, by wondering
whether its a unique name or UUID, its relation to alias, whether it has
bearing on APIs.
How about we change tack, and do what we did when we needed application
specific information at the top level - just declare a general purpose
<metadata> element and say it is a completely opaque blob. Libvirt will
*never* do anything with it, other than to preserve it exactly as is.
No API will ever use the metadata in any way. Libvirt will never try to
guarantee uniqueness of metadata for each device. It can be JSON or a
gziped microsoft word document for all we care. Entirely upto the app
developer to decide what metadata is saved and guarantee uniqueness if
they so desired.
That is kind of what I was aiming for. But in order for it to be cleaner and
easier to use from user as well (and not only mgmt apps) I thought the metadata
would just be one identifier. If you want to store more metadata for the
device, then you can do all that in the domain metadata and just reference the
particular device using the identifier if mgmt app wants to do that.
I'm just trying to see this from both of the big pictures. a) making it usable
for mgmt apps and b) not constraining it to programs, but also allowing users to
take "advantage" of that.
So in the end metadata feels better than uuid, but I think we could still make
it *way* simpler. Not only for mgmt apps and users, but also for us as libvirt
developers by minimizing the technical debt and possible future drawbacks.
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 :|
9:10 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
> > On Tue, Oct 03, 2017 at 12:58:59PM +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
> > > UUID 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'/>
> > > <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > > <address type='drive' controller='0'
bus='0' target='0' unit='0'/>
> > > </disk>
> > >
> > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> > > ---
> > >
> > > This is just a very basic implementation. If I get a green light on this,
I can
> > > implement the feature further, i.e. allow device lookup on the UUID. For
> > > instance:
> > >
> > > virsh domiftune fedora $UUID $bandwidth
>
> <snip>
>
> I'm thinking that part of the problem we're having with agreeing how to
> deal with this RFE is that we're over-analysing semantics, by wondering
> whether its a unique name or UUID, its relation to alias, whether it has
> bearing on APIs.
>
> How about we change tack, and do what we did when we needed application
> specific information at the top level - just declare a general purpose
> <metadata> element and say it is a completely opaque blob. Libvirt will
> *never* do anything with it, other than to preserve it exactly as is.
> No API will ever use the metadata in any way. Libvirt will never try to
> guarantee uniqueness of metadata for each device. It can be JSON or a
> gziped microsoft word document for all we care. Entirely upto the app
> developer to decide what metadata is saved and guarantee uniqueness if
> they so desired.
>
That is kind of what I was aiming for. But in order for it to be cleaner and
easier to use from user as well (and not only mgmt apps) I thought the metadata
would just be one identifier. If you want to store more metadata for the
device, then you can do all that in the domain metadata and just reference the
particular device using the identifier if mgmt app wants to do that.
Yes that is certainly possible. The caveats are that we still need a unique
identifier for the device, and the metadata update is not atomic wrt
to device hotplug.
The plus side of the global metadata is that we have APIs to update it
on the fly already, and its fully namespaced to allow multiple independant
data sets to be stored.
I don't think lack of atomicity is a big deal as you could order it so that
you update metadata before doing the hotplug. Then worst case you have a
device mentioned in metadata that doesn't exist, which is easy enough to
detect.
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 :|
1:31 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
> > On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
> > > On Tue, Oct 03, 2017 at 12:58:59PM +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
> > > > UUID 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'/>
> > > > <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > > > <address type='drive' controller='0'
bus='0' target='0' unit='0'/>
> > > > </disk>
> > > >
> > > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> > > > ---
> > > >
> > > > This is just a very basic implementation. If I get a green light on
this, I can
> > > > implement the feature further, i.e. allow device lookup on the UUID.
For
> > > > instance:
> > > >
> > > > virsh domiftune fedora $UUID $bandwidth
> >
> > <snip>
> >
> > I'm thinking that part of the problem we're having with agreeing how to
> > deal with this RFE is that we're over-analysing semantics, by wondering
> > whether its a unique name or UUID, its relation to alias, whether it has
> > bearing on APIs.
> >
> > How about we change tack, and do what we did when we needed application
> > specific information at the top level - just declare a general purpose
> > <metadata> element and say it is a completely opaque blob. Libvirt will
> > *never* do anything with it, other than to preserve it exactly as is.
> > No API will ever use the metadata in any way. Libvirt will never try to
> > guarantee uniqueness of metadata for each device. It can be JSON or a
> > gziped microsoft word document for all we care. Entirely upto the app
> > developer to decide what metadata is saved and guarantee uniqueness if
> > they so desired.
> >
>
> That is kind of what I was aiming for. But in order for it to be cleaner and
> easier to use from user as well (and not only mgmt apps) I thought the metadata
> would just be one identifier. If you want to store more metadata for the
> device, then you can do all that in the domain metadata and just reference the
> particular device using the identifier if mgmt app wants to do that.
Yes that is certainly possible. The caveats are that we still need a unique
identifier for the device, and the metadata update is not atomic wrt
to device hotplug.
Yes, well, our (libvirt) unique identifier is not going anywhere, so
that's OK, we'll be using what we have been until now.
The plus side of the global metadata is that we have APIs to update
it
on the fly already, and its fully namespaced to allow multiple independant
data sets to be stored.
Yes, exactly.
I don't think lack of atomicity is a big deal as you could order
it so that
you update metadata before doing the hotplug. Then worst case you have a
device mentioned in metadata that doesn't exist, which is easy enough to
detect.
Right, if you want metadata for device, then you'll just update
metadata, hotplug device, and if it failed you update the metadata once
more.
So are we on the same page? By that I mean agreeing on any sane user-supplied
identifier that we'll not guarantee uniqueness for, and neither will we use it
for anything for now? (We can deal with the issues regarding using it when
someone wants to actually implement it).
Thanks,
Martin
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 :|
3:10 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
> > On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
> > > On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
> > > > On Tue, Oct 03, 2017 at 12:58:59PM +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
> > > > > UUID 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'/>
> > > > >
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > > > > <address type='drive' controller='0'
bus='0' target='0' unit='0'/>
> > > > > </disk>
> > > > >
> > > > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> > > > > ---
> > > > >
> > > > > This is just a very basic implementation. If I get a green light
on this, I can
> > > > > implement the feature further, i.e. allow device lookup on the
UUID. For
> > > > > instance:
> > > > >
> > > > > virsh domiftune fedora $UUID $bandwidth
> > >
> > > <snip>
> > >
> > > I'm thinking that part of the problem we're having with agreeing
how to
> > > deal with this RFE is that we're over-analysing semantics, by
wondering
> > > whether its a unique name or UUID, its relation to alias, whether it has
> > > bearing on APIs.
> > >
> > > How about we change tack, and do what we did when we needed application
> > > specific information at the top level - just declare a general purpose
> > > <metadata> element and say it is a completely opaque blob. Libvirt
will
> > > *never* do anything with it, other than to preserve it exactly as is.
> > > No API will ever use the metadata in any way. Libvirt will never try to
> > > guarantee uniqueness of metadata for each device. It can be JSON or a
> > > gziped microsoft word document for all we care. Entirely upto the app
> > > developer to decide what metadata is saved and guarantee uniqueness if
> > > they so desired.
> > >
> >
> > That is kind of what I was aiming for. But in order for it to be cleaner and
> > easier to use from user as well (and not only mgmt apps) I thought the
metadata
> > would just be one identifier. If you want to store more metadata for the
> > device, then you can do all that in the domain metadata and just reference the
> > particular device using the identifier if mgmt app wants to do that.
>
> Yes that is certainly possible. The caveats are that we still need a unique
> identifier for the device, and the metadata update is not atomic wrt
> to device hotplug.
>
Yes, well, our (libvirt) unique identifier is not going anywhere, so
that's OK, we'll be using what we have been until now.
> The plus side of the global metadata is that we have APIs to update it
> on the fly already, and its fully namespaced to allow multiple independant
> data sets to be stored.
>
Yes, exactly.
> I don't think lack of atomicity is a big deal as you could order it so that
> you update metadata before doing the hotplug. Then worst case you have a
> device mentioned in metadata that doesn't exist, which is easy enough to
> detect.
>
Right, if you want metadata for device, then you'll just update
metadata, hotplug device, and if it failed you update the metadata once
more.
So are we on the same page? By that I mean agreeing on any sane user-supplied
identifier that we'll not guarantee uniqueness for, and neither will we use it
for anything for now? (We can deal with the issues regarding using it when
someone wants to actually implement it).
Per my reply to the earlier patch series, I'm now inclined to say that we
should
- Allow the mgmt app to set the aliases upfront
- Auto-fill missing aliases at XML define time
it has some downsides, but all the other solutions we've discussed have
their own downsides too. So on balance I think its not worth it to add
a second identifier for each device, when we already have alias.
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 :|
3:44 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>>> UUID 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'/>
>>>>>>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>>> <address type='drive' controller='0'
bus='0' target='0' unit='0'/>
>>>>>> </disk>
>>>>>>
>>>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>>>>> ---
>>>>>>
>>>>>> This is just a very basic implementation. If I get a green light
on this, I can
>>>>>> implement the feature further, i.e. allow device lookup on the
UUID. For
>>>>>> instance:
>>>>>>
>>>>>> virsh domiftune fedora $UUID $bandwidth
>>>>
>>>> <snip>
>>>>
>>>> I'm thinking that part of the problem we're having with agreeing
how to
>>>> deal with this RFE is that we're over-analysing semantics, by
wondering
>>>> whether its a unique name or UUID, its relation to alias, whether it has
>>>> bearing on APIs.
>>>>
>>>> How about we change tack, and do what we did when we needed application
>>>> specific information at the top level - just declare a general purpose
>>>> <metadata> element and say it is a completely opaque blob. Libvirt
will
>>>> *never* do anything with it, other than to preserve it exactly as is.
>>>> No API will ever use the metadata in any way. Libvirt will never try to
>>>> guarantee uniqueness of metadata for each device. It can be JSON or a
>>>> gziped microsoft word document for all we care. Entirely upto the app
>>>> developer to decide what metadata is saved and guarantee uniqueness if
>>>> they so desired.
>>>>
>>>
>>> That is kind of what I was aiming for. But in order for it to be cleaner
and
>>> easier to use from user as well (and not only mgmt apps) I thought the
metadata
>>> would just be one identifier. If you want to store more metadata for the
>>> device, then you can do all that in the domain metadata and just reference
the
>>> particular device using the identifier if mgmt app wants to do that.
>>
>> Yes that is certainly possible. The caveats are that we still need a unique
>> identifier for the device, and the metadata update is not atomic wrt
>> to device hotplug.
>>
>
> Yes, well, our (libvirt) unique identifier is not going anywhere, so
> that's OK, we'll be using what we have been until now.
>
>> The plus side of the global metadata is that we have APIs to update it
>> on the fly already, and its fully namespaced to allow multiple independant
>> data sets to be stored.
>>
>
> Yes, exactly.
>
>> I don't think lack of atomicity is a big deal as you could order it so that
>> you update metadata before doing the hotplug. Then worst case you have a
>> device mentioned in metadata that doesn't exist, which is easy enough to
>> detect.
>>
>
> Right, if you want metadata for device, then you'll just update
> metadata, hotplug device, and if it failed you update the metadata once
> more.
>
> So are we on the same page? By that I mean agreeing on any sane user-supplied
> identifier that we'll not guarantee uniqueness for, and neither will we use it
> for anything for now? (We can deal with the issues regarding using it when
> someone wants to actually implement it).
Per my reply to the earlier patch series, I'm now inclined to say that we
should
- Allow the mgmt app to set the aliases upfront
- Auto-fill missing aliases at XML define time
it has some downsides, but all the other solutions we've discussed have
their own downsides too. So on balance I think its not worth it to add
a second identifier for each device, when we already have alias.
Question is if we are confident enough that:
a) apps will provide unique aliases (since we'll be putting user input
onto qemu cmd line)
b) apps will provide only allowed characters in the alias (not every
character can be in id=, can it?)
But I think we still have not answered this question: what if we need to
change pattern by which we generate aliases in the future? On one hand,
an alias is just a string so the pattern should not matter. On the other
hand, that's not quite true. For instance, "pci.0" has a very special
meaning. IOW, if we now worry about users cutting off the branch they
are sitting on, this is like giving them flamethrower in fireworks
production hall.
Michal
4:13 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
>> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
>>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
>>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
>>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
>>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>>>> UUID 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'/>
>>>>>>>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>>>> <address type='drive' controller='0'
bus='0' target='0' unit='0'/>
>>>>>>> </disk>
>>>>>>>
>>>>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>>>>>> ---
>>>>>>>
>>>>>>> This is just a very basic implementation. If I get a green
light on this, I can
>>>>>>> implement the feature further, i.e. allow device lookup on
the UUID. For
>>>>>>> instance:
>>>>>>>
>>>>>>> virsh domiftune fedora $UUID $bandwidth
>>>>>
>>>>> <snip>
>>>>>
>>>>> I'm thinking that part of the problem we're having with
agreeing how to
>>>>> deal with this RFE is that we're over-analysing semantics, by
wondering
>>>>> whether its a unique name or UUID, its relation to alias, whether it
has
>>>>> bearing on APIs.
>>>>>
>>>>> How about we change tack, and do what we did when we needed
application
>>>>> specific information at the top level - just declare a general
purpose
>>>>> <metadata> element and say it is a completely opaque blob.
Libvirt will
>>>>> *never* do anything with it, other than to preserve it exactly as
is.
>>>>> No API will ever use the metadata in any way. Libvirt will never try
to
>>>>> guarantee uniqueness of metadata for each device. It can be JSON or
a
>>>>> gziped microsoft word document for all we care. Entirely upto the
app
>>>>> developer to decide what metadata is saved and guarantee uniqueness
if
>>>>> they so desired.
>>>>>
>>>>
>>>> That is kind of what I was aiming for. But in order for it to be
cleaner and
>>>> easier to use from user as well (and not only mgmt apps) I thought the
metadata
>>>> would just be one identifier. If you want to store more metadata for
the
>>>> device, then you can do all that in the domain metadata and just
reference the
>>>> particular device using the identifier if mgmt app wants to do that.
>>>
>>> Yes that is certainly possible. The caveats are that we still need a unique
>>> identifier for the device, and the metadata update is not atomic wrt
>>> to device hotplug.
>>>
>>
>> Yes, well, our (libvirt) unique identifier is not going anywhere, so
>> that's OK, we'll be using what we have been until now.
>>
>>> The plus side of the global metadata is that we have APIs to update it
>>> on the fly already, and its fully namespaced to allow multiple independant
>>> data sets to be stored.
>>>
>>
>> Yes, exactly.
>>
>>> I don't think lack of atomicity is a big deal as you could order it so
that
>>> you update metadata before doing the hotplug. Then worst case you have a
>>> device mentioned in metadata that doesn't exist, which is easy enough
to
>>> detect.
>>>
>>
>> Right, if you want metadata for device, then you'll just update
>> metadata, hotplug device, and if it failed you update the metadata once
>> more.
>>
>> So are we on the same page? By that I mean agreeing on any sane user-supplied
>> identifier that we'll not guarantee uniqueness for, and neither will we use
it
>> for anything for now? (We can deal with the issues regarding using it when
>> someone wants to actually implement it).
>
> Per my reply to the earlier patch series, I'm now inclined to say that we
> should
>
> - Allow the mgmt app to set the aliases upfront
> - Auto-fill missing aliases at XML define time
>
> it has some downsides, but all the other solutions we've discussed have
> their own downsides too. So on balance I think its not worth it to add
> a second identifier for each device, when we already have alias.
Question is if we are confident enough that:
a) apps will provide unique aliases (since we'll be putting user input
onto qemu cmd line)
b) apps will provide only allowed characters in the alias (not every
character can be in id=, can it?)
We will have to validate both these points when looking at the XML.
But I think we still have not answered this question: what if we need
to
change pattern by which we generate aliases in the future? On one hand,
an alias is just a string so the pattern should not matter. On the other
hand, that's not quite true. For instance, "pci.0" has a very special
meaning. IOW, if we now worry about users cutting off the branch they
are sitting on, this is like giving them flamethrower in fireworks
production hall.
'pci.0' is not an alias - 'pci' is the alias, the '0' is a bus
number,
so users only provide the first bit which has no special semantics
other than needing to comply with a permitted set of characters and
be unique.
In terms of validation I think we should permit a-Z, 0-9 and -, upto
a maximum of say 32 characters in length.
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 :|
4:28 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On 10/05/2017 11:13 AM, Daniel P. Berrange wrote:
On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
> On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
>> On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
>>> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
>>>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
>>>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
>>>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander
wrote:
>>>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>>>>> UUID 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'/>
>>>>>>>>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>>>>> <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
>>>>>>>> </disk>
>>>>>>>>
>>>>>>>> Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> This is just a very basic implementation. If I get a
green light on this, I can
>>>>>>>> implement the feature further, i.e. allow device lookup
on the UUID. For
>>>>>>>> instance:
>>>>>>>>
>>>>>>>> virsh domiftune fedora $UUID $bandwidth
>>>>>>
>>>>>> <snip>
>>>>>>
>>>>>> I'm thinking that part of the problem we're having with
agreeing how to
>>>>>> deal with this RFE is that we're over-analysing semantics, by
wondering
>>>>>> whether its a unique name or UUID, its relation to alias, whether
it has
>>>>>> bearing on APIs.
>>>>>>
>>>>>> How about we change tack, and do what we did when we needed
application
>>>>>> specific information at the top level - just declare a general
purpose
>>>>>> <metadata> element and say it is a completely opaque blob.
Libvirt will
>>>>>> *never* do anything with it, other than to preserve it exactly as
is.
>>>>>> No API will ever use the metadata in any way. Libvirt will never
try to
>>>>>> guarantee uniqueness of metadata for each device. It can be JSON
or a
>>>>>> gziped microsoft word document for all we care. Entirely upto the
app
>>>>>> developer to decide what metadata is saved and guarantee
uniqueness if
>>>>>> they so desired.
>>>>>>
>>>>>
>>>>> That is kind of what I was aiming for. But in order for it to be
cleaner and
>>>>> easier to use from user as well (and not only mgmt apps) I thought
the metadata
>>>>> would just be one identifier. If you want to store more metadata for
the
>>>>> device, then you can do all that in the domain metadata and just
reference the
>>>>> particular device using the identifier if mgmt app wants to do that.
>>>>
>>>> Yes that is certainly possible. The caveats are that we still need a
unique
>>>> identifier for the device, and the metadata update is not atomic wrt
>>>> to device hotplug.
>>>>
>>>
>>> Yes, well, our (libvirt) unique identifier is not going anywhere, so
>>> that's OK, we'll be using what we have been until now.
>>>
>>>> The plus side of the global metadata is that we have APIs to update it
>>>> on the fly already, and its fully namespaced to allow multiple
independant
>>>> data sets to be stored.
>>>>
>>>
>>> Yes, exactly.
>>>
>>>> I don't think lack of atomicity is a big deal as you could order it
so that
>>>> you update metadata before doing the hotplug. Then worst case you have a
>>>> device mentioned in metadata that doesn't exist, which is easy enough
to
>>>> detect.
>>>>
>>>
>>> Right, if you want metadata for device, then you'll just update
>>> metadata, hotplug device, and if it failed you update the metadata once
>>> more.
>>>
>>> So are we on the same page? By that I mean agreeing on any sane
user-supplied
>>> identifier that we'll not guarantee uniqueness for, and neither will we
use it
>>> for anything for now? (We can deal with the issues regarding using it when
>>> someone wants to actually implement it).
>>
>> Per my reply to the earlier patch series, I'm now inclined to say that we
>> should
>>
>> - Allow the mgmt app to set the aliases upfront
>> - Auto-fill missing aliases at XML define time
>>
>> it has some downsides, but all the other solutions we've discussed have
>> their own downsides too. So on balance I think its not worth it to add
>> a second identifier for each device, when we already have alias.
>
> Question is if we are confident enough that:
>
> a) apps will provide unique aliases (since we'll be putting user input
> onto qemu cmd line)
>
> b) apps will provide only allowed characters in the alias (not every
> character can be in id=, can it?)
We will have to validate both these points when looking at the XML.
> But I think we still have not answered this question: what if we need to
> change pattern by which we generate aliases in the future? On one hand,
> an alias is just a string so the pattern should not matter. On the other
> hand, that's not quite true. For instance, "pci.0" has a very special
> meaning. IOW, if we now worry about users cutting off the branch they
> are sitting on, this is like giving them flamethrower in fireworks
> production hall.
'pci.0' is not an alias - 'pci' is the alias, the '0' is a bus
number,
so users only provide the first bit which has no special semantics
other than needing to comply with a permitted set of characters and
be unique.
In terms of validation I think we should permit a-Z, 0-9 and -, upto
a maximum of say 32 characters in length.
Okay. We can check that. But now, does it make sense to generate the
aliases at define time? I mean, users could provide alias at define
time, and we can fill in the missing ones when starting up the domain.
Just like we're doing now.
Michal
4:42 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 11:28:35AM +0200, Michal Privoznik wrote:
On 10/05/2017 11:13 AM, Daniel P. Berrange wrote:
> On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
>> On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
>>> On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
>>>> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
>>>>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
>>>>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange
wrote:
>>>>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander
wrote:
>>>>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>>>>>> UUID 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'/>
>>>>>>>>>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>>>>>> <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
>>>>>>>>> </disk>
>>>>>>>>>
>>>>>>>>> Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> This is just a very basic implementation. If I get a
green light on this, I can
>>>>>>>>> implement the feature further, i.e. allow device
lookup on the UUID. For
>>>>>>>>> instance:
>>>>>>>>>
>>>>>>>>> virsh domiftune fedora $UUID $bandwidth
>>>>>>>
>>>>>>> <snip>
>>>>>>>
>>>>>>> I'm thinking that part of the problem we're having
with agreeing how to
>>>>>>> deal with this RFE is that we're over-analysing
semantics, by wondering
>>>>>>> whether its a unique name or UUID, its relation to alias,
whether it has
>>>>>>> bearing on APIs.
>>>>>>>
>>>>>>> How about we change tack, and do what we did when we needed
application
>>>>>>> specific information at the top level - just declare a
general purpose
>>>>>>> <metadata> element and say it is a completely opaque
blob. Libvirt will
>>>>>>> *never* do anything with it, other than to preserve it
exactly as is.
>>>>>>> No API will ever use the metadata in any way. Libvirt will
never try to
>>>>>>> guarantee uniqueness of metadata for each device. It can be
JSON or a
>>>>>>> gziped microsoft word document for all we care. Entirely
upto the app
>>>>>>> developer to decide what metadata is saved and guarantee
uniqueness if
>>>>>>> they so desired.
>>>>>>>
>>>>>>
>>>>>> That is kind of what I was aiming for. But in order for it to
be cleaner and
>>>>>> easier to use from user as well (and not only mgmt apps) I
thought the metadata
>>>>>> would just be one identifier. If you want to store more
metadata for the
>>>>>> device, then you can do all that in the domain metadata and just
reference the
>>>>>> particular device using the identifier if mgmt app wants to do
that.
>>>>>
>>>>> Yes that is certainly possible. The caveats are that we still need a
unique
>>>>> identifier for the device, and the metadata update is not atomic
wrt
>>>>> to device hotplug.
>>>>>
>>>>
>>>> Yes, well, our (libvirt) unique identifier is not going anywhere, so
>>>> that's OK, we'll be using what we have been until now.
>>>>
>>>>> The plus side of the global metadata is that we have APIs to update
it
>>>>> on the fly already, and its fully namespaced to allow multiple
independant
>>>>> data sets to be stored.
>>>>>
>>>>
>>>> Yes, exactly.
>>>>
>>>>> I don't think lack of atomicity is a big deal as you could order
it so that
>>>>> you update metadata before doing the hotplug. Then worst case you
have a
>>>>> device mentioned in metadata that doesn't exist, which is easy
enough to
>>>>> detect.
>>>>>
>>>>
>>>> Right, if you want metadata for device, then you'll just update
>>>> metadata, hotplug device, and if it failed you update the metadata once
>>>> more.
>>>>
>>>> So are we on the same page? By that I mean agreeing on any sane
user-supplied
>>>> identifier that we'll not guarantee uniqueness for, and neither will
we use it
>>>> for anything for now? (We can deal with the issues regarding using it
when
>>>> someone wants to actually implement it).
>>>
>>> Per my reply to the earlier patch series, I'm now inclined to say that
we
>>> should
>>>
>>> - Allow the mgmt app to set the aliases upfront
>>> - Auto-fill missing aliases at XML define time
>>>
>>> it has some downsides, but all the other solutions we've discussed have
>>> their own downsides too. So on balance I think its not worth it to add
>>> a second identifier for each device, when we already have alias.
>>
>> Question is if we are confident enough that:
>>
>> a) apps will provide unique aliases (since we'll be putting user input
>> onto qemu cmd line)
>>
>> b) apps will provide only allowed characters in the alias (not every
>> character can be in id=, can it?)
>
> We will have to validate both these points when looking at the XML.
>
>> But I think we still have not answered this question: what if we need to
>> change pattern by which we generate aliases in the future? On one hand,
>> an alias is just a string so the pattern should not matter. On the other
>> hand, that's not quite true. For instance, "pci.0" has a very
special
>> meaning. IOW, if we now worry about users cutting off the branch they
>> are sitting on, this is like giving them flamethrower in fireworks
>> production hall.
>
> 'pci.0' is not an alias - 'pci' is the alias, the '0' is a
bus number,
> so users only provide the first bit which has no special semantics
> other than needing to comply with a permitted set of characters and
> be unique.
>
> In terms of validation I think we should permit a-Z, 0-9 and -, upto
> a maximum of say 32 characters in length.
Okay. We can check that. But now, does it make sense to generate the
aliases at define time? I mean, users could provide alias at define
time, and we can fill in the missing ones when starting up the domain.
Just like we're doing now.
I think its beneficial to set them at define time, as it provides a unique
identifier that apps / admins can see throughout the lifetime of the guest,
avoiding the need to both about setting them manually to a significant
extent.
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 :|
4:27 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
>> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
>>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
>>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange wrote:
>>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
>>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>>>> UUID 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'/>
>>>>>>>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>>>> <address type='drive' controller='0'
bus='0' target='0' unit='0'/>
>>>>>>> </disk>
>>>>>>>
>>>>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>>>>>> ---
>>>>>>>
>>>>>>> This is just a very basic implementation. If I get a green
light on this, I can
>>>>>>> implement the feature further, i.e. allow device lookup on
the UUID. For
>>>>>>> instance:
>>>>>>>
>>>>>>> virsh domiftune fedora $UUID $bandwidth
>>>>>
>>>>> <snip>
>>>>>
>>>>> I'm thinking that part of the problem we're having with
agreeing how to
>>>>> deal with this RFE is that we're over-analysing semantics, by
wondering
>>>>> whether its a unique name or UUID, its relation to alias, whether it
has
>>>>> bearing on APIs.
>>>>>
>>>>> How about we change tack, and do what we did when we needed
application
>>>>> specific information at the top level - just declare a general
purpose
>>>>> <metadata> element and say it is a completely opaque blob.
Libvirt will
>>>>> *never* do anything with it, other than to preserve it exactly as
is.
>>>>> No API will ever use the metadata in any way. Libvirt will never try
to
>>>>> guarantee uniqueness of metadata for each device. It can be JSON or
a
>>>>> gziped microsoft word document for all we care. Entirely upto the
app
>>>>> developer to decide what metadata is saved and guarantee uniqueness
if
>>>>> they so desired.
>>>>>
>>>>
>>>> That is kind of what I was aiming for. But in order for it to be cleaner
and
>>>> easier to use from user as well (and not only mgmt apps) I thought the
metadata
>>>> would just be one identifier. If you want to store more metadata for
the
>>>> device, then you can do all that in the domain metadata and just
reference the
>>>> particular device using the identifier if mgmt app wants to do that.
>>>
>>> Yes that is certainly possible. The caveats are that we still need a unique
>>> identifier for the device, and the metadata update is not atomic wrt
>>> to device hotplug.
>>>
>>
>> Yes, well, our (libvirt) unique identifier is not going anywhere, so
>> that's OK, we'll be using what we have been until now.
>>
>>> The plus side of the global metadata is that we have APIs to update it
>>> on the fly already, and its fully namespaced to allow multiple independant
>>> data sets to be stored.
>>>
>>
>> Yes, exactly.
>>
>>> I don't think lack of atomicity is a big deal as you could order it so
that
>>> you update metadata before doing the hotplug. Then worst case you have a
>>> device mentioned in metadata that doesn't exist, which is easy enough to
>>> detect.
>>>
>>
>> Right, if you want metadata for device, then you'll just update
>> metadata, hotplug device, and if it failed you update the metadata once
>> more.
>>
>> So are we on the same page? By that I mean agreeing on any sane user-supplied
>> identifier that we'll not guarantee uniqueness for, and neither will we use
it
>> for anything for now? (We can deal with the issues regarding using it when
>> someone wants to actually implement it).
>
> Per my reply to the earlier patch series, I'm now inclined to say that we
> should
>
> - Allow the mgmt app to set the aliases upfront
> - Auto-fill missing aliases at XML define time
>
> it has some downsides, but all the other solutions we've discussed have
> their own downsides too. So on balance I think its not worth it to add
> a second identifier for each device, when we already have alias.
They are not the same thing. Our alias is an ID we pass to QEMU and we
are dependent on it in multiple ways, plus we generate it with some of
our rules in mind. Allowing users to modify this would add so many
possible problems that I would not consider next two releases stable. I
would also not be confident enough to tell someone "just use the alias"
because I see all the places where it's not treated as just another
configurable parameter. And there's no problem with that. But there
will be if we allow users to set an identifier that's heavily used for
internal purposes.
On the other hand it has benefits too. The same alias works throughout
the stack. You can use your strings where you could use aliases, etc.
So what if, just to ease the inclusion of this feature, we force very
strict rules on user-supplied aliases and maybe give them a prefix.
Let's say if you want to supply the alias yourself, it has to be in form
of:
"ua-[a-zA-Z0-9]*"
otherwise libvirt will generate the alias itself and the only thing we
guarantee is that our aliases will never start with 'ua-'?
Question is if we are confident enough that:
a) apps will provide unique aliases (since we'll be putting user input
onto qemu cmd line)
b) apps will provide only allowed characters in the alias (not every
character can be in id=, can it?)
But I think we still have not answered this question: what if we need to
change pattern by which we generate aliases in the future? On one hand,
an alias is just a string so the pattern should not matter. On the other
hand, that's not quite true. For instance, "pci.0" has a very special
meaning. IOW, if we now worry about users cutting off the branch they
are sitting on, this is like giving them flamethrower in fireworks
production hall.
Michal
4:40 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 11:27:29AM +0200, Martin Kletzander wrote:
On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
> On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> > On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
> > > On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
> > > > On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander wrote:
> > > > > On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange
wrote:
> > > > > > On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander
wrote:
> > > > > > > On Tue, Oct 03, 2017 at 12:58:59PM +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
> > > > > > > > UUID 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'/>
> > > > > > > >
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > > > > > > > <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
> > > > > > > > </disk>
> > > > > > > >
> > > > > > > > Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
> > > > > > > > ---
> > > > > > > >
> > > > > > > > This is just a very basic implementation. If I
get a green light on this, I can
> > > > > > > > implement the feature further, i.e. allow device
lookup on the UUID. For
> > > > > > > > instance:
> > > > > > > >
> > > > > > > > virsh domiftune fedora $UUID $bandwidth
> > > > > >
> > > > > > <snip>
> > > > > >
> > > > > > I'm thinking that part of the problem we're having
with agreeing how to
> > > > > > deal with this RFE is that we're over-analysing
semantics, by wondering
> > > > > > whether its a unique name or UUID, its relation to alias,
whether it has
> > > > > > bearing on APIs.
> > > > > >
> > > > > > How about we change tack, and do what we did when we needed
application
> > > > > > specific information at the top level - just declare a
general purpose
> > > > > > <metadata> element and say it is a completely opaque
blob. Libvirt will
> > > > > > *never* do anything with it, other than to preserve it
exactly as is.
> > > > > > No API will ever use the metadata in any way. Libvirt will
never try to
> > > > > > guarantee uniqueness of metadata for each device. It can be
JSON or a
> > > > > > gziped microsoft word document for all we care. Entirely
upto the app
> > > > > > developer to decide what metadata is saved and guarantee
uniqueness if
> > > > > > they so desired.
> > > > > >
> > > > >
> > > > > That is kind of what I was aiming for. But in order for it to
be cleaner and
> > > > > easier to use from user as well (and not only mgmt apps) I
thought the metadata
> > > > > would just be one identifier. If you want to store more
metadata for the
> > > > > device, then you can do all that in the domain metadata and just
reference the
> > > > > particular device using the identifier if mgmt app wants to do
that.
> > > >
> > > > Yes that is certainly possible. The caveats are that we still need a
unique
> > > > identifier for the device, and the metadata update is not atomic wrt
> > > > to device hotplug.
> > > >
> > >
> > > Yes, well, our (libvirt) unique identifier is not going anywhere, so
> > > that's OK, we'll be using what we have been until now.
> > >
> > > > The plus side of the global metadata is that we have APIs to update
it
> > > > on the fly already, and its fully namespaced to allow multiple
independant
> > > > data sets to be stored.
> > > >
> > >
> > > Yes, exactly.
> > >
> > > > I don't think lack of atomicity is a big deal as you could order
it so that
> > > > you update metadata before doing the hotplug. Then worst case you
have a
> > > > device mentioned in metadata that doesn't exist, which is easy
enough to
> > > > detect.
> > > >
> > >
> > > Right, if you want metadata for device, then you'll just update
> > > metadata, hotplug device, and if it failed you update the metadata once
> > > more.
> > >
> > > So are we on the same page? By that I mean agreeing on any sane
user-supplied
> > > identifier that we'll not guarantee uniqueness for, and neither will
we use it
> > > for anything for now? (We can deal with the issues regarding using it
when
> > > someone wants to actually implement it).
> >
> > Per my reply to the earlier patch series, I'm now inclined to say that we
> > should
> >
> > - Allow the mgmt app to set the aliases upfront
> > - Auto-fill missing aliases at XML define time
> >
> > it has some downsides, but all the other solutions we've discussed have
> > their own downsides too. So on balance I think its not worth it to add
> > a second identifier for each device, when we already have alias.
They are not the same thing. Our alias is an ID we pass to QEMU and we
are dependent on it in multiple ways, plus we generate it with some of
our rules in mind. Allowing users to modify this would add so many
possible problems that I would not consider next two releases stable. I
would also not be confident enough to tell someone "just use the alias"
because I see all the places where it's not treated as just another
configurable parameter. And there's no problem with that. But there
will be if we allow users to set an identifier that's heavily used for
internal purposes.
I honestly don't see where the major instability is going to come from ? We
don't have code that cares about the format of the string used in the alias.
What matters is what we keep track of the aliases for any running guests, and
we do that via the state XML, so if libvirt changes the format of aliases it
generates internally it would not break upgrades.
So provided we validate uniqueness & restrict charactersets / length, I
don't see what users would do that would cause the instability.
So what if, just to ease the inclusion of this feature, we force
very
strict rules on user-supplied aliases and maybe give them a prefix.
Let's say if you want to supply the alias yourself, it has to be in form
of:
"ua-[a-zA-Z0-9]*"
otherwise libvirt will generate the alias itself and the only thing we
guarantee is that our aliases will never start with 'ua-'?
Yes, having a standard prefix that mgmt apps use would be a reasonable
idea - it would protect them from possible future clashes with IDs that
libvirt uses for devices that aren't exposed in the XML.
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 :|
6:01 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 10:40:01AM +0100, Daniel P. Berrange wrote:
On Thu, Oct 05, 2017 at 11:27:29AM +0200, Martin Kletzander wrote:
> On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
> > On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> > > On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
> > > > On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
> > > > > On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander
wrote:
> > > > > > On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P.
Berrange wrote:
> > > > > > > On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin
Kletzander wrote:
> > > > > > > > On Tue, Oct 03, 2017 at 12:58:59PM +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
> > > > > > > > > UUID 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'/>
> > > > > > > > >
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > > > > > > > > <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
> > > > > > > > > </disk>
> > > > > > > > >
> > > > > > > > > Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > This is just a very basic implementation. If
I get a green light on this, I can
> > > > > > > > > implement the feature further, i.e. allow
device lookup on the UUID. For
> > > > > > > > > instance:
> > > > > > > > >
> > > > > > > > > virsh domiftune fedora $UUID $bandwidth
> > > > > > >
> > > > > > > <snip>
> > > > > > >
> > > > > > > I'm thinking that part of the problem we're
having with agreeing how to
> > > > > > > deal with this RFE is that we're over-analysing
semantics, by wondering
> > > > > > > whether its a unique name or UUID, its relation to
alias, whether it has
> > > > > > > bearing on APIs.
> > > > > > >
> > > > > > > How about we change tack, and do what we did when we
needed application
> > > > > > > specific information at the top level - just declare a
general purpose
> > > > > > > <metadata> element and say it is a completely
opaque blob. Libvirt will
> > > > > > > *never* do anything with it, other than to preserve it
exactly as is.
> > > > > > > No API will ever use the metadata in any way. Libvirt
will never try to
> > > > > > > guarantee uniqueness of metadata for each device. It
can be JSON or a
> > > > > > > gziped microsoft word document for all we care.
Entirely upto the app
> > > > > > > developer to decide what metadata is saved and
guarantee uniqueness if
> > > > > > > they so desired.
> > > > > > >
> > > > > >
> > > > > > That is kind of what I was aiming for. But in order for it
to be cleaner and
> > > > > > easier to use from user as well (and not only mgmt apps) I
thought the metadata
> > > > > > would just be one identifier. If you want to store more
metadata for the
> > > > > > device, then you can do all that in the domain metadata and
just reference the
> > > > > > particular device using the identifier if mgmt app wants to
do that.
> > > > >
> > > > > Yes that is certainly possible. The caveats are that we still
need a unique
> > > > > identifier for the device, and the metadata update is not atomic
wrt
> > > > > to device hotplug.
> > > > >
> > > >
> > > > Yes, well, our (libvirt) unique identifier is not going anywhere, so
> > > > that's OK, we'll be using what we have been until now.
> > > >
> > > > > The plus side of the global metadata is that we have APIs to
update it
> > > > > on the fly already, and its fully namespaced to allow multiple
independant
> > > > > data sets to be stored.
> > > > >
> > > >
> > > > Yes, exactly.
> > > >
> > > > > I don't think lack of atomicity is a big deal as you could
order it so that
> > > > > you update metadata before doing the hotplug. Then worst case
you have a
> > > > > device mentioned in metadata that doesn't exist, which is
easy enough to
> > > > > detect.
> > > > >
> > > >
> > > > Right, if you want metadata for device, then you'll just update
> > > > metadata, hotplug device, and if it failed you update the metadata
once
> > > > more.
> > > >
> > > > So are we on the same page? By that I mean agreeing on any sane
user-supplied
> > > > identifier that we'll not guarantee uniqueness for, and neither
will we use it
> > > > for anything for now? (We can deal with the issues regarding using
it when
> > > > someone wants to actually implement it).
> > >
> > > Per my reply to the earlier patch series, I'm now inclined to say that
we
> > > should
> > >
> > > - Allow the mgmt app to set the aliases upfront
> > > - Auto-fill missing aliases at XML define time
> > >
> > > it has some downsides, but all the other solutions we've discussed
have
> > > their own downsides too. So on balance I think its not worth it to add
> > > a second identifier for each device, when we already have alias.
>
> They are not the same thing. Our alias is an ID we pass to QEMU and we
> are dependent on it in multiple ways, plus we generate it with some of
> our rules in mind. Allowing users to modify this would add so many
> possible problems that I would not consider next two releases stable. I
> would also not be confident enough to tell someone "just use the alias"
> because I see all the places where it's not treated as just another
> configurable parameter. And there's no problem with that. But there
> will be if we allow users to set an identifier that's heavily used for
> internal purposes.
I honestly don't see where the major instability is going to come from ? We
don't have code that cares about the format of the string used in the alias.
What matters is what we keep track of the aliases for any running guests, and
we do that via the state XML, so if libvirt changes the format of aliases it
generates internally it would not break upgrades.
Actually we have some code that depends on the format of aliases.
Yes, it can be easily fixed but we may miss something.
In src/qemu/qemu_alias.c there is a lot of function that expect certain
format of aliases of existing devices in order to generate a new alias
for remaining devices and if they fail recognize the format they
will report an error instead of assigning new alias.
Another example what could go wrong is that in src/lxc/lxc_process.c
while starting new LXC process we simple generate new alias for consoles
regardless whether there was already one generated or not.
Pavel
So provided we validate uniqueness & restrict charactersets /
length, I
don't see what users would do that would cause the instability.
> So what if, just to ease the inclusion of this feature, we force very
> strict rules on user-supplied aliases and maybe give them a prefix.
> Let's say if you want to supply the alias yourself, it has to be in form
> of:
>
> "ua-[a-zA-Z0-9]*"
>
> otherwise libvirt will generate the alias itself and the only thing we
> guarantee is that our aliases will never start with 'ua-'?
Yes, having a standard prefix that mgmt apps use would be a reasonable
idea - it would protect them from possible future clashes with IDs that
libvirt uses for devices that aren't exposed in the XML.
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
6:31 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On 10/05/2017 01:01 PM, Pavel Hrdina wrote:
On Thu, Oct 05, 2017 at 10:40:01AM +0100, Daniel P. Berrange wrote:
> On Thu, Oct 05, 2017 at 11:27:29AM +0200, Martin Kletzander wrote:
>> On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
>>> On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
>>>> On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
>>>>> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
>>>>>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander
wrote:
>>>>>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P. Berrange
wrote:
>>>>>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin
Kletzander wrote:
>>>>>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>>>>>>> UUID 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'/>
>>>>>>>>>>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>>>>>>> <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
>>>>>>>>>> </disk>
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
>>>>>>>>>> ---
>>>>>>>>>>
>>>>>>>>>> This is just a very basic implementation. If I
get a green light on this, I can
>>>>>>>>>> implement the feature further, i.e. allow device
lookup on the UUID. For
>>>>>>>>>> instance:
>>>>>>>>>>
>>>>>>>>>> virsh domiftune fedora $UUID $bandwidth
>>>>>>>>
>>>>>>>> <snip>
>>>>>>>>
>>>>>>>> I'm thinking that part of the problem we're
having with agreeing how to
>>>>>>>> deal with this RFE is that we're over-analysing
semantics, by wondering
>>>>>>>> whether its a unique name or UUID, its relation to alias,
whether it has
>>>>>>>> bearing on APIs.
>>>>>>>>
>>>>>>>> How about we change tack, and do what we did when we
needed application
>>>>>>>> specific information at the top level - just declare a
general purpose
>>>>>>>> <metadata> element and say it is a completely
opaque blob. Libvirt will
>>>>>>>> *never* do anything with it, other than to preserve it
exactly as is.
>>>>>>>> No API will ever use the metadata in any way. Libvirt
will never try to
>>>>>>>> guarantee uniqueness of metadata for each device. It can
be JSON or a
>>>>>>>> gziped microsoft word document for all we care. Entirely
upto the app
>>>>>>>> developer to decide what metadata is saved and guarantee
uniqueness if
>>>>>>>> they so desired.
>>>>>>>>
>>>>>>>
>>>>>>> That is kind of what I was aiming for. But in order for it
to be cleaner and
>>>>>>> easier to use from user as well (and not only mgmt apps) I
thought the metadata
>>>>>>> would just be one identifier. If you want to store more
metadata for the
>>>>>>> device, then you can do all that in the domain metadata and
just reference the
>>>>>>> particular device using the identifier if mgmt app wants to
do that.
>>>>>>
>>>>>> Yes that is certainly possible. The caveats are that we still
need a unique
>>>>>> identifier for the device, and the metadata update is not atomic
wrt
>>>>>> to device hotplug.
>>>>>>
>>>>>
>>>>> Yes, well, our (libvirt) unique identifier is not going anywhere, so
>>>>> that's OK, we'll be using what we have been until now.
>>>>>
>>>>>> The plus side of the global metadata is that we have APIs to
update it
>>>>>> on the fly already, and its fully namespaced to allow multiple
independant
>>>>>> data sets to be stored.
>>>>>>
>>>>>
>>>>> Yes, exactly.
>>>>>
>>>>>> I don't think lack of atomicity is a big deal as you could
order it so that
>>>>>> you update metadata before doing the hotplug. Then worst case you
have a
>>>>>> device mentioned in metadata that doesn't exist, which is
easy enough to
>>>>>> detect.
>>>>>>
>>>>>
>>>>> Right, if you want metadata for device, then you'll just update
>>>>> metadata, hotplug device, and if it failed you update the metadata
once
>>>>> more.
>>>>>
>>>>> So are we on the same page? By that I mean agreeing on any sane
user-supplied
>>>>> identifier that we'll not guarantee uniqueness for, and neither
will we use it
>>>>> for anything for now? (We can deal with the issues regarding using
it when
>>>>> someone wants to actually implement it).
>>>>
>>>> Per my reply to the earlier patch series, I'm now inclined to say
that we
>>>> should
>>>>
>>>> - Allow the mgmt app to set the aliases upfront
>>>> - Auto-fill missing aliases at XML define time
>>>>
>>>> it has some downsides, but all the other solutions we've discussed
have
>>>> their own downsides too. So on balance I think its not worth it to add
>>>> a second identifier for each device, when we already have alias.
>>
>> They are not the same thing. Our alias is an ID we pass to QEMU and we
>> are dependent on it in multiple ways, plus we generate it with some of
>> our rules in mind. Allowing users to modify this would add so many
>> possible problems that I would not consider next two releases stable. I
>> would also not be confident enough to tell someone "just use the
alias"
>> because I see all the places where it's not treated as just another
>> configurable parameter. And there's no problem with that. But there
>> will be if we allow users to set an identifier that's heavily used for
>> internal purposes.
>
> I honestly don't see where the major instability is going to come from ? We
> don't have code that cares about the format of the string used in the alias.
> What matters is what we keep track of the aliases for any running guests, and
> we do that via the state XML, so if libvirt changes the format of aliases it
> generates internally it would not break upgrades.
Actually we have some code that depends on the format of aliases.
Yes, it can be easily fixed but we may miss something.
In src/qemu/qemu_alias.c there is a lot of function that expect certain
format of aliases of existing devices in order to generate a new alias
for remaining devices and if they fail recognize the format they
will report an error instead of assigning new alias.
Another example what could go wrong is that in src/lxc/lxc_process.c
while starting new LXC process we simple generate new alias for consoles
regardless whether there was already one generated or not.
Worse. Imagine an user who gives a disk alias "vnet0". Now, when we
would generate the aliases for the rest of devices, say interfaces, we
would have to check all other devices and see if the alias we generated
is not already in use. Possibly by any other device.
Although, if we have the prefixed aliases, such clash would not be possible.
Michal
7:41 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 01:31:02PM +0200, Michal Privoznik wrote:
On 10/05/2017 01:01 PM, Pavel Hrdina wrote:
> On Thu, Oct 05, 2017 at 10:40:01AM +0100, Daniel P. Berrange wrote:
>> On Thu, Oct 05, 2017 at 11:27:29AM +0200, Martin Kletzander wrote:
>>> On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
>>>> On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
>>>>> On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
>>>>>> On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange
wrote:
>>>>>>> On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander
wrote:
>>>>>>>> On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P.
Berrange wrote:
>>>>>>>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin
Kletzander wrote:
>>>>>>>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>>>>>>>> UUID 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'/>
>>>>>>>>>>>
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>>>>>>>> <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
>>>>>>>>>>> </disk>
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>> This is just a very basic implementation. If
I get a green light on this, I can
>>>>>>>>>>> implement the feature further, i.e. allow
device lookup on the UUID. For
>>>>>>>>>>> instance:
>>>>>>>>>>>
>>>>>>>>>>> virsh domiftune fedora $UUID $bandwidth
>>>>>>>>>
>>>>>>>>> <snip>
>>>>>>>>>
>>>>>>>>> I'm thinking that part of the problem we're
having with agreeing how to
>>>>>>>>> deal with this RFE is that we're over-analysing
semantics, by wondering
>>>>>>>>> whether its a unique name or UUID, its relation to
alias, whether it has
>>>>>>>>> bearing on APIs.
>>>>>>>>>
>>>>>>>>> How about we change tack, and do what we did when we
needed application
>>>>>>>>> specific information at the top level - just declare
a general purpose
>>>>>>>>> <metadata> element and say it is a completely
opaque blob. Libvirt will
>>>>>>>>> *never* do anything with it, other than to preserve
it exactly as is.
>>>>>>>>> No API will ever use the metadata in any way.
Libvirt will never try to
>>>>>>>>> guarantee uniqueness of metadata for each device. It
can be JSON or a
>>>>>>>>> gziped microsoft word document for all we care.
Entirely upto the app
>>>>>>>>> developer to decide what metadata is saved and
guarantee uniqueness if
>>>>>>>>> they so desired.
>>>>>>>>>
>>>>>>>>
>>>>>>>> That is kind of what I was aiming for. But in order for
it to be cleaner and
>>>>>>>> easier to use from user as well (and not only mgmt apps)
I thought the metadata
>>>>>>>> would just be one identifier. If you want to store more
metadata for the
>>>>>>>> device, then you can do all that in the domain metadata
and just reference the
>>>>>>>> particular device using the identifier if mgmt app wants
to do that.
>>>>>>>
>>>>>>> Yes that is certainly possible. The caveats are that we
still need a unique
>>>>>>> identifier for the device, and the metadata update is not
atomic wrt
>>>>>>> to device hotplug.
>>>>>>>
>>>>>>
>>>>>> Yes, well, our (libvirt) unique identifier is not going
anywhere, so
>>>>>> that's OK, we'll be using what we have been until now.
>>>>>>
>>>>>>> The plus side of the global metadata is that we have APIs to
update it
>>>>>>> on the fly already, and its fully namespaced to allow
multiple independant
>>>>>>> data sets to be stored.
>>>>>>>
>>>>>>
>>>>>> Yes, exactly.
>>>>>>
>>>>>>> I don't think lack of atomicity is a big deal as you
could order it so that
>>>>>>> you update metadata before doing the hotplug. Then worst
case you have a
>>>>>>> device mentioned in metadata that doesn't exist, which
is easy enough to
>>>>>>> detect.
>>>>>>>
>>>>>>
>>>>>> Right, if you want metadata for device, then you'll just
update
>>>>>> metadata, hotplug device, and if it failed you update the
metadata once
>>>>>> more.
>>>>>>
>>>>>> So are we on the same page? By that I mean agreeing on any sane
user-supplied
>>>>>> identifier that we'll not guarantee uniqueness for, and
neither will we use it
>>>>>> for anything for now? (We can deal with the issues regarding
using it when
>>>>>> someone wants to actually implement it).
>>>>>
>>>>> Per my reply to the earlier patch series, I'm now inclined to
say that we
>>>>> should
>>>>>
>>>>> - Allow the mgmt app to set the aliases upfront
>>>>> - Auto-fill missing aliases at XML define time
>>>>>
>>>>> it has some downsides, but all the other solutions we've
discussed have
>>>>> their own downsides too. So on balance I think its not worth it to
add
>>>>> a second identifier for each device, when we already have alias.
>>>
>>> They are not the same thing. Our alias is an ID we pass to QEMU and we
>>> are dependent on it in multiple ways, plus we generate it with some of
>>> our rules in mind. Allowing users to modify this would add so many
>>> possible problems that I would not consider next two releases stable. I
>>> would also not be confident enough to tell someone "just use the
alias"
>>> because I see all the places where it's not treated as just another
>>> configurable parameter. And there's no problem with that. But there
>>> will be if we allow users to set an identifier that's heavily used for
>>> internal purposes.
>>
>> I honestly don't see where the major instability is going to come from ? We
>> don't have code that cares about the format of the string used in the
alias.
>> What matters is what we keep track of the aliases for any running guests, and
>> we do that via the state XML, so if libvirt changes the format of aliases it
>> generates internally it would not break upgrades.
>
> Actually we have some code that depends on the format of aliases.
> Yes, it can be easily fixed but we may miss something.
> In src/qemu/qemu_alias.c there is a lot of function that expect certain
> format of aliases of existing devices in order to generate a new alias
> for remaining devices and if they fail recognize the format they
> will report an error instead of assigning new alias.
>
> Another example what could go wrong is that in src/lxc/lxc_process.c
> while starting new LXC process we simple generate new alias for consoles
> regardless whether there was already one generated or not.
Worse. Imagine an user who gives a disk alias "vnet0". Now, when we
would generate the aliases for the rest of devices, say interfaces, we
would have to check all other devices and see if the alias we generated
is not already in use. Possibly by any other device.
This is an easy problem to resolve though - no different to how we deal
with PCI addressing. Just populate a hash of all currently defined aliases
in the XML, then when generating the new alias, just increment the counter
until we find a free entry.
Although, if we have the prefixed aliases, such clash would not be
possible.
Even if mgmt apps need to use a fixed prefix, we still need the global hash
todo uniqueness checking of their aliases.
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 :|
6:26 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 10:40:01 +0100, Daniel Berrange wrote:
On Thu, Oct 05, 2017 at 11:27:29AM +0200, Martin Kletzander wrote:
> On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
> > On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> > > On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
> > > > On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange wrote:
> > > > > On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander
wrote:
> > > > > > On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P.
Berrange wrote:
> > > > > > > On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin
Kletzander wrote:
> > > > > > > > On Tue, Oct 03, 2017 at 12:58:59PM +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
> > > > > > > > > UUID 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'/>
> > > > > > > > >
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > > > > > > > > <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
> > > > > > > > > </disk>
> > > > > > > > >
> > > > > > > > > Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
> > > > > > > > > ---
> > > > > > > > >
> > > > > > > > > This is just a very basic implementation. If
I get a green light on this, I can
> > > > > > > > > implement the feature further, i.e. allow
device lookup on the UUID. For
> > > > > > > > > instance:
> > > > > > > > >
> > > > > > > > > virsh domiftune fedora $UUID $bandwidth
> > > > > > >
> > > > > > > <snip>
> > > > > > >
> > > > > > > I'm thinking that part of the problem we're
having with agreeing how to
> > > > > > > deal with this RFE is that we're over-analysing
semantics, by wondering
> > > > > > > whether its a unique name or UUID, its relation to
alias, whether it has
> > > > > > > bearing on APIs.
> > > > > > >
> > > > > > > How about we change tack, and do what we did when we
needed application
> > > > > > > specific information at the top level - just declare a
general purpose
> > > > > > > <metadata> element and say it is a completely
opaque blob. Libvirt will
> > > > > > > *never* do anything with it, other than to preserve it
exactly as is.
> > > > > > > No API will ever use the metadata in any way. Libvirt
will never try to
> > > > > > > guarantee uniqueness of metadata for each device. It
can be JSON or a
> > > > > > > gziped microsoft word document for all we care.
Entirely upto the app
> > > > > > > developer to decide what metadata is saved and
guarantee uniqueness if
> > > > > > > they so desired.
> > > > > > >
> > > > > >
> > > > > > That is kind of what I was aiming for. But in order for it
to be cleaner and
> > > > > > easier to use from user as well (and not only mgmt apps) I
thought the metadata
> > > > > > would just be one identifier. If you want to store more
metadata for the
> > > > > > device, then you can do all that in the domain metadata and
just reference the
> > > > > > particular device using the identifier if mgmt app wants to
do that.
> > > > >
> > > > > Yes that is certainly possible. The caveats are that we still
need a unique
> > > > > identifier for the device, and the metadata update is not atomic
wrt
> > > > > to device hotplug.
> > > > >
> > > >
> > > > Yes, well, our (libvirt) unique identifier is not going anywhere, so
> > > > that's OK, we'll be using what we have been until now.
> > > >
> > > > > The plus side of the global metadata is that we have APIs to
update it
> > > > > on the fly already, and its fully namespaced to allow multiple
independant
> > > > > data sets to be stored.
> > > > >
> > > >
> > > > Yes, exactly.
> > > >
> > > > > I don't think lack of atomicity is a big deal as you could
order it so that
> > > > > you update metadata before doing the hotplug. Then worst case
you have a
> > > > > device mentioned in metadata that doesn't exist, which is
easy enough to
> > > > > detect.
> > > > >
> > > >
> > > > Right, if you want metadata for device, then you'll just update
> > > > metadata, hotplug device, and if it failed you update the metadata
once
> > > > more.
> > > >
> > > > So are we on the same page? By that I mean agreeing on any sane
user-supplied
> > > > identifier that we'll not guarantee uniqueness for, and neither
will we use it
> > > > for anything for now? (We can deal with the issues regarding using
it when
> > > > someone wants to actually implement it).
> > >
> > > Per my reply to the earlier patch series, I'm now inclined to say that
we
> > > should
> > >
> > > - Allow the mgmt app to set the aliases upfront
> > > - Auto-fill missing aliases at XML define time
> > >
> > > it has some downsides, but all the other solutions we've discussed
have
> > > their own downsides too. So on balance I think its not worth it to add
> > > a second identifier for each device, when we already have alias.
>
> They are not the same thing. Our alias is an ID we pass to QEMU and we
> are dependent on it in multiple ways, plus we generate it with some of
> our rules in mind. Allowing users to modify this would add so many
> possible problems that I would not consider next two releases stable. I
> would also not be confident enough to tell someone "just use the alias"
> because I see all the places where it's not treated as just another
> configurable parameter. And there's no problem with that. But there
> will be if we allow users to set an identifier that's heavily used for
> internal purposes.
I honestly don't see where the major instability is going to come from ? We
don't have code that cares about the format of the string used in the alias.
Note that allowing custom aliases for memory devices (dimms) may burn us
since they need to be treated as guest ABI. The migration stream refers
to the backing memory objects via the alias name so they need to stay
stable across migration.
Currently the alias is generated from the dimm slot number rather than
the sequence, since the dimm slot is validated in the ABI stability
check.
7:43 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Thu, Oct 05, 2017 at 01:26:54PM +0200, Peter Krempa wrote:
On Thu, Oct 05, 2017 at 10:40:01 +0100, Daniel Berrange wrote:
> On Thu, Oct 05, 2017 at 11:27:29AM +0200, Martin Kletzander wrote:
> > On Thu, Oct 05, 2017 at 10:44:29AM +0200, Michal Privoznik wrote:
> > > On 10/05/2017 10:10 AM, Daniel P. Berrange wrote:
> > > > On Wed, Oct 04, 2017 at 08:31:36AM +0200, Martin Kletzander wrote:
> > > > > On Tue, Oct 03, 2017 at 03:10:48PM +0100, Daniel P. Berrange
wrote:
> > > > > > On Tue, Oct 03, 2017 at 04:03:20PM +0200, Martin Kletzander
wrote:
> > > > > > > On Tue, Oct 03, 2017 at 02:53:46PM +0100, Daniel P.
Berrange wrote:
> > > > > > > > On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin
Kletzander wrote:
> > > > > > > > > On Tue, Oct 03, 2017 at 12:58:59PM +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
> > > > > > > > > > UUID 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'/>
> > > > > > > > > >
<uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
> > > > > > > > > > <address type='drive'
controller='0' bus='0' target='0' unit='0'/>
> > > > > > > > > > </disk>
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Michal Privoznik
<mprivozn(a)redhat.com>
> > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > This is just a very basic
implementation. If I get a green light on this, I can
> > > > > > > > > > implement the feature further, i.e.
allow device lookup on the UUID. For
> > > > > > > > > > instance:
> > > > > > > > > >
> > > > > > > > > > virsh domiftune fedora $UUID
$bandwidth
> > > > > > > >
> > > > > > > > <snip>
> > > > > > > >
> > > > > > > > I'm thinking that part of the problem
we're having with agreeing how to
> > > > > > > > deal with this RFE is that we're
over-analysing semantics, by wondering
> > > > > > > > whether its a unique name or UUID, its relation
to alias, whether it has
> > > > > > > > bearing on APIs.
> > > > > > > >
> > > > > > > > How about we change tack, and do what we did when
we needed application
> > > > > > > > specific information at the top level - just
declare a general purpose
> > > > > > > > <metadata> element and say it is a
completely opaque blob. Libvirt will
> > > > > > > > *never* do anything with it, other than to
preserve it exactly as is.
> > > > > > > > No API will ever use the metadata in any way.
Libvirt will never try to
> > > > > > > > guarantee uniqueness of metadata for each device.
It can be JSON or a
> > > > > > > > gziped microsoft word document for all we care.
Entirely upto the app
> > > > > > > > developer to decide what metadata is saved and
guarantee uniqueness if
> > > > > > > > they so desired.
> > > > > > > >
> > > > > > >
> > > > > > > That is kind of what I was aiming for. But in order
for it to be cleaner and
> > > > > > > easier to use from user as well (and not only mgmt
apps) I thought the metadata
> > > > > > > would just be one identifier. If you want to store
more metadata for the
> > > > > > > device, then you can do all that in the domain
metadata and just reference the
> > > > > > > particular device using the identifier if mgmt app
wants to do that.
> > > > > >
> > > > > > Yes that is certainly possible. The caveats are that we
still need a unique
> > > > > > identifier for the device, and the metadata update is not
atomic wrt
> > > > > > to device hotplug.
> > > > > >
> > > > >
> > > > > Yes, well, our (libvirt) unique identifier is not going
anywhere, so
> > > > > that's OK, we'll be using what we have been until now.
> > > > >
> > > > > > The plus side of the global metadata is that we have APIs
to update it
> > > > > > on the fly already, and its fully namespaced to allow
multiple independant
> > > > > > data sets to be stored.
> > > > > >
> > > > >
> > > > > Yes, exactly.
> > > > >
> > > > > > I don't think lack of atomicity is a big deal as you
could order it so that
> > > > > > you update metadata before doing the hotplug. Then worst
case you have a
> > > > > > device mentioned in metadata that doesn't exist, which
is easy enough to
> > > > > > detect.
> > > > > >
> > > > >
> > > > > Right, if you want metadata for device, then you'll just
update
> > > > > metadata, hotplug device, and if it failed you update the
metadata once
> > > > > more.
> > > > >
> > > > > So are we on the same page? By that I mean agreeing on any sane
user-supplied
> > > > > identifier that we'll not guarantee uniqueness for, and
neither will we use it
> > > > > for anything for now? (We can deal with the issues regarding
using it when
> > > > > someone wants to actually implement it).
> > > >
> > > > Per my reply to the earlier patch series, I'm now inclined to say
that we
> > > > should
> > > >
> > > > - Allow the mgmt app to set the aliases upfront
> > > > - Auto-fill missing aliases at XML define time
> > > >
> > > > it has some downsides, but all the other solutions we've
discussed have
> > > > their own downsides too. So on balance I think its not worth it to
add
> > > > a second identifier for each device, when we already have alias.
> >
> > They are not the same thing. Our alias is an ID we pass to QEMU and we
> > are dependent on it in multiple ways, plus we generate it with some of
> > our rules in mind. Allowing users to modify this would add so many
> > possible problems that I would not consider next two releases stable. I
> > would also not be confident enough to tell someone "just use the
alias"
> > because I see all the places where it's not treated as just another
> > configurable parameter. And there's no problem with that. But there
> > will be if we allow users to set an identifier that's heavily used for
> > internal purposes.
>
> I honestly don't see where the major instability is going to come from ? We
> don't have code that cares about the format of the string used in the alias.
Note that allowing custom aliases for memory devices (dimms) may burn us
since they need to be treated as guest ABI. The migration stream refers
to the backing memory objects via the alias name so they need to stay
stable across migration.
Currently the alias is generated from the dimm slot number rather than
the sequence, since the dimm slot is validated in the ABI stability
check.
Are those aliases exposed in the XML ? If so, that should be fine since
we'll generate the DIMM alias once at define time and never change it
again thereafter. This would actually be safer than our current approach
which relies on us always generating the same aliases at startup 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 :|
4:31 p.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On 10/03/2017 07:53 AM, Daniel P. Berrange wrote:
On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
> On Tue, Oct 03, 2017 at 12:58:59PM +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
>> UUID 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'/>
>> <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>> <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
>> </disk>
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>>
>> This is just a very basic implementation. If I get a green light on this, I can
>> implement the feature further, i.e. allow device lookup on the UUID. For
>> instance:
>>
>> virsh domiftune fedora $UUID $bandwidth
<snip>
I'm thinking that part of the problem we're having with agreeing how to
deal with this RFE is that we're over-analysing semantics, by wondering
whether its a unique name or UUID, its relation to alias, whether it has
bearing on APIs.
How about we change tack, and do what we did when we needed application
specific information at the top level - just declare a general purpose
<metadata> element and say it is a completely opaque blob. Libvirt will
*never* do anything with it, other than to preserve it exactly as is.
No API will ever use the metadata in any way. Libvirt will never try to
guarantee uniqueness of metadata for each device. It can be JSON or a
gziped microsoft word document for all we care. Entirely upto the app
developer to decide what metadata is saved and guarantee uniqueness if
they so desired.
I vote for the opaque blob since it would be helpful for my use case described here
https://www.redhat.com/archives/libvir-list/2017-October/msg01329.html
The <metadata> element could contain the 'some-dev-config' from my example
and
alleviate the need to modify XML. <metadata> at the device level is a bit
cleaner for hot (un)plug IMO. E.g.
virsh attach-device foo << 'EOF'
<disk type='block' device='disk'>
<metadata>some-dev-config</metadata>
<driver name='qemu' type='raw'/>
<source dev='/dev/vg1/lv1'/>
<target dev='vdb' bus='virtio'/>
</disk>
EOF
Regards,
Jim
5:12 p.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On 10/30/2017 03:31 PM, Jim Fehlig wrote:
On 10/03/2017 07:53 AM, Daniel P. Berrange wrote:
> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>> UUID 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'/>
>>> <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>> <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
>>> </disk>
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>> ---
>>>
>>> This is just a very basic implementation. If I get a green light on this, I
can
>>> implement the feature further, i.e. allow device lookup on the UUID. For
>>> instance:
>>>
>>> virsh domiftune fedora $UUID $bandwidth
>
> <snip>
>
> I'm thinking that part of the problem we're having with agreeing how to
> deal with this RFE is that we're over-analysing semantics, by wondering
> whether its a unique name or UUID, its relation to alias, whether it has
> bearing on APIs.
>
> How about we change tack, and do what we did when we needed application
> specific information at the top level - just declare a general purpose
> <metadata> element and say it is a completely opaque blob. Libvirt will
> *never* do anything with it, other than to preserve it exactly as is.
> No API will ever use the metadata in any way. Libvirt will never try to
> guarantee uniqueness of metadata for each device. It can be JSON or a
> gziped microsoft word document for all we care. Entirely upto the app
> developer to decide what metadata is saved and guarantee uniqueness if
> they so desired.
I vote for the opaque blob since it would be helpful for my use case described here
https://www.redhat.com/archives/libvir-list/2017-October/msg01329.html
I should have read further ahead in my mail backlog, where I would have
discovered this has already be solved using <alias>. That's fine, but I'm
curious why the domain of permissible characters is restricted? Could it include
():;/ ?
Regards,
Jim
The <metadata> element could contain the 'some-dev-config' from my example
and
alleviate the need to modify XML. <metadata> at the device level is a bit
cleaner for hot (un)plug IMO. E.g.
virsh attach-device foo << 'EOF'
<disk type='block' device='disk'>
<metadata>some-dev-config</metadata>
<driver name='qemu' type='raw'/>
<source dev='/dev/vg1/lv1'/>
<target dev='vdb' bus='virtio'/>
</disk>
EOF
Regards,
Jim
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
1:04 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On 10/30/2017 11:12 PM, Jim Fehlig wrote:
On 10/30/2017 03:31 PM, Jim Fehlig wrote:
> On 10/03/2017 07:53 AM, Daniel P. Berrange wrote:
>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>> UUID 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'/>
>>>> <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>> <address type='drive' controller='0'
bus='0' target='0'
>>>> unit='0'/>
>>>> </disk>
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>>> ---
>>>>
>>>> This is just a very basic implementation. If I get a green light on
>>>> this, I can
>>>> implement the feature further, i.e. allow device lookup on the
>>>> UUID. For
>>>> instance:
>>>>
>>>> virsh domiftune fedora $UUID $bandwidth
>>
>> <snip>
>>
>> I'm thinking that part of the problem we're having with agreeing how to
>> deal with this RFE is that we're over-analysing semantics, by wondering
>> whether its a unique name or UUID, its relation to alias, whether it has
>> bearing on APIs.
>>
>> How about we change tack, and do what we did when we needed application
>> specific information at the top level - just declare a general purpose
>> <metadata> element and say it is a completely opaque blob. Libvirt will
>> *never* do anything with it, other than to preserve it exactly as is.
>> No API will ever use the metadata in any way. Libvirt will never try to
>> guarantee uniqueness of metadata for each device. It can be JSON or a
>> gziped microsoft word document for all we care. Entirely upto the app
>> developer to decide what metadata is saved and guarantee uniqueness if
>> they so desired.
>
> I vote for the opaque blob since it would be helpful for my use case
> described here
>
> https://www.redhat.com/archives/libvir-list/2017-October/msg01329.html
I should have read further ahead in my mail backlog, where I would have
discovered this has already be solved using <alias>. That's fine, but
I'm curious why the domain of permissible characters is restricted?
Could it include ():;/ ?
Currently, only: a-zA-Z0-9_- is allowed. This is because in qemu driver
aliases are put onto qemu cmd line where only limited set of characters
is allowed. However, the check for that could be moved to drivers so
that each driver can define its own set of allowed characters.
Michal
10:11 a.m.
New subject: [libvirt] [PATCH v2 3/3] conf: Allow users to define UUID for devices
On Tue, Oct 31, 2017 at 07:04:35AM +0100, Michal Privoznik wrote:
On 10/30/2017 11:12 PM, Jim Fehlig wrote:
> On 10/30/2017 03:31 PM, Jim Fehlig wrote:
>> On 10/03/2017 07:53 AM, Daniel P. Berrange wrote:
>>> On Tue, Oct 03, 2017 at 02:11:44PM +0200, Martin Kletzander wrote:
>>>> On Tue, Oct 03, 2017 at 12:58:59PM +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
>>>>> UUID 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'/>
>>>>> <uuid>1efaf08b-9317-4b0f-b227-912e4bd9f483</uuid>
>>>>> <address type='drive' controller='0'
bus='0' target='0'
>>>>> unit='0'/>
>>>>> </disk>
>>>>>
>>>>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>>>>> ---
>>>>>
>>>>> This is just a very basic implementation. If I get a green light on
>>>>> this, I can
>>>>> implement the feature further, i.e. allow device lookup on the
>>>>> UUID. For
>>>>> instance:
>>>>>
>>>>> virsh domiftune fedora $UUID $bandwidth
>>>
>>> <snip>
>>>
>>> I'm thinking that part of the problem we're having with agreeing how
to
>>> deal with this RFE is that we're over-analysing semantics, by wondering
>>> whether its a unique name or UUID, its relation to alias, whether it has
>>> bearing on APIs.
>>>
>>> How about we change tack, and do what we did when we needed application
>>> specific information at the top level - just declare a general purpose
>>> <metadata> element and say it is a completely opaque blob. Libvirt
will
>>> *never* do anything with it, other than to preserve it exactly as is.
>>> No API will ever use the metadata in any way. Libvirt will never try to
>>> guarantee uniqueness of metadata for each device. It can be JSON or a
>>> gziped microsoft word document for all we care. Entirely upto the app
>>> developer to decide what metadata is saved and guarantee uniqueness if
>>> they so desired.
>>
>> I vote for the opaque blob since it would be helpful for my use case
>> described here
>>
>> https://www.redhat.com/archives/libvir-list/2017-October/msg01329.html
>
> I should have read further ahead in my mail backlog, where I would have
> discovered this has already be solved using <alias>. That's fine, but
> I'm curious why the domain of permissible characters is restricted?
> Could it include ():;/ ?
Currently, only: a-zA-Z0-9_- is allowed. This is because in qemu driver
aliases are put onto qemu cmd line where only limited set of characters
is allowed. However, the check for that could be moved to drivers so
that each driver can define its own set of allowed characters.
That would not be particularly nice for an application that wants to set
aliases though. I think we should stick to a strict subset
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 :|
2579
days inactive
2607
days old
27 comments
6 participants
participants (6)
-
Daniel P. Berrange -
Jim Fehlig -
Martin Kletzander -
Michal Privoznik -
Pavel Hrdina -
Peter Krempa