September 2009 - Devel - libvirt List Archives

[libvirt] [PATCH] Fix more OOM handling bugs
by Daniel P. Berrange 04 Sep '09

04 Sep '09

* src/qemu_conf.c: Fix leak of values upon OOM * src/xend_internal.c: Fix missing check for OOM failure * tests/qemuargv2xmltest.c, tests/qemuxml2argvtest.c: Free stateDir upon exit to avoid leak --- src/qemu_conf.c | 21 ++++++++++++++++++--- src/xend_internal.c | 6 ++++++ tests/qemuargv2xmltest.c | 1 + tests/qemuxml2argvtest.c | 1 + 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/qemu_conf.c b/src/qemu_conf.c index 22f5edd..ad41104 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1722,6 +1722,8 @@ int qemudBuildCommandLine(virConnectPtr conn, continue; } + ADD_ARG_SPACE; + if (idx < 0) { qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported disk type '%s'"), disk->dst); @@ -1773,7 +1775,10 @@ int qemudBuildCommandLine(virConnectPtr conn, optstr = virBufferContentAndReset(&opt); - ADD_ARG_LIT("-drive"); + if ((qargv[qargc++] = strdup("-drive")) == NULL) { + VIR_FREE(optstr); + goto no_memory; + } ADD_ARG(optstr); } } else { @@ -1829,6 +1834,7 @@ int qemudBuildCommandLine(virConnectPtr conn, net->vlan = i; + ADD_ARG_SPACE; if ((qemuCmdFlags & QEMUD_CMD_FLAG_NET_NAME) && qemuAssignNetNames(def, net) < 0) goto no_memory; @@ -1836,9 +1842,14 @@ int qemudBuildCommandLine(virConnectPtr conn, if (qemuBuildNicStr(conn, net, NULL, ',', net->vlan, &nic) < 0) goto error; - ADD_ARG_LIT("-net"); + if ((qargv[qargc++] = strdup("-net")) == NULL) { + VIR_FREE(nic); + goto no_memory; + } ADD_ARG(nic); + + ADD_ARG_SPACE; if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK || net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { int tapfd = qemudNetworkIfaceConnect(conn, driver, net, qemuCmdFlags); @@ -1862,7 +1873,10 @@ int qemudBuildCommandLine(virConnectPtr conn, goto error; } - ADD_ARG_LIT("-net"); + if ((qargv[qargc++] = strdup("-net")) == NULL) { + VIR_FREE(host); + goto no_memory; + } ADD_ARG(host); VIR_FREE(tapfd_name); @@ -2230,6 +2244,7 @@ static int qemuStringToArgvEnv(const char *args, goto no_memory; for (i = 0 ; i < envend ; i++) { progenv[i] = arglist[i]; + arglist[i] = NULL; } progenv[i] = NULL; } diff --git a/src/xend_internal.c b/src/xend_internal.c index 99847b0..20ddb89 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -5212,6 +5212,9 @@ xenDaemonFormatSxprChr(virConnectPtr conn, break; } + if (virBufferError(buf)) + return -1; + return 0; } @@ -5535,6 +5538,9 @@ xenDaemonFormatSxprSound(virConnectPtr conn, virBufferVSprintf(buf, "%s%s", i ? "," : "", str); } + if (virBufferError(buf)) + return -1; + return 0; } diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index 7861520..4a92280 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -223,6 +223,7 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + free(driver.stateDir); virCapabilitiesFree(driver.caps); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6f25e7d..2f91288 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -270,6 +270,7 @@ mymain(int argc, char **argv) DO_TEST_FULL("restore-v2", QEMUD_CMD_FLAG_MIGRATE_QEMU_EXEC, "exec:cat"); DO_TEST_FULL("migrate", QEMUD_CMD_FLAG_MIGRATE_QEMU_TCP, "tcp:10.0.0.1:5000"); + free(driver.stateDir); virCapabilitiesFree(driver.caps); return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); -- 1.6.2.5

2 1

[libvirt] [PATCH] Fix logging buffer overrun read
by Daniel P. Berrange 04 Sep '09

04 Sep '09

* src/logging.c: Fix buffer offset in logging read --- src/logging.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/logging.c b/src/logging.c index e46e2be..07c2b0e 100644 --- a/src/logging.c +++ b/src/logging.c @@ -240,7 +240,7 @@ static void virLogStr(const char *str, int len) { tmp = LOG_BUFFER_SIZE - virLogEnd; memcpy(&virLogBuffer[virLogEnd], str, tmp); virLogBuffer[LOG_BUFFER_SIZE] = 0; - memcpy(&virLogBuffer[0], &str[len], len - tmp); + memcpy(&virLogBuffer[0], &str[tmp], len - tmp); virLogEnd = len - tmp; } else { memcpy(&virLogBuffer[virLogEnd], str, len); -- 1.6.2.5

3 2

[libvirt] [PATCH] uml_conf.c: don't return an uninitialized pointer
by Jim Meyering 04 Sep '09

04 Sep '09

Another "real" bug: >From 6697607bf0b023ffb692576b31d652d10719b08a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 12:05:52 +0200 Subject: [PATCH] uml_conf.c: don't return an uninitialized pointer * src/uml_conf.c (umlBuildCommandLineChr): Initialize "ret" also in the final switch cases. --- src/uml_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/uml_conf.c b/src/uml_conf.c index 838fedd..2e9c25c 100644 --- a/src/uml_conf.c +++ b/src/uml_conf.c @@ -331,6 +331,7 @@ umlBuildCommandLineChr(virConnectPtr conn, default: umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("unsupported chr device type %d"), def->type); + ret = NULL; break; } -- 1.6.4.2.395.ge3d52

3 9

[libvirt] three more clang-inspired dead-store-fixing patches
by Jim Meyering 04 Sep '09

04 Sep '09

>From a2d03c987bb724283207dbeef873178c08a6c4c5 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:14:48 +0200 Subject: [PATCH 1/3] storage_backend_logical.c: appease clang: remove useless increment * src/storage_backend_logical.c (virStorageBackendLogicalBuildPool): Don't increment "n" when we won't use the result. --- src/storage_backend_logical.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c index bc40dd7..43117e8 100644 --- a/src/storage_backend_logical.c +++ b/src/storage_backend_logical.c @@ -437,7 +437,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn, goto cleanup; } - vgargv[n++] = NULL; + vgargv[n] = NULL; /* Now create the volume group itself */ if (virRun(conn, vgargv, NULL) < 0) -- 1.6.4.2.395.ge3d52 >From b72ed1ba5d5e6444f000fd9be706a51a90dd2292 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:23:51 +0200 Subject: [PATCH 2/3] openvz_conf.c: Remove dead store to copy_fd * src/openvz_conf.c (openvz_copyfile): Remove unused assignment. --- src/openvz_conf.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index 41c6684..2f0471c 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -692,7 +692,6 @@ openvz_copyfile(char* from_path, char* to_path) fd = -1; if (close(copy_fd) < 0) goto error; - copy_fd = -1; return 0; -- 1.6.4.2.395.ge3d52 >From a8d2eca13635578084e3c2cc2093562d9a8fcfed Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 18:25:03 +0200 Subject: [PATCH 3/3] eventtest.c: detect write failure and avoid dead stores * tests/eventtest.c (mymain): Exit nonzero upon write failure. This also avoids several dead stores of the form ret = safewrite... --- tests/eventtest.c | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/eventtest.c b/tests/eventtest.c index d25381d..ef37ff1 100644 --- a/tests/eventtest.c +++ b/tests/eventtest.c @@ -248,7 +248,6 @@ resetAll(void) static int mymain(int argc, char **argv) { - int ret = 0; char *progname; int i; pthread_t eventThread; @@ -304,7 +303,8 @@ mymain(int argc, char **argv) /* First time, is easy - just try triggering one of our * registered handles */ startJob("Simple write", &test); - ret = safewrite(handles[1].pipeFD[1], &one, 1); + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(1, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -314,7 +314,8 @@ mymain(int argc, char **argv) * try triggering another handle */ virEventRemoveHandleImpl(handles[0].watch); startJob("Deleted before poll", &test); - ret = safewrite(handles[1].pipeFD[1], &one, 1); + if (safewrite(handles[1].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(1, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -346,8 +347,9 @@ mymain(int argc, char **argv) * about */ startJob("Deleted during dispatch", &test); handles[2].delete = handles[3].watch; - ret = safewrite(handles[2].pipeFD[1], &one, 1); - ret = safewrite(handles[3].pipeFD[1], &one, 1); + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1 + || safewrite(handles[3].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(2, -1) != EXIT_SUCCESS) return EXIT_FAILURE; @@ -356,7 +358,8 @@ mymain(int argc, char **argv) /* Extreme fun, lets delete ourselves during dispatch */ startJob("Deleted during dispatch", &test); handles[2].delete = handles[2].watch; - ret = safewrite(handles[2].pipeFD[1], &one, 1); + if (safewrite(handles[2].pipeFD[1], &one, 1) != 1) + return EXIT_FAILURE; if (finishJob(2, -1) != EXIT_SUCCESS) return EXIT_FAILURE; -- 1.6.4.2.395.ge3d52

2 2

[libvirt] [PATCH] Added Storage Volume support to VIrtualBox
by Pritesh Kothari 04 Sep '09

04 Sep '09

Hi All, I have added support for Storage Volumes in VirtualBox, the patch for the same is attached here. (needs the "little bit cleanup" patch previously posted) Regards, Pritesh

2 1

[libvirt] [PATCH] openvz_conf.c: don't use undefined local, "net"
by Jim Meyering 04 Sep '09

04 Sep '09

This looks like a real bug. >From 170af3320e68a0ac2cfe854fba28abe0e4040d2c Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 11:24:44 +0200 Subject: [PATCH] openvz_conf.c: don't use undefined local, "net" * src/openvz_conf.c (openvzReadNetworkConf): Upon openvzRead... failure, simply return -1, rather than "goto error;" where an uninitialized "net" could be dereferenced. --- src/openvz_conf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/openvz_conf.c b/src/openvz_conf.c index a172fe3..b1cb31a 100644 --- a/src/openvz_conf.c +++ b/src/openvz_conf.c @@ -197,7 +197,7 @@ openvzReadNetworkConf(virConnectPtr conn, openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not read 'IP_ADDRESS' from config for container %d"), veid); - goto error; + return -1; } else if (ret > 0) { token = strtok_r(temp, " ", &saveptr); while (token != NULL) { -- 1.6.4.2.395.ge3d52

3 4

[libvirt] [PATCH] storage_backend.c: assure clang that inputvol can't be NULL
by Jim Meyering 04 Sep '09

04 Sep '09

clang was complaining that a NULL inputvol would be dereferenced in that "could not open..." diagnostic. Since the two sole callers of this function are careful to call it only when inputvol is non-NULL, this is a good case for giving the parameter the nonnull attribute: >From 314278acb021a1f2e63494fab352bd6e9a4a38bb Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Thu, 3 Sep 2009 10:22:57 +0200 Subject: [PATCH] storage_backend.c: assure clang that inputvol can't be NULL * src/storage_backend.c (virStorageBackendCopyToFD): Declare inputvol parameter to be "nonnull". Remove test for non-NULL inputvol. Both callers ensure it's non-NULL. --- src/storage_backend.c | 16 +++++++--------- 1 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/storage_backend.c b/src/storage_backend.c index c818142..8d1187e 100644 --- a/src/storage_backend.c +++ b/src/storage_backend.c @@ -1,7 +1,7 @@ /* * storage_backend.c: internal storage driver backend contract * - * Copyright (C) 2007-2008 Red Hat, Inc. + * Copyright (C) 2007-2009 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -106,7 +106,7 @@ enum { static int virStorageBackendCopyToFD(virConnectPtr conn, virStorageVolDefPtr vol, - virStorageVolDefPtr inputvol, + virStorageVolDefPtr inputvol ATTRIBUTE_NONNULL, int fd, unsigned long long *total, int is_dest_file) @@ -119,13 +119,11 @@ virStorageBackendCopyToFD(virConnectPtr conn, char zerobuf[512]; char *buf = NULL; - if (inputvol) { - if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { - virReportSystemError(conn, errno, - _("could not open input path '%s'"), - inputvol->target.path); - goto cleanup; - } + if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { + virReportSystemError(conn, errno, + _("could not open input path '%s'"), + inputvol->target.path); + goto cleanup; } bzero(&zerobuf, sizeof(zerobuf)); -- 1.6.4.2.395.ge3d52

3 4

[libvirt] [PATCH] little bit cleanup
by Pritesh Kothari 04 Sep '09

04 Sep '09

Hi All, Just cleaned up some code to make the it more readable. The patch is attached here with. Directly applies to the HEAD as of today. Regards, Pritesh

3 2

[libvirt] [PATCH] Minor comment changes.
by Laine Stump 04 Sep '09

04 Sep '09

Fix some minor grammer (and one other) nits in comments that end up in generated API reference documentation. No functional/binary differences. --- src/libvirt.c | 24 ++++++++++++------------ 1 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index ca8e003..ce3d6ba 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -592,9 +592,9 @@ virRegisterNetworkDriver(virNetworkDriverPtr driver) /** * virRegisterInterfaceDriver: - * @driver: pointer to a interface driver block + * @driver: pointer to an interface driver block * - * Register a interface virtualization driver + * Register an interface virtualization driver * * Returns the driver priority or -1 in case of error. */ @@ -5365,7 +5365,7 @@ error: /** * virNetworkGetXMLDesc: * @network: a network object - * @flags: and OR'ed set of extraction flags, not used yet + * @flags: an OR'ed set of extraction flags, not used yet * * Provide an XML description of the network. The description may be reused * later to relaunch the network with virNetworkCreateXML(). @@ -5545,7 +5545,7 @@ error: /** * virInterfaceGetConnect: - * @iface: pointer to a interface + * @iface: pointer to an interface * * Provides the connection pointer associated with an interface. The * reference counter on the connection is not increased by this @@ -5819,7 +5819,7 @@ error: /** * virInterfaceGetName: - * @iface: a interface object + * @iface: an interface object * * Get the public name for that interface * @@ -5842,9 +5842,9 @@ virInterfaceGetName(virInterfacePtr iface) /** * virInterfaceGetMACString: - * @iface: a interface object + * @iface: an interface object * - * Get the MAC for a interface as string. For more information about + * Get the MAC for an interface as string. For more information about * MAC see RFC4122. * * Returns a pointer to the MAC address (in null-terminated ASCII @@ -5867,11 +5867,11 @@ virInterfaceGetMACString(virInterfacePtr iface) /** * virInterfaceGetXMLDesc: - * @iface: a interface object - * @flags: and OR'ed set of extraction flags, not used yet + * @iface: an interface object + * @flags: an OR'ed set of extraction flags, not used yet * * Provide an XML description of the interface. The description may be reused - * later to recreate the interface with virInterfaceCreateXML(). + * later to redefine the interface with virInterfaceDefineXML(). * * Returns a 0 terminated UTF-8 encoded XML instance, or NULL in case of error. * the caller must free() the returned value. @@ -6100,7 +6100,7 @@ error: * This method is typically useful for applications where multiple * threads are using a connection, and it is required that the * connection remain open until all threads have finished using - * it. ie, each new thread using a interface would increment + * it. ie, each new thread using an interface would increment * the reference count. * * Returns 0 in case of success, -1 in case of failure. @@ -6121,7 +6121,7 @@ virInterfaceRef(virInterfacePtr iface) /** * virInterfaceFree: - * @iface: a interface object + * @iface: an interface object * * Free the interface object. The interface itself is unaltered. * The data structure is freed and should not be used thereafter. -- 1.6.2.5

3 2

[libvirt] [PATCH] Fix a memory leak in virsh.
by Laine Stump 04 Sep '09

04 Sep '09

I found this while looking for examples of using virNodeDeviceGetXMLDesc(). AFAIK, *all* of the *GetXMLDesc() functions return a newly allocated chunk of memory that is owned by the caller, who must free it when they're done... --- src/virsh.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/src/virsh.c b/src/virsh.c index 15e0cef..910d860 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -5521,6 +5521,7 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) { const char *name; virNodeDevicePtr device; + char *xml; if (!vshConnectionUsability(ctl, ctl->conn, TRUE)) return FALSE; @@ -5531,7 +5532,14 @@ cmdNodeDeviceDumpXML (vshControl *ctl, const vshCmd *cmd) return FALSE; } - vshPrint(ctl, "%s\n", virNodeDeviceGetXMLDesc(device, 0)); + xml = virNodeDeviceGetXMLDesc(device, 0); + if (!xml) { + virNodeDeviceFree(device); + return FALSE; + } + + vshPrint(ctl, "%s\n", xml); + free(xml); virNodeDeviceFree(device); return TRUE; } -- 1.6.2.5

3 2