[libvirt] [PATCH] virsh: Plug memory leak on cmdUndefine

ajia at redhat.com ajia at redhat.com
Thu Feb 2 06:25:25 UTC 2012


From: Alex Jia <ajia at 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 at 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




More information about the libvir-list mailing list