From 39c7e7a6b79bbdfa36928a430d56fa88a204e8fd Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Mon, 29 Jun 2009 10:41:56 +0000 Subject: [PATCH] Fix crash in QEMU driver with bad capabilities data --- ChangeLog | 11 ++++++ src/capabilities.c | 14 +++++-- src/capabilities.h | 3 ++ src/libvirt_private.syms | 1 + src/qemu_conf.c | 10 ++++- src/qemu_driver.c | 80 +++++++++++++++++++++++++++------------- 6 files changed, 88 insertions(+), 31 deletions(-) diff --git a/ChangeLog b/ChangeLog index 784e274e5c..ad29f259c0 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 00a4407552..def3fd07c1 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 0d476d1073..c3ca89a6f4 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 52c49674c2..bd7910b3e5 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 a68a79b921..99193dcfff 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 14d00cc95a..2599212793 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; -- GitLab