Bug 999057

+++ This bug was initially created as a clone of Bug #996050 +++

Description of problem:
virsh undefine command with '--remove-all-storage' or '--storage' option when storage volume is unmanaged will cause memory leak.

Version-Release number of selected component (if applicable):
libvirt-0.10.2-21.el6.x86_64 or libvirt-0.10.2-18.el6_4.9.x86_64
kernel-2.6.32-410.el6.x86_64
qemu-kvm-rhev-0.12.1.2-2.386.el6.x86_64

How reproducible:
always

Steps to Reproduce:
1. create a new volume
# dd if=/dev/null of=/root/foo bs=1 count=1 seek=10M

2. define a domain using that volume
# virsh define foo.xml                   (disk source file points to '/root/foo')

3. check whether the new volume is managed by libvirt
# virsh vol-list default                 (make sure 'foo' volume don't exists)

4. check memory leak when undefined with remove-all-storage or --storage option
# valgrind -v --leak-check=full virsh undefine foo --remove-all-storage
or
# valgrind -v --leak-check=full virsh undefine foo --storage /root/foo


Actual results:

1. virsh output
Storage volume 'hda'(/root/foo) is not managed by libvirt. Remove it manually.
Domain foo has been undefined


2. valgrind result
==19011== 4 bytes in 1 blocks are definitely lost in loss record 1 of 82
==19011==    at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==19011==    by 0x38F0881041: strdup (in /lib64/libc-2.12.so)
==19011==    by 0x4C88054: virXPathString (xml.c:89)
==19011==    by 0x414EA1: cmdUndefine (virsh-domain.c:2624)
==19011==    by 0x40D200: vshCommandRun (virsh.c:1652)
==19011==    by 0x410964: main (virsh.c:3073)
==19011==
==19011== 10 bytes in 1 blocks are definitely lost in loss record 14 of 82
==19011==    at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==19011==    by 0x38F0881041: strdup (in /lib64/libc-2.12.so)
==19011==    by 0x40EB00: _vshStrdup (virsh.c:129)
==19011==    by 0x40FC28: vshStringToArray (virsh.c:182)
==19011==    by 0x414A66: cmdUndefine (virsh-domain.c:2609)
==19011==    by 0x40D200: vshCommandRun (virsh.c:1652)
==19011==    by 0x410964: main (virsh.c:3073)
==19011==
==19011== 10 bytes in 1 blocks are definitely lost in loss record 15 of 82
==19011==    at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==19011==    by 0x38F0881041: strdup (in /lib64/libc-2.12.so)
==19011==    by 0x4C88054: virXPathString (xml.c:89)
==19011==    by 0x414EBF: cmdUndefine (virsh-domain.c:2627)
==19011==    by 0x40D200: vshCommandRun (virsh.c:1652)
==19011==    by 0x410964: main (virsh.c:3073)
==19011==
==19011== LEAK SUMMARY:
==19011==    definitely lost: 24 bytes in 3 blocks

Expected results:
fix memory leak.

--- Additional comment from Peter Krempa on 2013-08-15 18:41:20 CEST ---

First fix for some of the issues described above was pushed upstream as

commit 5075248ac9e60dd73da3487f83be9aa188200688
Author: Peter Krempa <pkrempa>
Date:   Mon Aug 12 16:09:53 2013 +0200

    virsh-domain: Fix memleak in cmdUndefine with storage
    
    When undefining a domain with storage when the volume isn't managed by
    libvirt the name and path strings weren't freed properly.
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=996050

--- Additional comment from Peter Krempa on 2013-08-20 18:01:06 CEST ---

commit 04898f60d2b509c4d106bf7e1ee269acc240b93d
Author: Peter Krempa <pkrempa>
Date:   Thu Aug 15 16:48:20 2013 +0200

    virsh: Don't leak list of volumes when undefining domain with storage
    
    Use the new semantics of vshStringToArray to avoid leaking the array of
    volumes to be deleted. The array would be leaked in case the first
    volume was found in the domain definition. Also refactor the code a bit
    to sanitize naming of variables hoding arrays and dimensions of the
    arrays.
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=996050