diff --git a/ChangeLog b/ChangeLog index 784e274e5c8bb0e15ad349d4e0ae22dc590975c2..ad29f259c0272f49171e9b2b57fd436e1a7a7b47 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +Mon Jun 29 10:51:20 BST 2009 Daniel P. Berrange + + 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 * src/parthelper.c: fix a superfluous % on printf format problem diff --git a/src/capabilities.c b/src/capabilities.c index 00a44075527dbb9ca6293ca41cc73d1fb1f89492..def3fd07c1a3e85ecfdf93fabce038826bd112cf 100644 --- 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 --- 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 --- 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 --- 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 --- 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;