Fix crash in QEMU driver with bad capabilities data

ChangeLog

+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

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]);

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);

src/libvirt_private.syms

@@ -24,6 +24,7 @@ virCapabilitiesDefaultGuestEmulator;
 virCapabilitiesDefaultGuestMachine;
 virCapabilitiesFormatXML;
 virCapabilitiesFree;
+virCapabilitiesFreeNUMAInfo;
 virCapabilitiesNew;
 virCapabilitiesSetMacPrefix;
 virCapabilitiesGenerateMac;

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");

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;