[libvirt] [PATCH] virsh: Plug memory leak on cmdUndefine
From: Alex Jia <ajia(a)redhat.com>
Detected by valgrind. Leak is introduced in commit 3bb6bcf.
Free 'vol' memory before allocating memory, the codes will miss one time
free when 'vol_i = nvolumes' in for loop, so plug memory leak.
* tools/virsh.c: fix memory leak on cmdUndefine.
* How to reproduce?
% dd if=/dev/null of=/var/lib/libvirt/images/foo bs=1 count=1 seek=10M
% virsh define foo.xml (disk source file points to
'/var/lib/libvirt/images/foo')
% virsh vol-clone foo foo-clone default (the original guest name is 'foo')
% virsh pool-refresh default
% virsh vol-list default (make sure 'foo-clone' volume exists)
% virsh define foo-clone.xml (disk source file points to
'/var/lib/libvirt/images/foo-clone')
% valgrind -v --leak-check=full virsh undefine foo-clone --remove-all-storage
* Actual results:
1. virsh output
Domain foo-clone has been undefined
Volume '/var/lib/libvirt/images/foo-clone' removed.
error: Failed to disconnect from the hypervisor, 1 leaked reference(s)
2. valgrind result
==6515== 92 (40 direct, 52 indirect) bytes in 1 blocks are definitely lost in loss record
46 of 69
==6515== at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==6515== by 0x4C89B71: virAlloc (memory.c:101)
==6515== by 0x4CFCACE: virGetStorageVol (datatypes.c:724)
==6515== by 0x4D4A8E0: remoteStorageVolLookupByPath (remote_driver.c:4664)
==6515== by 0x4D07153: virStorageVolLookupByPath (libvirt.c:12508)
==6515== by 0x4270E6: cmdUndefine (virsh.c:2828)
==6515== by 0x4151B6: vshCommandRun (virsh.c:17693)
==6515== by 0x4264D3: main (virsh.c:19270)
==6515==
==6515== LEAK SUMMARY:
==6515== definitely lost: 40 bytes in 1 blocks
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=786674
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/virsh.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index c8fd448..73c2192 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2787,10 +2787,6 @@ out:
ctxt->node = vol_nodes[vol_i];
VIR_FREE(target);
VIR_FREE(source);
- if (vol) {
- virStorageVolFree(vol);
- vol = NULL;
- }
/* get volume source and target paths */
if (!(target = virXPathString("string(./target/@dev)", ctxt))) {
@@ -2852,6 +2848,10 @@ out:
vol_del_failed = true;
}
vshPrint(ctl, _("Volume '%s' removed.\n"),
volume_tok?volume_tok:source);
+
+ /* cleanup */
+ virStorageVolFree(vol);
+ vol = NULL;
}
/* print volumes specified by user that were not found in domain definition */
--
1.7.1