Re: [libvirt] [PATCHv2 4/5] uuid: require smbios uuid and domain uuid to match

2010-12-05 Thread Daniel Veillard
On Fri, Dec 03, 2010 at 02:56:17PM -0700, Eric Blake wrote:
 * src/conf/domain_conf.c (virDomainDefParseXML): Prefer sysinfo
 uuid over generating one, and if both uuids are present, require
 them to be identical.
 * src/qemu/qemu_conf.c (qemuBuildSmbiosSystemStr): Allow skipping
 the uuid.
 (qemudBuildCommandLine): Adjust caller; smbios mode=host/ must
 not use host uuid in place of guest uuid.
 ---
 
 The qemu developers pointed out that '-uuid xyz' and '-smbios
 type=1,uuid=xyz' both have the same effect.  They argued that -uuid is
 deprecated, but it's easier for libvirt to always supply it.
 Meanwhile, they were clear that if both values were supplied, the two
 uuids had better be identical (current qemu favors -uuid over -smbios,
 but it's not guaranteed to stay that way in the future).

  in practice I ended up at least on one test where -uuid and 
smbios mode=host/ failed with the code in qemu complaining that
the UUID was not well formed, so yes that's a good thing !

ACK

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

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


[libvirt] [PATCHv2 4/5] uuid: require smbios uuid and domain uuid to match

2010-12-03 Thread Eric Blake
* src/conf/domain_conf.c (virDomainDefParseXML): Prefer sysinfo
uuid over generating one, and if both uuids are present, require
them to be identical.
* src/qemu/qemu_conf.c (qemuBuildSmbiosSystemStr): Allow skipping
the uuid.
(qemudBuildCommandLine): Adjust caller; smbios mode=host/ must
not use host uuid in place of guest uuid.
---

The qemu developers pointed out that '-uuid xyz' and '-smbios
type=1,uuid=xyz' both have the same effect.  They argued that -uuid is
deprecated, but it's easier for libvirt to always supply it.
Meanwhile, they were clear that if both values were supplied, the two
uuids had better be identical (current qemu favors -uuid over -smbios,
but it's not guaranteed to stay that way in the future).

 src/conf/domain_conf.c |   22 +-
 src/qemu/qemu_conf.c   |   18 +++---
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3dfd4ee..9c54a59 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4498,6 +4498,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 long id = -1;
 virDomainDefPtr def;
 unsigned long count;
+bool uuid_generated = false;

 if (VIR_ALLOC(def)  0) {
 virReportOOMError();
@@ -4529,7 +4530,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
 goto error;
 }

-/* Extract domain uuid */
+/* Extract domain uuid. If both uuid and sysinfo/system/entry/uuid
+ * exist, they must match; and if only the latter exists, it can
+ * also serve as the uuid. */
 tmp = virXPathString(string(./uuid[1]), ctxt);
 if (!tmp) {
 if (virUUIDGenerate(def-uuid)) {
@@ -4537,6 +4540,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,
  %s, _(Failed to generate UUID));
 goto error;
 }
+uuid_generated = true;
 } else {
 if (virUUIDParse(tmp, def-uuid)  0) {
 virDomainReportError(VIR_ERR_INTERNAL_ERROR,
@@ -5279,6 +5283,22 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
caps,

 if (def-sysinfo == NULL)
 goto error;
+if (def-sysinfo-system_uuid != NULL) {
+unsigned char uuidbuf[VIR_UUID_BUFLEN];
+if (virUUIDParse(def-sysinfo-system_uuid, uuidbuf)  0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ %s, _(malformed uuid element));
+goto error;
+}
+if (uuid_generated)
+memcpy(def-uuid, uuidbuf, VIR_UUID_BUFLEN);
+else if (memcmp(def-uuid, uuidbuf, VIR_UUID_BUFLEN) != 0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(UUID mismatch between uuid and 
+   sysinfo));
+goto error;
+}
+}
 }
 tmp = virXPathString(string(./os/smbios/@mode), ctxt);
 if (tmp) {
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 9a9d924..9ca1dad 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -3645,15 +3645,15 @@ error:
 return(NULL);
 }

-static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def)
+static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr def, bool skip_uuid)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;

 if ((def-system_manufacturer == NULL)  (def-system_sku == NULL) 
-(def-system_product == NULL)  (def-system_uuid == NULL) 
-(def-system_version == NULL)  (def-system_serial == NULL) 
-(def-system_family == NULL))
-return(NULL);
+(def-system_product == NULL)  (def-system_version == NULL) 
+(def-system_serial == NULL)  (def-system_family == NULL) 
+(def-system_uuid == NULL || skip_uuid))
+return NULL;

 virBufferAddLit(buf, type=1);

@@ -3671,7 +3671,7 @@ static char *qemuBuildSmbiosSystemStr(virSysinfoDefPtr 
def)
 if (def-system_serial)
 virBufferVSprintf(buf, ,serial=%s, def-system_serial);
 /* 1:UUID */
-if (def-system_uuid)
+if (def-system_uuid  !skip_uuid)
 virBufferVSprintf(buf, ,uuid=%s, def-system_uuid);
 /* 1:SKU Number */
 if (def-system_sku)
@@ -4173,6 +4173,7 @@ qemudBuildCommandLine(virConnectPtr conn,
 if ((def-os.smbios_mode != VIR_DOMAIN_SMBIOS_NONE) 
 (def-os.smbios_mode != VIR_DOMAIN_SMBIOS_EMULATE)) {
 virSysinfoDefPtr source = NULL;
+bool skip_uuid = false;

 if (!(qemuCmdFlags  QEMUD_CMD_FLAG_SMBIOS_TYPE)) {
 qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -4189,6 +4190,8 @@ qemudBuildCommandLine(virConnectPtr conn,
 goto error;
 }
 source = driver-hostsysinfo;
+/* Host and guest uuid must differ, by definition of UUID. */
+skip_uuid = true;
 } else if (def-os.smbios_mode == VIR_DOMAIN_SMBIOS_SYSINFO)