May 2010 - Devel - libvirt List Archives

[libvirt] [PATCH] correct typo: s/VIR_MIGRATE_TUNNELLED/VIR_MIGRATE_TUNNELED/, ...
by Jim Meyering 19 May '10

19 May '10

I noticed a typo in a public interface. IMHO it's well worth fixing, so propose this: (imagine someone searching for all occurrences of "tunneled" with the proper spelling. They would miss this symbol.) >From b9a1ea6c9e3d4e5150c39868b430f184c10b2660 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 18:28:17 +0200 Subject: [PATCH] correct typo: s/VIR_MIGRATE_TUNNELLED/VIR_MIGRATE_TUNNELED/, ... in public interface, but retain the original name for compatibility Correct all uses. * include/libvirt/libvirt.h.in [enum virDomainMigrateFlags] (VIR_MIGRATE_TUNNELED): Rename from spelling-error VIR_MIGRATE_TUNNELLED. (VIR_MIGRATE_TUNNELLED): Define, to map misspelled name to the new name. * src/libvirt.c (virDomainMigrate): Use correct spelling. * src/qemu/qemu_driver.c: Likewise. * tools/virsh.c (cmdMigrate): Likewise. --- include/libvirt/libvirt.h.in | 7 +++++-- src/libvirt.c | 10 +++++----- src/qemu/qemu_driver.c | 10 +++++----- tools/virsh.c | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 838028d..6e8e52e 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4,7 +4,7 @@ * Description: Provides the interfaces of the libvirt library to handle * virtualized domains * - * Copy: Copyright (C) 2005,2006 Red Hat, Inc. + * Copy: Copyright (C) 2005,2006,2010 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -407,7 +407,7 @@ typedef enum { typedef enum { VIR_MIGRATE_LIVE = (1 << 0), /* live migration */ VIR_MIGRATE_PEER2PEER = (1 << 1), /* direct source -> dest host control channel */ - VIR_MIGRATE_TUNNELLED = (1 << 2), /* tunnel migration data over libvirtd connection */ + VIR_MIGRATE_TUNNELED = (1 << 2), /* tunnel migration data over libvirtd connection */ VIR_MIGRATE_PERSIST_DEST = (1 << 3), /* persist the VM on the destination */ VIR_MIGRATE_UNDEFINE_SOURCE = (1 << 4), /* undefine the VM on the source */ VIR_MIGRATE_PAUSED = (1 << 5), /* pause on remote side */ @@ -417,6 +417,9 @@ typedef enum { } virDomainMigrateFlags; +/* Legacy typo. */ +#define VIR_MIGRATE_TUNNELLED VIR_MIGRATE_TUNNELED + /* Domain migration. */ virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, unsigned long flags, const char *dname, diff --git a/src/libvirt.c b/src/libvirt.c index eb05337..c10db19 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -3421,14 +3421,14 @@ virDomainMigrateDirect (virDomainPtr domain, * Flags may be one of more of the following: * VIR_MIGRATE_LIVE Do not pause the VM during migration * VIR_MIGRATE_PEER2PEER Direct connection between source & destination hosts - * VIR_MIGRATE_TUNNELLED Tunnel migration data over the libvirt RPC channel + * VIR_MIGRATE_TUNNELED Tunnel migration data over the libvirt RPC channel * VIR_MIGRATE_PERSIST_DEST If the migration is successful, persist the domain * on the destination host. * VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the * domain on the source host. * VIR_MIGRATE_PAUSED Leave the domain suspended on the remote side. * - * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. + * VIR_MIGRATE_TUNNELED requires that VIR_MIGRATE_PEER2PEER be set. * Applications using the VIR_MIGRATE_PEER2PEER flag will probably * prefer to invoke virDomainMigrateToURI, avoiding the need to * open connection to the destination host themselves. @@ -3530,7 +3530,7 @@ virDomainMigrate (virDomainPtr domain, goto error; } } else { - if (flags & VIR_MIGRATE_TUNNELLED) { + if (flags & VIR_MIGRATE_TUNNELED) { virLibConnError(domain->conn, VIR_ERR_OPERATION_INVALID, _("cannot perform tunnelled migration without using peer2peer flag")); goto error; @@ -3579,7 +3579,7 @@ error: * Flags may be one of more of the following: * VIR_MIGRATE_LIVE Do not pause the VM during migration * VIR_MIGRATE_PEER2PEER Direct connection between source & destination hosts - * VIR_MIGRATE_TUNNELLED Tunnel migration data over the libvirt RPC channel + * VIR_MIGRATE_TUNNELED Tunnel migration data over the libvirt RPC channel * VIR_MIGRATE_PERSIST_DEST If the migration is successful, persist the domain * on the destination host. * VIR_MIGRATE_UNDEFINE_SOURCE If the migration is successful, undefine the @@ -3598,7 +3598,7 @@ error: * must be a valid libvirt connection URI, by which the source * libvirt driver can connect to the destination libvirt. * - * VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set. + * VIR_MIGRATE_TUNNELED requires that VIR_MIGRATE_PEER2PEER be set. * * If a hypervisor supports renaming domains during migration, * the dname parameter specifies the new name for the domain. diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 609fabe..246ad9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9880,7 +9880,7 @@ qemudDomainMigratePrepareTunnel(virConnectPtr dconn, "%s", _("no domain XML passed")); goto cleanup; } - if (!(flags & VIR_MIGRATE_TUNNELLED)) { + if (!(flags & VIR_MIGRATE_TUNNELED)) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("PrepareTunnel called but no TUNNELLED flag set")); goto cleanup; @@ -10038,9 +10038,9 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn, *uri_out = NULL; qemuDriverLock(driver); - if (flags & VIR_MIGRATE_TUNNELLED) { + if (flags & VIR_MIGRATE_TUNNELED) { /* this is a logical error; we never should have gotten here with - * VIR_MIGRATE_TUNNELLED set + * VIR_MIGRATE_TUNNELED set */ qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Tunnelled migration requested but invalid RPC method called")); @@ -10604,7 +10604,7 @@ static int doPeer2PeerMigrate(virDomainPtr dom, goto cleanup; } - if (flags & VIR_MIGRATE_TUNNELLED) + if (flags & VIR_MIGRATE_TUNNELED) ret = doTunnelMigrate(dom, driver, dconn, vm, dom_xml, uri, flags, dname, resource); else ret = doNonTunnelMigrate(dom, driver, dconn, vm, dom_xml, uri, flags, dname, resource); @@ -10665,7 +10665,7 @@ qemudDomainMigratePerform (virDomainPtr dom, goto endjob; } - if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) { + if ((flags & (VIR_MIGRATE_TUNNELED | VIR_MIGRATE_PEER2PEER))) { if (doPeer2PeerMigrate(dom, driver, vm, uri, flags, dname, resource) < 0) /* doPeer2PeerMigrate already set the error, so just get out */ goto endjob; diff --git a/tools/virsh.c b/tools/virsh.c index 4930ad7..9d9cf5d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2898,7 +2898,7 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool (cmd, "p2p")) flags |= VIR_MIGRATE_PEER2PEER; if (vshCommandOptBool (cmd, "tunnelled")) - flags |= VIR_MIGRATE_TUNNELLED; + flags |= VIR_MIGRATE_TUNNELED; if (vshCommandOptBool (cmd, "persistent")) flags |= VIR_MIGRATE_PERSIST_DEST; -- 1.7.1.250.g7d1e8

3 4

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

19 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 19 May '10

19 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 19 May '10

19 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 19 May '10

19 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 19 May '10

19 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 19 May '10

19 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 19 May '10

19 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 19 May '10

19 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 19 May '10

19 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