Re: [libvirt] [PATCH v3] Add host UUID (to libvirt capabilities)

On Mon, May 24, 2010 at 02:57:33PM -0400, Stefan Berger wrote: The XML files here shouldn't really have had UUIDs added. This was only needed because the schema marked the UUID field as mandatory. I'm attaching an update of your patch which makes this optional, since many of the libvirt drivers are not filling in the host UUID field with this code yet. This was causing the non-QEMU drivers to include a all-zeroes UUID since they didn't initialize host_uuid. I've exported the isValid function and added a check here I've wrapped this in <optional> I've added this same code to the UML and LXC drivers. We'll need a different solution for things like VMWare/VirtualBox/Phyp/etc, so for now they don't get any host UUID in XML. I don't think my new patch should cause any merge problems with the VEPA patches, since my changes touched different code. Daniel diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 195c50a..8fa78b8 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -57,6 +57,7 @@ #include "dispatch.h" #include "util.h" +#include "uuid.h" #include "remote_driver.h" #include "conf.h" #include "event.h" @@ -2718,6 +2719,7 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) char *unix_sock_rw_perms = NULL; char *unix_sock_group = NULL; char *buf = NULL; + char *host_uuid = NULL; #if HAVE_POLKIT /* Change the default back to no auth for non-root */ @@ -2840,11 +2842,18 @@ remoteReadConfigFile (struct qemud_server *server, const char *filename) GET_CONF_INT (conf, filename, max_requests); GET_CONF_INT (conf, filename, max_client_requests); + GET_CONF_STR (conf, filename, host_uuid); + if (virSetHostUUIDStr(host_uuid)) + goto free_and_fail; + + VIR_FREE(host_uuid); + virConfFree (conf); return 0; free_and_fail: virConfFree (conf); + VIR_FREE(host_uuid); VIR_FREE(mdns_name); VIR_FREE(unix_sock_ro_perms); VIR_FREE(unix_sock_rw_perms); diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf index 49de466..61e4dde 100644 --- a/daemon/libvirtd.conf +++ b/daemon/libvirtd.conf @@ -312,3 +312,13 @@ # e.g.: # log_outputs="3:syslog:libvirtd" # to log all warnings and errors to syslog under the libvirtd ident + +# UUID of the host: +# Provide the UUID of the host here in case the command +# 'dmidecode -s system-uuid' does not provide a valid uuid. In case +# 'dmidecode' does not provide a valid UUID and none is provided here, a +# temporary UUID will be generated. +# Keep the format of the example UUID below. UUID must not have all digits +# be the same. + +#host_uuid = "8510b1a1-1afa-4da6-8111-785fae202c1e" diff --git a/docs/schemas/capability.rng b/docs/schemas/capability.rng index eb3c50a..67e8cf2 100644 --- a/docs/schemas/capability.rng +++ b/docs/schemas/capability.rng @@ -18,6 +18,11 @@ <define name='hostcaps'> <element name='host'> + <optional> + <element name='uuid'> + <ref name='UUID'/> + </element> + </optional> <element name='cpu'> <element name='arch'> <ref name='archnames'/> @@ -349,4 +354,15 @@ <param name='pattern'>[a-zA-Z0-9\-_]+</param> </data> </define> + + <define name="UUID"> + <choice> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{32}</param> + </data> + <data type="string"> + <param name="pattern">[a-fA-F0-9]{8}\-([a-fA-F0-9]{4}\-){3}[a-fA-F0-9]{12}</param> + </data> + </choice> + </define> </grammar> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index dafd821..4478d85 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -27,6 +27,7 @@ #include "buf.h" #include "memory.h" #include "util.h" +#include "uuid.h" #include "cpu_conf.h" /** @@ -662,9 +663,14 @@ virCapabilitiesFormatXML(virCapsPtr caps) { virBuffer xml = VIR_BUFFER_INITIALIZER; int i, j, k; + char host_uuid[VIR_UUID_STRING_BUFLEN]; virBufferAddLit(&xml, "<capabilities>\n\n"); virBufferAddLit(&xml, " <host>\n"); + if (virUUIDIsValid(caps->host.host_uuid)) { + virUUIDFormat(caps->host.host_uuid, host_uuid); + virBufferVSprintf(&xml," <uuid>%s</uuid>\n", host_uuid); + } virBufferAddLit(&xml, " <cpu>\n"); virBufferVSprintf(&xml, " <arch>%s</arch>\n", caps->host.arch); diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 0ed0166..bdf44fa 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -110,6 +110,7 @@ struct _virCapsHost { virCapsHostNUMACellPtr *numaCell; virCapsHostSecModel secModel; virCPUDefPtr cpu; + unsigned char host_uuid[VIR_UUID_BUFLEN]; }; typedef struct _virCaps virCaps; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1e4bfd0..a30ea4f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -708,6 +708,8 @@ usbDeviceFileIterate; virUUIDFormat; virUUIDGenerate; virUUIDParse; +virSetHostUUIDStr; +virGetHostUUID; # virterror_internal.h diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index 8d7f69e..f4479e6 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -33,6 +33,7 @@ #include "conf.h" #include "memory.h" #include "logging.h" +#include "uuid.h" #define VIR_FROM_THIS VIR_FROM_LXC @@ -48,7 +49,7 @@ virCapsPtr lxcCapsInit(void) if ((caps = virCapabilitiesNew(utsname.machine, 0, 0)) == NULL) - goto no_memory; + goto error; /* Some machines have problematic NUMA toplogy causing * unexpected failures. We don't want to break the QEMU @@ -59,6 +60,12 @@ virCapsPtr lxcCapsInit(void) VIR_WARN0("Failed to query host NUMA topology, disabling NUMA capabilities"); } + if (virGetHostUUID(caps->host.host_uuid)) { + lxcError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot get the host uuid")); + goto error; + } + /* XXX shouldn't 'borrow' KVM's prefix */ virCapabilitiesSetMacPrefix(caps, (unsigned char []){ 0x52, 0x54, 0x00 }); @@ -70,7 +77,7 @@ virCapsPtr lxcCapsInit(void) NULL, 0, NULL)) == NULL) - goto no_memory; + goto error; if (virCapabilitiesAddGuestDomain(guest, "lxc", @@ -78,14 +85,14 @@ virCapsPtr lxcCapsInit(void) NULL, 0, NULL) == NULL) - goto no_memory; + goto error; /* LXC Requires an emulator in the XML */ virCapabilitiesSetEmulatorRequired(caps); return caps; -no_memory: +error: virCapabilitiesFree(caps); return NULL; } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index a1ced87..f0485f1 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1473,12 +1473,8 @@ udevGetDMIData(union _virNodeDevCapData *data) goto out; } - if (udevGetStringSysfsAttr(device, - "product_uuid", - &tmp) == PROPERTY_ERROR) { + if (virGetHostUUID(data->system.hardware.uuid)) goto out; - } - virUUIDParse(tmp, data->system.hardware.uuid); if (udevGetStringSysfsAttr(device, "bios_vendor", diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ef0e193..1e9141c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1334,6 +1334,11 @@ qemuCreateCapabilities(virCapsPtr oldcaps, caps->privateDataXMLFormat = qemuDomainObjPrivateXMLFormat; caps->privateDataXMLParse = qemuDomainObjPrivateXMLParse; + if (virGetHostUUID(caps->host.host_uuid)) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot get the host uuid")); + goto err_exit; + } /* Security driver data */ if (driver->securityPrimaryDriver) { @@ -1355,6 +1360,7 @@ qemuCreateCapabilities(virCapsPtr oldcaps, no_memory: virReportOOMError(); +err_exit: virCapabilitiesFree(caps); return NULL; } diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index a10bca0..785d627 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -62,7 +62,7 @@ virCapsPtr umlCapsInit(void) { if ((caps = virCapabilitiesNew(utsname.machine, 0, 0)) == NULL) - goto no_memory; + goto error; /* Some machines have problematic NUMA toplogy causing * unexpected failures. We don't want to break the QEMU @@ -73,6 +73,12 @@ virCapsPtr umlCapsInit(void) { VIR_WARN0("Failed to query host NUMA topology, disabling NUMA capabilities"); } + if (virGetHostUUID(caps->host.host_uuid)) { + umlReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot get the host uuid")); + goto error; + } + if ((guest = virCapabilitiesAddGuest(caps, "uml", utsname.machine, @@ -81,7 +87,7 @@ virCapsPtr umlCapsInit(void) { NULL, 0, NULL)) == NULL) - goto no_memory; + goto error; if (virCapabilitiesAddGuestDomain(guest, "uml", @@ -89,11 +95,11 @@ virCapsPtr umlCapsInit(void) { NULL, 0, NULL) == NULL) - goto no_memory; + goto error; return caps; - no_memory: + error: virCapabilitiesFree(caps); return NULL; } diff --git a/src/util/uuid.c b/src/util/uuid.c index 9c626ce..f188148 100644 --- a/src/util/uuid.c +++ b/src/util/uuid.c @@ -38,11 +38,14 @@ #include "util.h" #include "virterror_internal.h" #include "logging.h" +#include "memory.h" #ifndef ENODATA # define ENODATA EIO #endif +static unsigned char host_uuid[VIR_UUID_BUFLEN]; + static int virUUIDGenerateRandomBytes(unsigned char *buf, int buflen) @@ -208,3 +211,116 @@ void virUUIDFormat(const unsigned char *uuid, char *uuidstr) uuid[12], uuid[13], uuid[14], uuid[15]); uuidstr[VIR_UUID_STRING_BUFLEN-1] = '\0'; } + + + +/** + * virUUIDIsValid + * + * @uuid: The UUID to test + * + * Do some basic tests to check whether the given UUID is + * valid as a host UUID. + * Basic tests: + * - Not all of the digits may be equal + */ +int +virUUIDIsValid(unsigned char *uuid) +{ + unsigned int i, ctr = 1; + unsigned char c; + + if (!uuid) + return 0; + + c = uuid[0]; + + for (i = 1; i < VIR_UUID_BUFLEN; i++) + if (uuid[i] == c) + ctr++; + + return (ctr != VIR_UUID_BUFLEN); +} + +static int +getDMISystemUUID(char *uuid, int len) +{ + unsigned int i = 0; + const char *paths[] = { + "/sys/devices/virtual/dmi/id/product_uuid", + "/sys/class/dmi/id/product_uuid", + NULL + }; + + while (paths[i]) { + int fd = open(paths[i], O_RDONLY); + if (fd > 0) { + if (saferead(fd, uuid, len) == len) { + close(fd); + return 0; + } + close(fd); + } + i++; + } + + return -1; +} + + +/** + * setHostUUID + * + * @host_uuid: UUID that the host is supposed to have + * + * Set the UUID of the host if it hasn't been set, yet + * Returns 0 in case of success, an error code in case of error. + */ +int +virSetHostUUIDStr(const char *uuid) +{ + int rc; + char dmiuuid[VIR_UUID_STRING_BUFLEN]; + + if (virUUIDIsValid(host_uuid)) + return EEXIST; + + if (!uuid) { + if (!getDMISystemUUID(dmiuuid, sizeof(dmiuuid))) { + if (!virUUIDParse(dmiuuid, host_uuid)) + return 0; + } + + if (!virUUIDIsValid(host_uuid)) + return virUUIDGenerate(host_uuid); + } else { + rc = virUUIDParse(uuid, host_uuid); + if (rc) + return rc; + if (!virUUIDIsValid(host_uuid)) + return EINVAL; + } + + return 0; +} + +/** + * getHostUUID: + * + * @host_uuid: memory to store the host_uuid into + * + * Get the UUID of the host. Returns 0 in case of success, + * an error code otherwise. + * Returns 0 in case of success, an error code in case of error. + */ +int virGetHostUUID(unsigned char *uuid) +{ + int ret = 0; + + if (!virUUIDIsValid(host_uuid)) + ret = virSetHostUUIDStr(NULL); + + memcpy(uuid, host_uuid, sizeof(host_uuid)); + + return ret; +} diff --git a/src/util/uuid.h b/src/util/uuid.h index b6ac372..36abcfc 100644 --- a/src/util/uuid.h +++ b/src/util/uuid.h @@ -22,6 +22,11 @@ #ifndef __VIR_UUID_H__ # define __VIR_UUID_H__ +int virSetHostUUIDStr(const char *host_uuid); +int virGetHostUUID(unsigned char *host_uuid); + +int virUUIDIsValid(unsigned char *uuid); + int virUUIDGenerate(unsigned char *uuid); int virUUIDParse(const char *uuidstr, diff --git a/tests/confdata/libvirtd.conf b/tests/confdata/libvirtd.conf index 1fc875a..a943bfa 100644 --- a/tests/confdata/libvirtd.conf +++ b/tests/confdata/libvirtd.conf @@ -218,3 +218,12 @@ tls_allowed_dn_list = ["DN1", "DN2"] # # By default, no Username's are checked sasl_allowed_username_list = [&quot;joe(a)EXAMPLE.COM&quot;, &quot;fred(a)EXAMPLE.COM&quot; ] + +# UUID of the host: +# Provide the UUID of the host here in case the command +# 'dmidecode -s system-uuid' does not provide a valid uuid. In case +# 'dmidecode' does not provide a valid UUID and none is provided here, a +# temporary UUID will be generated. +# Keep the format of the example UUID below. + +host_uuid = "8510b1a1-1afa-4da6-8111-785fae202c1e" diff --git a/tests/confdata/libvirtd.out b/tests/confdata/libvirtd.out index 685744a..0bebe2f 100644 --- a/tests/confdata/libvirtd.out +++ b/tests/confdata/libvirtd.out @@ -178,3 +178,10 @@ tls_allowed_dn_list = [ "DN1", "DN2" ] # # By default, no Username's are checked sasl_allowed_username_list = [ &quot;joe(a)EXAMPLE.COM&quot;, &quot;fred(a)EXAMPLE.COM&quot; ] +# UUID of the host: +# Provide the UUID of the host here in case the command +# 'dmidecode -s system-uuid' does not provide a valid uuid. In case +# 'dmidecode' does not provide a valid UUID and none is provided here, a +# temporary UUID will be generated. +# Keep the format of the example UUID below. +host_uuid = "8510b1a1-1afa-4da6-8111-785fae202c1e" -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|