- Devel - libvirt List Archives

[libvirt] [PATCH] vboxDomainDumpXML: avoid a leak on OOM error path
by Jim Meyering 15 Feb '10

15 Feb '10

Yet another... >From 05e87caf9da26f9038935d7e54b6038b9362875b Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 20:09:55 +0100 Subject: [PATCH] vboxDomainDumpXML: avoid a leak on OOM error path * src/vbox/vbox_tmpl.c (vboxDomainDumpXML): Free vboxCallback buffer upon OOM. --- src/vbox/vbox_tmpl.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index d1a701e..8a9af52 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -5276,10 +5276,11 @@ static nsresult vboxCallbackQueryInterface(nsISupports *pThis, const nsID *iid, static IVirtualBoxCallback *vboxAllocCallbackObj(void) { IVirtualBoxCallback *vboxCallback = NULL; - /* Allocate, Initialize and return a validi + /* Allocate, Initialize and return a valid * IVirtualBoxCallback object here */ if ((VIR_ALLOC(vboxCallback) < 0) || (VIR_ALLOC(vboxCallback->vtbl) < 0)) { + VIR_FREE(vboxCallback); virReportOOMError(); return NULL; } -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH] virNodeDevCapScsiHostParseXML: avoid an unconditional leak
by Jim Meyering 15 Feb '10

15 Feb '10

It looks like there may still be error-path leaks (the buffers pointed to by nodes[i]), but that can wait for another day. >From c293a97ed55c99c87bc658aaffc10831f8ba8c68 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 19:54:45 +0100 Subject: [PATCH] virNodeDevCapScsiHostParseXML: avoid an unconditional leak * src/conf/node_device_conf.c (virNodeDevCapScsiHostParseXML): Free the "nodes" buffer allocated by virXPathNodeSet. --- src/conf/node_device_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 4b65953..09c0f41 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -797,6 +797,7 @@ virNodeDevCapScsiHostParseXML(xmlXPathContextPtr ctxt, out: VIR_FREE(type); ctxt->node = orignode; + VIR_FREE(nodes); return ret; } -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH] uml_driver.c: avoid leak upon failure
by Jim Meyering 15 Feb '10

15 Feb '10

Without this, it looked like every caller (there's only one; the other use is if-0'd out) could leak the result buffer. >From 8b8874b956c822562151cb90b1a8950d17b565ac Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 19:22:38 +0100 Subject: [PATCH] uml_driver.c: avoid leak upon failure * src/uml/uml_driver.c (umlMonitorCommand): This function would sometimes return -1, yet fail to free the "reply" it had allocated. Hence, no caller would know to free the corresponding argument. When returning -1, be sure to free all allocated resources. --- src/uml/uml_driver.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 5049c92..10268db 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1,7 +1,7 @@ /* * uml_driver.c: core driver methods for managing UML guests * - * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc. + * Copyright (C) 2006-2010 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -757,7 +757,10 @@ static int umlMonitorCommand(virConnectPtr conn, VIR_DEBUG("Command reply is '%s'", NULLSTR(retdata)); - *reply = retdata; + if (ret < 0) + VIR_FREE(retdata); + else + *reply = retdata; return ret; -- 1.7.0.181.g41533

2 1

[libvirt] [PATCH 2/4] qemu: Make SetVcpu command hotplug only
by Cole Robinson 15 Feb '10

15 Feb '10

Similar to the Set*Mem commands, this implementation was bogus and misleading. Make it clear this is a hotplug only operation, and that the hotplug piece isn't even implemented. Also drop the overkill maxvcpus validation: we don't perform this check at XML define time so clearly no one is missing it, and there is always the risk that our info will be out of date, possibly preventing legitimate CPU values. --- src/qemu/qemu_driver.c | 42 +++++++----------------------------------- 1 files changed, 7 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 56a450c..ef1f638 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4100,12 +4100,11 @@ cleanup: } -static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { +static int qemudDomainSetVcpus(virDomainPtr dom, + ATTRIBUTE_UNUSED unsigned int nvcpus) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - int max; int ret = -1; - const char *type; qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); @@ -4119,41 +4118,14 @@ static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) { goto cleanup; } - if (qemuDomainObjBeginJob(vm) < 0) + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); goto cleanup; - - if (virDomainObjIsActive(vm)) { - qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("cannot change vcpu count of an active domain")); - goto endjob; - } - - if (!(type = virDomainVirtTypeToString(vm->def->virtType))) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("unknown virt type in domain definition '%d'"), - vm->def->virtType); - goto endjob; } - if ((max = qemudGetMaxVCPUs(NULL, type)) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("could not determine max vcpus for the domain")); - goto endjob; - } - - if (nvcpus > max) { - qemuReportError(VIR_ERR_INVALID_ARG, - _("requested vcpus is greater than max allowable" - " vcpus for the domain: %d > %d"), nvcpus, max); - goto endjob; - } - - vm->def->vcpus = nvcpus; - ret = 0; - -endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuReportError(VIR_ERR_NO_SUPPORT, + "%s", _("cpu hotplug not yet supported")); cleanup: if (vm) -- 1.6.5.2

2 2

[libvirt] [PATCH] openvz_driver.c: avoid leak on OOM error path
by Jim Meyering 15 Feb '10

15 Feb '10

It looks like openvzFreeDriver cannot call VIR_FREE on its buffer argument, because of the use in the function that just happens to be a few lines below. These are the only two uses of openvzFreeDriver. >From 0a616cd03cca9c9d89f23dadd52cd23c30789aca Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 15 Feb 2010 18:33:38 +0100 Subject: [PATCH] openvz_driver.c: avoid leak on OOM error path * src/openvz/openvz_driver.c (openvzOpen): Free "driver" buffer upon failure. --- src/openvz/openvz_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 68d0398..3a8a82f 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1245,22 +1245,23 @@ static virDrvOpenStatus openvzOpen(virConnectPtr conn, goto cleanup; if (openvzExtractVersion(conn, driver) < 0) goto cleanup; conn->privateData = driver; return VIR_DRV_OPEN_SUCCESS; cleanup: openvzFreeDriver(driver); + VIR_FREE(driver); return VIR_DRV_OPEN_ERROR; }; static int openvzClose(virConnectPtr conn) { struct openvz_driver *driver = conn->privateData; openvzFreeDriver(driver); conn->privateData = NULL; return 0; } -- 1.7.0.181.g41533

2 3

[libvirt] [PATCH 3/5] [FIX] macvtap support for libvirt -- qemu support
by Stefan Berger 15 Feb '10

15 Feb '10

I needed to fix the case where macvtap support is not to be compiled in. This part adds support for qemu making a macvtap tap device available via file descriptor passed to qemu command line. This also attempts to tear down the macvtap device when a VM terminates. This includes support for attachment and detachment to/from running VM. Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com>

3 3

[libvirt] [PATCH] Treat missing QEMU 'thread_id' as non-fatal in JSON monitor
by Daniel P. Berrange 15 Feb '10

15 Feb '10

The plain QEMU tree does not include 'thread_id' in the JSON output. Thus we need to treat it as non-fatal if missing. * src/qemu/qemu_monitor_json.c: Treat missing thread_id as non-fatal --- src/qemu/qemu_monitor_json.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 9875f38..49e0370 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -630,8 +630,9 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply, } if (virJSONValueObjectGetNumberInt(entry, "thread_id", &thread) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cpu information was missing thread ID")); + /* Only qemu-kvm tree includs thread_id, so treat this as + non-fatal, simply returning no data */ + ret = 0; goto cleanup; } -- 1.6.6

2 1

[libvirt] [PATCH] Make error reporting for QEMU JSON mode more friendly
by Daniel P. Berrange 15 Feb '10

15 Feb '10

Current error reporting for JSON mode returns the full JSON command string and full JSON error string. This is not very user friendly, so this change makes the error report only contain the basic command name, and friendly error message description string. The full JSON data is logged instead. * src/qemu/qemu_monitor_json.c: Always return the 'desc' field from the JSON error message to users. --- src/qemu/qemu_monitor_json.c | 58 ++++++++++++++++++++++++++++------------- 1 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f0dcf81..9875f38 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -246,15 +246,33 @@ qemuMonitorJSONCommand(qemuMonitorPtr mon, * * XXX see qerror.h for different klasses & fill out useful params */ -static char *qemuMonitorJSONStringifyError(virJSONValuePtr error) +static const char * +qemuMonitorJSONStringifyError(virJSONValuePtr error) { const char *klass = virJSONValueObjectGetString(error, "class"); + const char *detail = NULL; - if (klass) { - return strdup(klass); - } else { - return strdup(_("Missing QEMU error klass")); - } + /* The QMP 'desc' field is usually sufficient for our generic + * error reporting needs. + */ + if (klass) + detail = virJSONValueObjectGetString(error, "desc"); + + + if (!detail) + detail = "unknown QEMU command error"; + + return detail; +} + +static const char * +qemuMonitorJSONCommandName(virJSONValuePtr cmd) +{ + const char *name = virJSONValueObjectGetString(cmd, "execute"); + if (name) + return name; + else + return "<unknown>"; } static int @@ -266,20 +284,21 @@ qemuMonitorJSONCheckError(virJSONValuePtr cmd, char *cmdstr = virJSONValueToString(cmd); char *replystr = virJSONValueToString(reply); - if (!error) { - VIR_DEBUG("Saw a JSON error, but value is null for %s: %s", - cmdstr, replystr); + /* Log the full JSON formatted command & error */ + VIR_DEBUG("unable to execute QEMU command %s: %s", + cmdstr, replystr); + + /* Only send the user the command name + friendly error */ + if (!error) qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("error running QEMU command '%s': '%s'"), - cmdstr, replystr); - } else { - VIR_DEBUG("Got a JSON error set for %s", cmdstr); - char *detail = qemuMonitorJSONStringifyError(error); + _("unable to execute QEMU command '%s'"), + qemuMonitorJSONCommandName(cmd)); + else qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("error running QEMU command '%s': %s ('%s')"), - cmdstr, detail, replystr); - VIR_FREE(detail); - } + _("unable to execute QEMU command '%s': %s"), + qemuMonitorJSONCommandName(cmd), + qemuMonitorJSONStringifyError(error)); + VIR_FREE(cmdstr); VIR_FREE(replystr); return -1; @@ -290,7 +309,8 @@ qemuMonitorJSONCheckError(virJSONValuePtr cmd, VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s", cmdstr, replystr); qemuReportError(VIR_ERR_INTERNAL_ERROR, - _("error running QEMU command '%s': '%s'"), cmdstr, replystr); + _("unable to execute QEMU command '%s'"), + qemuMonitorJSONCommandName(cmd)); VIR_FREE(cmdstr); VIR_FREE(replystr); return -1; -- 1.6.6

2 1

[libvirt] [PATCH] Fix check for primary IDE controller in QEMU PCI slot assignment
by Daniel P. Berrange 15 Feb '10

15 Feb '10

A typo in the check for the primary IDE controller could cause a crash on restore depending on the exact guest config. * src/qemu/qemu_conf.c: Fix s/video/controller/ typo & slot number typo --- src/qemu/qemu_conf.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 00f761d..dd5efd6 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -2085,10 +2085,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && def->controllers[i]->idx == 0) { if (def->controllers[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (def->videos[i]->info.addr.pci.domain != 0 || - def->videos[i]->info.addr.pci.bus != 0 || - def->videos[i]->info.addr.pci.slot != 2 || - def->videos[i]->info.addr.pci.function != 0) { + if (def->controllers[i]->info.addr.pci.domain != 0 || + def->controllers[i]->info.addr.pci.bus != 0 || + def->controllers[i]->info.addr.pci.slot != 1 || + def->controllers[i]->info.addr.pci.function != 1) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Primary IDE controller must have PCI address 0:0:1.1")); goto error; -- 1.6.6

2 1

[libvirt] [PATCH] interface: Use proper return codes in the open function
by Matthias Bolte 15 Feb '10

15 Feb '10

The open function returned -1 in case of an error, but -1 maps to VIR_DRV_OPEN_DECLINED instead of VIR_DRV_OPEN_ERROR. --- src/interface/netcf_driver.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c index 2753049..7f4d43d 100644 --- a/src/interface/netcf_driver.c +++ b/src/interface/netcf_driver.c @@ -139,7 +139,7 @@ static virDrvOpenStatus interfaceOpenInterface(virConnectPtr conn, } conn->interfacePrivateData = driverState; - return 0; + return VIR_DRV_OPEN_SUCCESS; netcf_error: if (driverState->netcf) @@ -150,7 +150,7 @@ netcf_error: mutex_error: VIR_FREE(driverState); alloc_error: - return -1; + return VIR_DRV_OPEN_ERROR; } static int interfaceCloseInterface(virConnectPtr conn) -- 1.6.3.3

2 2