Fix crash in QEMU driver with bad capabilities data

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

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]);
+    virCapabilitiesFreeNUMAInfo(caps);
-    VIR_FREE(caps->host.numaCell);
 
     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)
+    /* Some machines have problematic NUMA toplogy causing
-        goto no_memory;
+     * 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,38 +347,15 @@ qemuReconnectDomains(struct qemud_driver *driver)
     }
 }
 
+
 static int
-qemudSecurityInit(struct qemud_driver *qemud_drv)
+qemudSecurityCapsInit(virSecurityDriverPtr secdrv,
+                      virCapsPtr caps)
 {
-    int ret;
     const char *doi, *model;
-    virCapsPtr caps;
-    virSecurityDriverPtr security_drv;
 
-    ret = virSecurityDriverStartup(&security_drv,
+    doi = virSecurityDriverGetDOI(secdrv);
-                                   qemud_drv->securityDriverName);
+    model = virSecurityDriverGetModel(secdrv);
-    if (ret == -1) {
-        VIR_ERROR0(_("Failed to start security driver"));
-        return -1;
-    }
-    /* No security driver wanted to be enabled: just return */
-    if (ret == -2) {
-        VIR_INFO0(_("No security driver available"));
-        return 0;
-    }
-
-    qemud_drv->securityDriver = security_drv;
-    doi = virSecurityDriverGetDOI(security_drv);
-    model = virSecurityDriverGetModel(security_drv);
-
-    VIR_DEBUG("Initialized security driver \"%s\" with "
-              "DOI \"%s\"", model, doi);
-
-    /*
-     * 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) {
@@ -396,9 +373,44 @@ qemudSecurityInit(struct qemud_driver *qemud_drv)
         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;
+    virSecurityDriverPtr security_drv;
+
+    ret = virSecurityDriverStartup(&security_drv,
+                                   qemud_drv->securityDriverName);
+    if (ret == -1) {
+        VIR_ERROR0(_("Failed to start security driver"));
+        return -1;
+    }
+    /* No security driver wanted to be enabled: just return */
+    if (ret == -2) {
+        VIR_INFO0(_("No security driver available"));
+        return 0;
+    }
+
+    qemud_drv->securityDriver = security_drv;
+
+    VIR_INFO("Initialized security driver %s", security_drv->name);
+
+    /*
+     * Add security policy host caps now that the security driver is
+     * initialized.
+     */
+    return qemudSecurityCapsInit(security_drv, qemud_drv->caps);
+}
+
+
+
 /**
  * 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);
-    virCapabilitiesFree(qemu_driver->caps);
+    if ((caps = qemudCapsInit()) == NULL) {
-    if ((qemu_driver->caps = qemudCapsInit()) == NULL ||
-        (xml = virCapabilitiesFormatXML(driver->caps)) == 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);
+    qemu_driver->caps = caps;
+
+    if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL)
+        virReportOOMError(conn);
+
+cleanup:
     qemuDriverUnlock(driver);
 
     return xml;