May 2010 - Devel - libvirt List Archives

[libvirt] reviewing of patches
by Stephen Shaw 18 May '10

18 May '10

I was wondering after watching this mailing list fro the last few weeks if you guys have maybe thought about using something like reviewboard. We have been using it for our team for the past several months and have really liked it. I imagine it would cut down on the volume of traffic and make giving inline comments easier. It also tests to make sure the patch still applies when you go to review it. Here is a link to the instance that we use for our project: http://reviews.mono-a11y.org/dashboard/ Cheers, Stephen

1 0

[libvirt] [PATCH] Fix NULL dereference in remoteDomainMigratePrepare2
by jdenemar＠redhat.com 18 May '10

18 May '10

From: Jiri Denemark <jdenemar(a)redhat.com> --- src/remote/remote_driver.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 990bfce..c62e3d6 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2849,8 +2849,12 @@ remoteDomainMigratePrepare2 (virConnectPtr dconn, goto done; if (ret.cookie.cookie_len > 0) { - *cookie = ret.cookie.cookie_val; /* Caller frees. */ - *cookielen = ret.cookie.cookie_len; + if (cookie && cookielen) { + *cookie = ret.cookie.cookie_val; /* Caller frees. */ + *cookielen = ret.cookie.cookie_len; + } else { + VIR_FREE(ret.cookie.cookie_val); + } } if (ret.uri_out) *uri_out = *ret.uri_out; /* Caller frees. */ -- 1.7.1

4 6

[libvirt] [PATCH] Add support for SSE4.1 and SSE4.2 CPU features
by Jiri Denemark 18 May '10

18 May '10

--- src/cpu/cpu_map.xml | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml index 30c28d7..084b879 100644 --- a/src/cpu/cpu_map.xml +++ b/src/cpu/cpu_map.xml @@ -126,6 +126,12 @@ <feature name='dca'>  <cpuid function='0x00000001' ecx='0x00040000'/> </feature> + <feature name='sse4.1'>  + <cpuid function='0x00000001' ecx='0x00080000'/> + </feature> + <feature name='sse4.2'>  + <cpuid function='0x00000001' ecx='0x00100000'/> + </feature> <feature name='x2apic'>  <cpuid function='0x00000001' ecx='0x00200000'/> </feature> -- 1.7.1

2 2

[libvirt] FYI, I've pushed a bunch of ACK'd patches
by Jim Meyering 18 May '10

18 May '10

I've pushed these: ebiptablesWriteToTempFile: don't close a negative file descriptor virDomainNetDefParseXML: avoid leak upon multiple "filterref" linuxNodeInfoCPUPopulate: avoid used-uninitialized via a test virNWFilterDefParseXML: avoid leak on error paths qemuMonitorTextMigrate: avoid leak on OOM-error path tests: do not ignore virInitialize failure python: don't ignore virInitialize failure in module initialization qemudDomainRestore: handle a case of virDomainSaveStatus failure ebtablesAddRemoveRule, iptablesAddRemoveRule: don't skip va_end do not ignore qemuMonitorAddDrive failure; make uses identical

1 0

[libvirt] [PATCH] virDomainNetDefParseXML: avoid leak upon multiple "filterref"
by Jim Meyering 18 May '10

18 May '10

The offending code below appears in this loop: virNWFilterHashTablePtr filterparams = NULL; ... cur = node->children; while (cur != NULL) { ... } so the first assignment works fine, but second and subsequent ones leak the buffer returned by each preceding virNWFilterParseParamAttributes call. >From 8659fb1ae879befe360e1ec7b8b62434c22698cd Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 10:19:30 +0200 Subject: [PATCH] virDomainNetDefParseXML: avoid leak upon multiple "filterref" * src/conf/domain_conf.c (virDomainNetDefParseXML): Don't leak memory when parsing two or more "filterref" elements. --- src/conf/domain_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e45f79..0c717f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1905,6 +1905,7 @@ virDomainNetDefParseXML(virCapsPtr caps, model = virXMLPropString(cur, "type"); } else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) { filter = virXMLPropString(cur, "filter"); + free(filterparams); filterparams = virNWFilterParseParamAttributes(cur); } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && xmlStrEqual(cur->name, BAD_CAST "state")) { -- 1.7.1.250.g7d1e8

3 3

[libvirt] [PATCH] tests: do not ignore virInitialize failure
by Jim Meyering 18 May '10

18 May '10

Simple... >From f5ee09ed08473478b3ea3135d51125fbf687e402 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 12:32:39 +0200 Subject: [PATCH] tests: do not ignore virInitialize failure * tests/nodeinfotest.c (mymain): Do not ignore virInitialize failure. Most other callers of virInitialize test for failure. --- tests/nodeinfotest.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c index ff056b9..d3c500d 100644 --- a/tests/nodeinfotest.c +++ b/tests/nodeinfotest.c @@ -106,7 +106,8 @@ mymain(int argc, char **argv) return(EXIT_FAILURE); } - virInitialize(); + if (virInitialize() < 0) + return EXIT_FAILURE; for (i = 0 ; i < ARRAY_CARDINALITY(nodeData); i++) if (virtTestRun(nodeData[i], 1, linuxTestNodeInfo, nodeData[i]) != 0) -- 1.7.1.250.g7d1e8

3 5

[libvirt] [PATCH] ebtablesAddRemoveRule, iptablesAddRemoveRule: don't skip va_end
by Jim Meyering 18 May '10

18 May '10

Coverity spotted two missing uses of va_end: >From 4baa4228a74f640fe789914bd034e4c5b805cdfb Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 14:42:33 +0200 Subject: [PATCH] ebtablesAddRemoveRule, iptablesAddRemoveRule: don't skip va_end * src/util/ebtables.c (ebtablesAddRemoveRule): Don't skip va_end(args) on an error path. * src/util/iptables.c (iptablesAddRemoveRule): Identical change. --- src/util/ebtables.c | 7 +++++-- src/util/iptables.c | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/util/ebtables.c b/src/util/ebtables.c index e2b9608..f707756 100644 --- a/src/util/ebtables.c +++ b/src/util/ebtables.c @@ -207,15 +207,18 @@ ebtablesAddRemoveRule(ebtRules *rules, int action, const char *arg, ...) if (!(argv[n++] = strdup(arg))) goto error; va_start(args, arg); - while ((s = va_arg(args, const char *))) - if (!(argv[n++] = strdup(s))) + while ((s = va_arg(args, const char *))) { + if (!(argv[n++] = strdup(s))) { + va_end(args); goto error; + } + } va_end(args); if (!(rule = virArgvToString(&argv[command_idx]))) goto error; diff --git a/src/util/iptables.c b/src/util/iptables.c index 4f95a02..d06b857 100644 --- a/src/util/iptables.c +++ b/src/util/iptables.c @@ -134,15 +134,18 @@ iptablesAddRemoveRule(iptRules *rules, int action, const char *arg, ...) if (!(argv[n++] = strdup(arg))) goto error; va_start(args, arg); - while ((s = va_arg(args, const char *))) - if (!(argv[n++] = strdup(s))) + while ((s = va_arg(args, const char *))) { + if (!(argv[n++] = strdup(s))) { + va_end(args); goto error; + } + } va_end(args); if (virRun(argv, NULL) < 0) { retval = errno; goto error; -- 1.7.1.250.g7d1e8

2 1

[libvirt] [PATCH] qemudDomainRestore: handle a case of virDomainSaveStatus failure
by Jim Meyering 18 May '10

18 May '10

For each of the other uses of virDomainSaveStatus, the caller handles failure. Here it was ignored. At first I was inclined to simply "goto endjob;", like a few other uses in this file, but probing a bit, I found a few paths by which it could fail with no diagnostic whatsoever. And even if we've already emitted a diagnostic about some lower-level failure, it probably helps the user to know what high-level operation failed. >From af983f0c730fcecf90a8c948a7537095bc2e80e7 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 14:07:18 +0200 Subject: [PATCH] qemudDomainRestore: handle a case of virDomainSaveStatus failure * src/qemu/qemu_driver.c (qemudDomainRestore): Don't ignore virDomainSaveStatus failure. --- src/qemu/qemu_driver.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 948ab5b..5649a20 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6167,25 +6167,28 @@ static int qemudDomainRestore(virConnectPtr conn, if (header.was_running) { qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainObjEnterMonitorWithDriver(driver, vm); if (qemuMonitorStartCPUs(priv->mon, conn) < 0) { if (virGetLastError() == NULL) qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to resume domain")); qemuDomainObjExitMonitorWithDriver(driver,vm); goto endjob; } qemuDomainObjExitMonitorWithDriver(driver, vm); vm->state = VIR_DOMAIN_RUNNING; - virDomainSaveStatus(driver->caps, driver->stateDir, vm); + if (virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) { + VIR_WARN("Failed to save status on vm %s", vm->def->name); + goto endjob; + } } ret = 0; endjob: if (vm && qemuDomainObjEndJob(vm) == 0) vm = NULL; cleanup: virDomainDefFree(def); VIR_FREE(xml); if (fd != -1) -- 1.7.1.250.g7d1e8

2 1

[libvirt] python module set-up ignores virInitialize failure
by Jim Meyering 18 May '10

18 May '10

I've just fixed code in a test that ignored virInitialize failure. Looking at all uses, I saw one other: in python/libvirt-override.c, where the initialization function ignores virInitialize failure: void #ifndef __CYGWIN__ initlibvirtmod #else initcygvirtmod #endif (void) { static int initialized = 0; if (initialized != 0) return; virInitialize(); /* initialize the python extension module */ Py_InitModule((char *) #ifndef __CYGWIN__ "libvirtmod" #else "cygvirtmod" #endif , libvirtMethods); initialized = 1; } Unfortunately, this function is public, so we can't change its signature. Any suggestions? For reference, here's the function definition. It shows that there are many ways in which virInitialize can fail, including its many registration functions: /** * virInitialize: * * Initialize the library. It's better to call this routine at startup * in multithreaded applications to avoid potential race when initializing * the library. * * Returns 0 in case of success, -1 in case of error */ int virInitialize(void) { if (initialized) return(0); initialized = 1; if (virThreadInitialize() < 0 || virErrorInitialize() < 0 || virRandomInitialize(time(NULL) ^ getpid())) return -1; gcry_control(GCRYCTL_SET_THREAD_CBS, &virTLSThreadImpl); gcry_check_version(NULL); virLogSetFromEnv(); DEBUG0("register drivers"); #if HAVE_WINSOCK2_H if (winsock_init () == -1) return -1; #endif if (!bindtextdomain(GETTEXT_PACKAGE, LOCALEBASEDIR)) return (-1); /* * Note that the order is important: the first ones have a higher * priority when calling virConnectOpen. */ #ifdef WITH_DRIVER_MODULES /* We don't care if any of these fail, because the whole point * is to allow users to only install modules they want to use. * If they try to open a connection for a module that * is not loaded they'll get a suitable error at that point */ virDriverLoadModule("test"); virDriverLoadModule("xen"); virDriverLoadModule("openvz"); virDriverLoadModule("vbox"); virDriverLoadModule("esx"); virDriverLoadModule("xenapi"); virDriverLoadModule("remote"); #else # ifdef WITH_TEST if (testRegister() == -1) return -1; # endif # ifdef WITH_XEN if (xenRegister () == -1) return -1; # endif # ifdef WITH_OPENVZ if (openvzRegister() == -1) return -1; # endif # ifdef WITH_PHYP if (phypRegister() == -1) return -1; # endif # ifdef WITH_VBOX if (vboxRegister() == -1) return -1; # endif # ifdef WITH_ESX if (esxRegister() == -1) return -1; # endif # ifdef WITH_XENAPI if (xenapiRegister() == -1) return -1; # endif # ifdef WITH_REMOTE if (remoteRegister () == -1) return -1; # endif #endif return(0); }

3 6

[libvirt] [PATCH] qemuMonitorTextMigrate: avoid leak on OOM-error path
by Jim Meyering 18 May '10

18 May '10

Oops. Nearly forgot to post this one: >From f1b58f1f7bd287b4a2157f62ebb63100b45a8b62 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 12:17:23 +0200 Subject: [PATCH] qemuMonitorTextMigrate: avoid leak on OOM-error path * src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrate): Also free "safedest" buffer when failing. --- src/qemu/qemu_monitor_text.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index ae5d4d2..ec3d69d 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -1149,6 +1149,7 @@ static int qemuMonitorTextMigrate(qemuMonitorPtr mon, if (virBufferError(&extra)) { virBufferFreeAndReset(&extra); virReportOOMError(); + free(safedest); return -1; } if (virAsprintf(&cmd, "migrate %s\"%s\"", virBufferContentAndReset(&extra), safedest) < 0) { -- 1.7.1.250.g7d1e8

2 1