May 2010 - Devel - libvirt List Archives

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

19 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

[libvirt] handling qemuMonitorAddDevice failure: missing drive_del function?
by Jim Meyering 19 May '10

19 May '10

In src/qemu/qemu_driver.c, coverity gripes (rightly) about this: 6912 qemuDomainObjEnterMonitorWithDriver(driver, vm); 6913 if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { 6914 ret = qemuMonitorAddDrive(priv->mon, drivestr); 6915 if (ret == 0) No check of the return value of "qemuMonitorAddDevice(priv->mon, devstr)". Calling function "qemuMonitorAddDevice" without checking return value. 6916 qemuMonitorAddDevice(priv->mon, devstr); 6917 /* XXX remove the drive upon fail */ 6918 } else { Does anyone have a preference on how to deal with it while we wait for a drive-removal function? I think it deserves at least a diagnostic. I suppose this comment is still relevant: if (ret == 0) ret = qemuMonitorAddDevice(priv->mon, devstr); /* XXX should call 'drive_del' on error but this does not exist yet */ This XXX marks the same problem -- though note that coverity could not possibly see this one.

2 3

[libvirt] [PATCH] Init script for suspending/resuming guests on shutdown/boot
by Jiri Denemark 19 May '10

19 May '10

Example output during shutdown: Running guests on default URI: console, rhel6-1, rhel5-64 Running guests on lxc:/// URI: lxc-shell Running guests on xen:/// URI: error: no hypervisor driver available for xen:/// error: failed to connect to the hypervisor Running guests on vbox+tcp://orkuz/system URI: no running guests. Suspending guests on default URI... Suspending console: done Suspending rhel6-1: done Suspending rhel5-64: done Suspending guests on lxc:/// URI... Suspending lxc-shell: error: Failed to save domain 9cba8bfb-56f4-6589-2d12-8a58c886dd3b state error: this function is not supported by the hypervisor: virDomainManagedSave Note, the "Suspending $guest: " shows progress during the suspend phase if domjobinfo gives meaningful output. Example output during boot: Resuming guests on default URI... Resuming guest rhel6-1: done Resuming guest rhel5-64: done Resuming guest console: done Resuming guests on lxc:/// URI... Resuming guest lxc-shell: already active Configuration used for generating the examples above: URIS='default lxc:/// xen:/// vbox+tcp://orkuz/system' The script uses /var/lib/libvirt/libvirt-guests files to note all active guest it should try to resume on next boot. It's content looks like: default 7f8b9d93-30e1-f0b9-47a7-cb408482654b 085b4c95-5da2-e8e1-712f-6ea6a4156af2 fb4d8360-5305-df3a-2da1-07d682891b8c lxc:/// 9cba8bfb-56f4-6589-2d12-8a58c886dd3b --- daemon/Makefile.am | 16 +++- daemon/libvirt-guests.init.in | 194 +++++++++++++++++++++++++++++++++++++++++ daemon/libvirt-guests.sysconf | 3 + 3 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 daemon/libvirt-guests.init.in create mode 100644 daemon/libvirt-guests.sysconf diff --git a/daemon/Makefile.am b/daemon/Makefile.am index a82e9a9..ed469bf 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -29,6 +29,8 @@ EXTRA_DIST = \ libvirtd.lxc.logrotate.in \ libvirtd.uml.logrotate.in \ test_libvirtd.aug \ + libvirt-guests.init.in \ + libvirt-guests.sysconf \ $(AVAHI_SOURCES) \ $(DAEMON_SOURCES) @@ -216,21 +218,27 @@ install-logrotate: $(LOGROTATE_CONFS) $(INSTALL_DATA) libvirtd.uml.logrotate $(DESTDIR)$(sysconfdir)/logrotate.d/libvirtd.uml if LIBVIRT_INIT_SCRIPT_RED_HAT -install-init: libvirtd.init +install-init: libvirtd.init libvirt-guests.init mkdir -p $(DESTDIR)$(sysconfdir)/rc.d/init.d $(INSTALL_SCRIPT) libvirtd.init \ $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirtd + $(INSTALL_SCRIPT) libvirt-guests.init \ + $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirt-guests mkdir -p $(DESTDIR)$(sysconfdir)/sysconfig $(INSTALL_SCRIPT) $(srcdir)/libvirtd.sysconf \ $(DESTDIR)$(sysconfdir)/sysconfig/libvirtd + $(INSTALL_SCRIPT) $(srcdir)/libvirt-guests.sysconf \ + $(DESTDIR)$(sysconfdir)/sysconfig/libvirt-guests uninstall-init: rm -f $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirtd \ - $(DESTDIR)$(sysconfdir)/sysconfig/libvirtd + $(DESTDIR)$(sysconfdir)/sysconfig/libvirtd \ + $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirt-guests \ + $(DESTDIR)$(sysconfdir)/sysconfig/libvirt-guests -BUILT_SOURCES += libvirtd.init +BUILT_SOURCES += libvirtd.init libvirt-guests.init -libvirtd.init: libvirtd.init.in +%.init: %.init.in $(AM_V_GEN)sed \ -e s!\@localstatedir\@!@localstatedir@!g \ -e s!\@sbindir\@!@sbindir@!g \ diff --git a/daemon/libvirt-guests.init.in b/daemon/libvirt-guests.init.in new file mode 100644 index 0000000..e930115 --- /dev/null +++ b/daemon/libvirt-guests.init.in @@ -0,0 +1,194 @@ +#!/bin/sh + +# the following is the LSB init header see +# http://www.linux-foundation.org/spec//booksets/LSB-Core-generic/LSB-Core-ge… +# +### BEGIN INIT INFO +# Provides: libvirt-guests +# Required-Start: libvirtd +# Required-Stop: libvirtd +# Default-Start: 3 4 5 +# Short-Description: suspend/resume libvirt guests on shutdown/boot +# Description: This is a script for suspending active libvirt guests +# on shutdown and resuming them on next boot +# See http://libvirt.org +### END INIT INFO + +# the following is chkconfig init header +# +# libvirt-guests: suspend/resume libvirt guests on shutdown/boot +# +# chkconfig: 345 98 02 +# description: This is a script for suspending active libvirt guests +# on shutdown and resuming them on next boot +# See http://libvirt.org +# + +sysconfdir=@sysconfdir@ +localstatedir=@localstatedir@ + +# Source function library. +. $sysconfdir/rc.d/init.d/functions + +URIS=default + +test -f $sysconfdir/sysconfig/libvirt-guests && . $sysconfdir/sysconfig/libvirt-guests + +LISTFILE=$localstatedir/lib/libvirt/libvirt-guests + +RETVAL=0 + +retval() { + "$@" + if [ $? -ne 0 ]; then + RETVAL=1 + return 1 + else + return 0 + fi +} + +run_virsh() { + uri=$1 + shift + + if [ "x$uri" = xdefault ]; then + conn= + else + conn="-c $uri" + fi + + virsh $conn "$@" +} + +run_virsh_c() { + ( export LC_ALL=C; run_virsh "$@" ) +} + +list_guests() { + uri=$1 + + list=`run_virsh_c $uri list` + if [ $? -ne 0 ]; then + RETVAL=1 + return 1 + fi + + for id in `echo "$list" | awk 'NR > 2 {print $1}'`; do + run_virsh_c $uri dominfo $id | awk '/^UUID:/{print $2}' + done +} + +guest_name() { + uri=$1 + uuid=$2 + + name=`run_virsh_c $uri dominfo $uuid 2>/dev/null | \ + awk '/^Name:/{print $2}'` + [ -n "$name" ] || name=$uuid + + echo "$name" +} + +guest_is_on() { + uri=$1 + uuid=$2 + + id=`run_virsh_c $uri dominfo $uuid 2>/dev/null | \ + awk '/^Id:/{print $2}'` + + [ -n "$id" ] && ! [ "x$id" = x- ] +} + +start() { + while read uri list; do + configured=false + for confuri in $URIS; do + if [ $confuri = $uri ]; then + configured=true + break + fi + done + if ! $configured; then + echo $"Ignoring guests on $uri URI" + continue + fi + + echo $"Resuming guests on $uri URI..." + for guest in $list; do + name=`guest_name $uri $guest` + echo -n $"Resuming guest $name: " + if retval guest_is_on $uri $guest; then + echo $"already active" + else + retval run_virsh $uri start "$name" >/dev/null && echo $"done" + fi + done + done <$LISTFILE + + rm -f $LISTFILE +} + +stop() { + >$LISTFILE + for uri in $URIS; do + echo -n $"Running guests on $uri URI: " + list=`list_guests $uri` + if [ $? -eq 0 ]; then + empty=true + for uuid in $list; do + $empty || echo -n ", " + echo -n `guest_name $uri $uuid` + empty=false + done + if $empty; then + echo $"no running guests." + else + echo + echo $uri $list >>$LISTFILE + fi + fi + done + + while read uri list; do + echo $"Suspending guests on $uri URI..." + for guest in $list; do + name=`guest_name $uri $guest` + label=$"Suspending $name: " + echo -n "$label" + run_virsh $uri managedsave $guest >/dev/null & + virsh_pid=$! + while true; do + sleep .5 + kill -0 $virsh_pid >&/dev/null || break + progress=`run_virsh_c $uri domjobinfo $guest 2>/dev/null | \ + awk '/^Data processed:/{print $3, $4}'` + if [ -n "$progress" ]; then + printf '\r%s%12s ' "$label" "$progress" + else + printf '\r%s%-12s ' "$label" "..." + fi + done + retval wait $virsh_pid && printf '\r%s%-12s\n' "$label" $"done" + done + done <$LISTFILE +} + +gueststatus() { + for uri in $URIS; do + echo "* $uri URI:" + retval run_virsh $uri list || echo + done +} + +# See how we were called. +case "$1" in + start|stop|gueststatus) + $1 + ;; + *) + echo $"Usage: $0 {start|stop|gueststatus}" + exit 3 + ;; +esac +exit $RETVAL diff --git a/daemon/libvirt-guests.sysconf b/daemon/libvirt-guests.sysconf new file mode 100644 index 0000000..eb70ebf --- /dev/null +++ b/daemon/libvirt-guests.sysconf @@ -0,0 +1,3 @@ +# specify URIs to check for running guests +# example: URIS='default xen:/// vbox+tcp://host/system lxc:///' +#URIS=default -- 1.7.1

3 5

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

19 May '10

Fix a minor leak: >From 0fc88bfcae06346e871d2a4a89febb160bc0857e 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

[libvirt] managed save and autoboot
by Daniel Berteaud 18 May '10

18 May '10

Hi everyone. I'm trying to auto-suspend my guests when the host shutdown using the managedsave function introduced in libvirt 0.8.0. If I manually managedsave all my guest, then manually start all of the guest with virsh start <guest>, everything works as expected, the saved state is restored. But there's a problem with the autoboot option: - I configure a guest to automatically start on host boot (virsh autostart guest) - I save this guest (virsh managedsave guest) - I restart libvirt to simulate a host reboot The guest is started normally, instead of loading the saved state (and the saved state is still present in /var/lib/libvirt/qemu/save/guest.save, which means next manual boot will probably fails) So, it looks like there's a difference in the way libvirt starts guests between autoboot and a manual virsh start. Anybody else have this issue ? Is this a known issue ? I'm running libvirt 0.8.1 on a CentOS 5.5 x86_64 box Regards, Daniel -- Daniel Berteaud FIREWALL-SERVICES SARL. Société de Services en Logiciels Libres Technopôle Montesquieu 33650 MARTILLAC Tel : 05 56 64 15 32 Fax : 05 56 64 15 32 Mail: daniel(a)firewall-services.com Web : http://www.firewall-services.com

4 6

[libvirt] [PATCH] linuxNodeInfoCPUPopulate: avoid used-uninitialized via a test
by Jim Meyering 18 May '10

18 May '10

See the comment: >From 1b200ba22d742e685de0b9853c8cd276df8e129f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 11:58:32 +0200 Subject: [PATCH] linuxNodeInfoCPUPopulate: avoid used-uninitialized via a test * tests/nodeinfotest.c (linuxTestCompareFiles): Don't use nodeinfo->member uninitialized. linuxNodeInfoCPUPopulate requires that some of its nodeinfo members (including threads) be initialized upon input. The nodeinfotest.c program lacked the initialization, while the only other use (nodeGetInfo) did perform it. It's not trivial to move the initialization into the function, since nodeGetInfo sets at least one member after clearing the buffer but before calling linuxNodeInfoCPUPopulate. --- tests/nodeinfotest.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c index 9aeb459..cb92215 100644 --- a/tests/nodeinfotest.c +++ b/tests/nodeinfotest.c @@ -40,6 +40,8 @@ static int linuxTestCompareFiles(const char *cpuinfofile, const char *outputfile cpuinfo = fopen(cpuinfofile, "r"); if (!cpuinfo) return -1; + + memset(nodeinfo, 0, sizeof(*nodeinfo)); if (linuxNodeInfoCPUPopulate(cpuinfo, &nodeinfo) < 0) { fclose(cpuinfo); return -1; -- 1.7.1.250.g7d1e8

2 2

[libvirt] [PATCH] initialize "meta" in virStorageFileGetMetadata, not in each caller
by Jim Meyering 18 May '10

18 May '10

Here's proactive clean-up that is IMHO required. Otherwise, it's just too easy not to realize that "meta" must be cleared before each and every call to virStorageFileGetMetadata and virStorageFileGetMetadataFromFD. Besides, any change that adds just 2 lines and removes 11, in addition to making the code harder to abuse must be an improvement. >From 837732f1e307208b52721ffba83102e0edc361a7 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 07:53:31 +0200 Subject: [PATCH] initialize "meta" in virStorageFileGetMetadata, not in each caller Do not require each caller of virStorageFileGetMetadata and virStorageFileGetMetadataFromFD to first clear the storage of the "meta" buffer. Instead, initialize that storage in virStorageFileGetMetadataFromFD. * src/util/storage_file.c (virStorageFileGetMetadataFromFD): Clear "meta" here, not before each of the following callers. * src/qemu/qemu_driver.c (qemuSetupDiskCgroup): Don't clear "meta" here. (qemuTeardownDiskCgroup): Likewise. * src/qemu/qemu_security_dac.c (qemuSecurityDACSetSecurityImageLabel): Likewise. * src/security/security_selinux.c (SELinuxSetSecurityImageLabel): Likewise. * src/security/virt-aa-helper.c (get_files): Likewise. --- src/qemu/qemu_driver.c | 5 ----- src/qemu/qemu_security_dac.c | 2 -- src/security/security_selinux.c | 2 -- src/security/virt-aa-helper.c | 2 -- src/util/storage_file.c | 2 ++ 5 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e025987..0e70b1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2982,8 +2982,6 @@ static int qemuSetupDiskCgroup(virCgroupPtr cgroup, } } - memset(&meta, 0, sizeof(meta)); - rc = virStorageFileGetMetadata(path, &meta); if (path != disk->src) @@ -3030,8 +3028,6 @@ static int qemuTeardownDiskCgroup(virCgroupPtr cgroup, } } - memset(&meta, 0, sizeof(meta)); - rc = virStorageFileGetMetadata(path, &meta); if (path != disk->src) @@ -9316,7 +9312,6 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, } /* Probe for magic formats */ - memset(&meta, 0, sizeof(meta)); if (virStorageFileGetMetadataFromFD(path, fd, &meta) < 0) goto cleanup; diff --git a/src/qemu/qemu_security_dac.c b/src/qemu/qemu_security_dac.c index a816441..52150f7 100644 --- a/src/qemu/qemu_security_dac.c +++ b/src/qemu/qemu_security_dac.c @@ -115,8 +115,6 @@ qemuSecurityDACSetSecurityImageLabel(virDomainObjPtr vm ATTRIBUTE_UNUSED, virStorageFileMetadata meta; int ret; - memset(&meta, 0, sizeof(meta)); - ret = virStorageFileGetMetadata(path, &meta); if (path != disk->src) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 669ef42..d90e17c 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -454,8 +454,6 @@ SELinuxSetSecurityImageLabel(virDomainObjPtr vm, virStorageFileMetadata meta; int ret; - memset(&meta, 0, sizeof(meta)); - ret = virStorageFileGetMetadata(path, &meta); if (path != disk->src) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 88cdc9d..c66f107 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -830,8 +830,6 @@ get_files(vahControl * ctl) do { virStorageFileMetadata meta; - memset(&meta, 0, sizeof(meta)); - ret = virStorageFileGetMetadata(path, &meta); if (path != ctl->def->disks[i]->src) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index a07bedc..b3ae905 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -275,6 +275,8 @@ virStorageFileGetMetadataFromFD(const char *path, unsigned char head[20*512]; /* vmdk4GetBackingStore needs this much. */ int len, i; + memset(meta, 0, sizeof (*meta)); + /* If all else fails, call it a raw file */ meta->format = VIR_STORAGE_FILE_RAW; -- 1.7.1.250.g7d1e8

2 1

[libvirt] [PATCH] (qemuTeardownDiskCgroup): avoid dead code
by Jim Meyering 18 May '10

18 May '10

It's a good thing the latter while loop condition could never be true -- otherwise it'd be an infloop. >From 319fd4536555d68316a2cb7967f1093be8de3945 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Mon, 17 May 2010 22:50:21 +0200 Subject: [PATCH] (qemuTeardownDiskCgroup): avoid dead code * src/qemu/qemu_driver.c (qemuTeardownDiskCgroup): Convert bogus while...while loop to the intended do...while loop. --- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 16a9646..3e44407 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2981,65 +2981,65 @@ static int qemuSetupDiskCgroup(virCgroupPtr cgroup, goto cleanup; } } memset(&meta, 0, sizeof(meta)); rc = virStorageFileGetMetadata(path, &meta); if (path != disk->src) VIR_FREE(path); path = NULL; if (rc < 0) goto cleanup; path = meta.backingStore; } while (path != NULL); ret = 0; cleanup: return ret; } static int qemuTeardownDiskCgroup(virCgroupPtr cgroup, virDomainObjPtr vm, virDomainDiskDefPtr disk) { char *path = disk->src; int ret = -1; - while (path != NULL) { + do { virStorageFileMetadata meta; int rc; VIR_DEBUG("Process path %s for disk", path); rc = virCgroupDenyDevicePath(cgroup, path); if (rc != 0) { /* Get this for non-block devices */ if (rc == -EINVAL) { VIR_DEBUG("Ignoring EINVAL for %s", path); } else { virReportSystemError(-rc, _("Unable to deny device %s for %s"), path, vm->def->name); if (path != disk->src) VIR_FREE(path); goto cleanup; } } memset(&meta, 0, sizeof(meta)); rc = virStorageFileGetMetadata(path, &meta); if (path != disk->src) VIR_FREE(path); path = NULL; if (rc < 0) goto cleanup; path = meta.backingStore; } while (path != NULL); -- 1.7.1.250.g7d1e8

2 4

[libvirt] [PATCH] ebiptablesWriteToTempFile: don't close a negative file descriptor
by Jim Meyering 18 May '10

18 May '10

If mkstemp fails here, we end up closing a negative FD: int fd = mkstemp(filename); if (fd < 0) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot create temporary file")); goto err_exit; } Here's the fix: >From b7c6593b3a8b59d49b492cd45fbf5f9c706bb78f Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 10:11:23 +0200 Subject: [PATCH] ebiptablesWriteToTempFile: don't close a negative file descriptor * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesWriteToTempFile): Skip the close if "fd" is negative. --- src/nwfilter/nwfilter_ebiptables_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 63bcbd7..ae21906 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2245,7 +2245,8 @@ ebiptablesWriteToTempFile(const char *string) { err_exit: VIR_FREE(header); - close(fd); + if (fd >= 0) + close(fd); unlink(filename); return NULL; } -- 1.7.1.250.g7d1e8

2 1

[libvirt] [PATCH] virNWFilterDefParseXML: avoid leak on error paths
by Jim Meyering 18 May '10

18 May '10

In this function, static virNWFilterDefPtr virNWFilterDefParseXML(xmlXPathContextPtr ctxt) { we allocate space for the result we expect to return: if (VIR_ALLOC(ret) < 0) { virReportOOMError(); return NULL; } and later, ... if (VIR_REALLOC_N(ret->filterEntries, ret->nentries+1) < 0) { VIR_FREE(entry); virReportOOMError(); goto cleanup; } Hence, the lack of anything to free "ret" when this function returns NULL constitutes a leak. Here's the patch: >From 4c13990a15b33f03595d58b46b6e34e03bfffa65 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Tue, 18 May 2010 12:05:53 +0200 Subject: [PATCH] virNWFilterDefParseXML: avoid leak on error paths * src/conf/nwfilter_conf.c (virNWFilterDefParseXML): Also free "ret" via cleanup. --- src/conf/nwfilter_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index ea73a33..fc6d461 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -1767,6 +1767,7 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) { return ret; cleanup: + virNWFilterDefFree(ret); VIR_FREE(chain); VIR_FREE(uuid); return NULL; -- 1.7.1.250.g7d1e8

2 1