Fix crash in QEMU driver with bad capabilities data

diff --git a/ChangeLog b/ChangeLog
index 784e274e5c8bb0e15ad349d4e0ae22dc590975c2..ad29f259c0272f49171e9b2b57fd436e1a7a7b47 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+Mon Jun 29 10:51:20 BST 2009 Daniel P. Berrange <berrange@redhat.com>
+
+       Fix crash in QEMU driver with bad capabilities data
+       * src/capabilities.c, src/capabilities.h: Export a method
+       virCapabilitiesFreeNUMAInfo()
+       * src/qemu_conf.c: Don't kill the whole QEMU driver if
+       populating capabilities with NUMA info fails.
+       * src/qemu_driver.c: Fix missing security model data
+       after capabilities refresh. Avoid leaving driver with
+       NULL capabilities if refresh fails.
+
 Fri Jun 26 22:13:16 CEST 2009 Daniel Veillard <veillard@redhat.com>
 
        * src/parthelper.c: fix a superfluous % on printf format problem

diff --git a/src/capabilities.c b/src/capabilities.c
index 00a44075527dbb9ca6293ca41cc73d1fb1f89492..def3fd07c1a3e85ecfdf93fabce038826bd112cf 100644 (file)
--- a/src/capabilities.c
+++ b/src/capabilities.c
@@ -121,6 +121,15 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
     VIR_FREE(guest);
 }
 
+void
+virCapabilitiesFreeNUMAInfo(virCapsPtr caps)
+{
+    int i;
+
+    for (i = 0 ; i < caps->host.nnumaCell ; i++)
+        virCapabilitiesFreeHostNUMACell(caps->host.numaCell[i]);
+    VIR_FREE(caps->host.numaCell);
+}
 
 /**
  * virCapabilitiesFree:
@@ -141,9 +150,8 @@ virCapabilitiesFree(virCapsPtr caps) {
     for (i = 0 ; i < caps->host.nfeatures ; i++)
         VIR_FREE(caps->host.features[i]);
     VIR_FREE(caps->host.features);
-    for (i = 0 ; i < caps->host.nnumaCell ; i++)
-        virCapabilitiesFreeHostNUMACell(caps->host.numaCell[i]);
-    VIR_FREE(caps->host.numaCell);
+
+    virCapabilitiesFreeNUMAInfo(caps);
 
     for (i = 0 ; i < caps->host.nmigrateTrans ; i++)
         VIR_FREE(caps->host.migrateTrans[i]);

diff --git a/src/capabilities.h b/src/capabilities.h
index 0d476d1073634fca06e1f3b4cd9382fac06b70c1..c3ca89a6f4f9cf5d56a6200075790195c9377ec3 100644 (file)
--- a/src/capabilities.h
+++ b/src/capabilities.h
@@ -118,6 +118,9 @@ virCapabilitiesNew(const char *arch,
 extern void
 virCapabilitiesFree(virCapsPtr caps);
 
+extern void
+virCapabilitiesFreeNUMAInfo(virCapsPtr caps);
+
 extern void
 virCapabilitiesSetMacPrefix(virCapsPtr caps,
                             unsigned char *prefix);

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 52c49674c2199a8548cf00b68ce0ab5ed08e67c7..bd7910b3e5fba9bab7d39c7f1c83fe1270cc19a9 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -24,6 +24,7 @@ virCapabilitiesDefaultGuestEmulator;
 virCapabilitiesDefaultGuestMachine;
 virCapabilitiesFormatXML;
 virCapabilitiesFree;
+virCapabilitiesFreeNUMAInfo;
 virCapabilitiesNew;
 virCapabilitiesSetMacPrefix;
 virCapabilitiesGenerateMac;

diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index a68a79b92155b6cf571a0d1128477d0810b38a44..99193dcfffaa9c67572d00f510dbfd84cac4b20f 100644 (file)
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -377,8 +377,14 @@ virCapsPtr qemudCapsInit(void) {
     /* Using KVM's mac prefix for QEMU too */
     virCapabilitiesSetMacPrefix(caps, (unsigned char[]){ 0x52, 0x54, 0x00 });
 
-    if (nodeCapsInitNUMA(caps) < 0)
-        goto no_memory;
+    /* Some machines have problematic NUMA toplogy causing
+     * unexpected failures. We don't want to break the QEMU
+     * driver in this scenario, so log errors & carry on
+     */
+    if (nodeCapsInitNUMA(caps) < 0) {
+        virCapabilitiesFreeNUMAInfo(caps);
+        VIR_WARN0("Failed to query host NUMA topology, disabling NUMA capabilities");
+    }
 
     virCapabilitiesAddHostMigrateTransport(caps,
                                            "tcp");

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 14d00cc95affe9333f925f33ce04dbdf59b4d055..2599212793fd388e87da24269956802201e88ac4 100644 (file)
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -347,12 +347,43 @@ qemuReconnectDomains(struct qemud_driver *driver)
     }
 }
 
+
+static int
+qemudSecurityCapsInit(virSecurityDriverPtr secdrv,
+                      virCapsPtr caps)
+{
+    const char *doi, *model;
+
+    doi = virSecurityDriverGetDOI(secdrv);
+    model = virSecurityDriverGetModel(secdrv);
+
+    caps->host.secModel.model = strdup(model);
+    if (!caps->host.secModel.model) {
+        char ebuf[1024];
+        VIR_ERROR(_("Failed to copy secModel model: %s"),
+                  virStrerror(errno, ebuf, sizeof ebuf));
+        return -1;
+    }
+
+    caps->host.secModel.doi = strdup(doi);
+    if (!caps->host.secModel.doi) {
+        char ebuf[1024];
+        VIR_ERROR(_("Failed to copy secModel DOI: %s"),
+                  virStrerror(errno, ebuf, sizeof ebuf));
+        return -1;
+    }
+
+    VIR_DEBUG("Initialized caps for security driver \"%s\" with "
+              "DOI \"%s\"", model, doi);
+
+    return 0;
+}
+
+
 static int
 qemudSecurityInit(struct qemud_driver *qemud_drv)
 {
     int ret;
-    const char *doi, *model;
-    virCapsPtr caps;
     virSecurityDriverPtr security_drv;
 
     ret = virSecurityDriverStartup(&security_drv,
@@ -368,36 +399,17 @@ qemudSecurityInit(struct qemud_driver *qemud_drv)
     }
 
     qemud_drv->securityDriver = security_drv;
-    doi = virSecurityDriverGetDOI(security_drv);
-    model = virSecurityDriverGetModel(security_drv);
 
-    VIR_DEBUG("Initialized security driver \"%s\" with "
-              "DOI \"%s\"", model, doi);
+    VIR_INFO("Initialized security driver %s", security_drv->name);
 
     /*
      * Add security policy host caps now that the security driver is
      * initialized.
      */
-    caps = qemud_drv->caps;
-
-    caps->host.secModel.model = strdup(model);
-    if (!caps->host.secModel.model) {
-        char ebuf[1024];
-        VIR_ERROR(_("Failed to copy secModel model: %s"),
-                  virStrerror(errno, ebuf, sizeof ebuf));
-        return -1;
-    }
+    return qemudSecurityCapsInit(security_drv, qemud_drv->caps);
+}
 
-    caps->host.secModel.doi = strdup(doi);
-    if (!caps->host.secModel.doi) {
-        char ebuf[1024];
-        VIR_ERROR(_("Failed to copy secModel DOI: %s"),
-                  virStrerror(errno, ebuf, sizeof ebuf));
-        return -1;
-    }
 
-    return 0;
-}
 
 /**
  * qemudStartup:
@@ -1866,13 +1878,29 @@ static int qemudGetMaxVCPUs(virConnectPtr conn, const char *type) {
 
 static char *qemudGetCapabilities(virConnectPtr conn) {
     struct qemud_driver *driver = conn->privateData;
+    virCapsPtr caps;
     char *xml = NULL;
 
     qemuDriverLock(driver);
+    if ((caps = qemudCapsInit()) == NULL) {
+        virReportOOMError(conn);
+        goto cleanup;
+    }
+
+    if (qemu_driver->securityDriver &&
+        qemudSecurityCapsInit(qemu_driver->securityDriver, caps) < 0) {
+        virCapabilitiesFree(caps);
+        virReportOOMError(conn);
+        goto cleanup;
+    }
+
     virCapabilitiesFree(qemu_driver->caps);
-    if ((qemu_driver->caps = qemudCapsInit()) == NULL ||
-        (xml = virCapabilitiesFormatXML(driver->caps)) == NULL)
+    qemu_driver->caps = caps;
+
+    if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL)
         virReportOOMError(conn);
+
+cleanup:
     qemuDriverUnlock(driver);
 
     return xml;