Bug 802854 - memory leak when performing persistent network device update (e.g. virsh domif-setlink --persistent)
memory leak when performing persistent network device update (e.g. virsh dom...
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt (Show other bugs)
6.2
Unspecified Unspecified
unspecified Severity unspecified
: rc
: ---
Assigned To: Laine Stump
Virtualization Bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-13 12:13 EDT by Laine Stump
Modified: 2012-06-20 02:50 EDT (History)
6 users (show)

See Also:
Fixed In Version: libvirt-0.9.10-6.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-06-20 02:50:13 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Laine Stump 2012-03-13 12:13:43 EDT
If a device's configuration is updated on a running domain, and that update is requested to be made to the domain's config (most commonly with the command given in the summary of this BZ), a virDomainNetDef will be freed using VIR_FREE() rather than the more correct virDomainNetDefFree(). This results in leakage of all the dynamically allocated data pointed to by the virDomainNetDef.

The following commit fixes the problem:

commit 7a23ba090dd8a374974d3da0180ea3a6ffdc05fb
Author: Laine Stump <laine@laine.org>
Date:   Thu Mar 8 01:46:36 2012 -0500

    qemu: eliminate memory leak in qemuDomainUpdateDeviceConfig
    
    This function was freeing a virDomainNetDef with
    VIR_FREE(). virDomainNetDef is a complex structure with many pointers
    to other dynamically allocated data; to properly free it
    virDomainNetDefFree() must be called instead, otherwise several
    strings (and potentially other things) will be leaked.
Comment 1 Laine Stump 2012-03-16 10:22:42 EDT
A backported fix has been posted to rhvirt-patches for inclusion in the RHEL build of libvirt:

http://post-office.corp.redhat.com/archives/rhvirt-patches/2012-March/msg01464.html
Comment 4 xhu 2012-03-21 04:28:04 EDT
Verify it with libvirt-0.9.10-6.el6.x86_64 and it passed. The steps are as follows:
1 install libvirt-0.9.10-6.el6.src.rpm and found it contains the following patch:
libvirt-qemu-eliminate-memory-leak-in-qemuDomainUpdateDeviceConfig.patch

2 use valgrind to check memory leak:
# valgrind --leak-check=full virsh domif-setlink vr-rhel6-x86_64-kvm 52:54:00:2f:e1:f7 up --persistent
...
==8761== LEAK SUMMARY:
==8761==    definitely lost: 0 bytes in 0 blocks
==8761==    indirectly lost: 0 bytes in 0 blocks
==8761==      possibly lost: 0 bytes in 0 blocks
...

# valgrind --leak-check=full virsh domif-setlink vr-rhel6-x86_64-kvm 52:54:00:2f:e1:f7 down --persistent
...
==8761== LEAK SUMMARY:
==8761==    definitely lost: 0 bytes in 0 blocks
==8761==    indirectly lost: 0 bytes in 0 blocks
==8761==      possibly lost: 0 bytes in 0 blocks
...
Comment 5 Laine Stump 2012-03-21 13:24:21 EDT
The test used to verify the fix is faulty - the memory leak was in libvirtd, not in virsh. Proper verification via valgrind would require running libvirtd under valgrind while running the virsh command separately.
Comment 6 xhu 2012-03-21 23:41:30 EDT
Hi laine,
I retest it and the steps are as follows:
1 use valgrind to start libvirtd:
# service libvirtd stop
Stopping libvirtd daemon:                                  [  OK  ]

# valgrind --leak-check=full libvirtd --daemon >& /root/valgrind.log 2>&1

2 excute domif-setlink command and check valgrind log:
# virsh domif-setlink vr-rhel6-x86_64-kvm 52:54:00:2f:e1:f7 down --persistent
Device updated successfully
# cat /root/valgrind.log
...
==28686== LEAK SUMMARY:
==28686==    definitely lost: 0 bytes in 0 blocks
==28686==    indirectly lost: 0 bytes in 0 blocks
==28686==      possibly lost: 349 bytes in 18 blocks
...

Is it right or enough for the test?
Comment 7 Laine Stump 2012-04-01 23:09:17 EDT
xhu: you need to run libvirtd without daemonizing it. In the end, the best thing is if you can come up with a valgrind command that shows a leak with the unpatched version of libvirtd, and doesn't show the leak with the patched version.

I think ajia has done quite a bit of leak detection in libvirtd using valgrind, so you may want to ask him for advice.
Comment 8 xhu 2012-04-05 01:13:12 EDT
Reproduce it with libvirt-0.9.10-1.el6.x86_64 and the valgrind leak info are as follows:
...
==5033== 1 bytes in 1 blocks are definitely lost in loss record 2 of 774
==5033==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==5033==    by 0x4A06167: realloc (vg_replace_malloc.c:525)
==5033==    by 0x4E72CCB: virReallocN (memory.c:161)
==5033==    by 0x4E65ECD: virCommandRun (command.c:1703)
==5033==    by 0x433110: remoteDispatchAuthPolkitHelper (remote.c:2518)
==5033==    by 0x4F35394: virNetServerProgramDispatch (virnetserverprogram.c:416)
==5033==    by 0x4F36620: virNetServerHandleJob (virnetserver.c:164)
==5033==    by 0x4E7CAAB: virThreadPoolWorker (threadpool.c:144)
==5033==    by 0x4E7C3C8: virThreadHelper (threads-pthread.c:161)
==5033==    by 0x31AC8077F0: start_thread (in /lib64/libpthread-2.12.so)
==5033==    by 0x31AC0E570C: clone (in /lib64/libc-2.12.so)
==5033== 
==5033== 7 bytes in 1 blocks are definitely lost in loss record 80 of 774
==5033==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==5033==    by 0x31B58A67DD: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6)
==5033==    by 0x4E9AE29: virDomainNetDefParseXML (domain_conf.c:3894)
==5033==    by 0x4EACB99: virDomainDefParseXML (domain_conf.c:7742)
==5033==    by 0x4EAFC13: virDomainDefParseNode (domain_conf.c:8437)
==5033==    by 0x4EB058E: virDomainDefParse (domain_conf.c:8387)
==5033==    by 0x4EB086B: virDomainObjCopyPersistentDef (domain_conf.c:13695)
==5033==    by 0x454DA0: qemuDomainModifyDeviceFlags (qemu_driver.c:5628)
==5033==    by 0x4EF2965: virDomainUpdateDeviceFlags (libvirt.c:9417)
==5033==    by 0x42AE18: remoteDispatchDomainUpdateDeviceFlagsHelper (remote_dispatch.h:6260)
==5033==    by 0x4F35394: virNetServerProgramDispatch (virnetserverprogram.c:416)
==5033==    by 0x4F36620: virNetServerHandleJob (virnetserver.c:164)
==5033== 
==5033== 8 bytes in 1 blocks are definitely lost in loss record 144 of 774
==5033==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==5033==    by 0x31B58A67DD: xmlStrndup (in /usr/lib64/libxml2.so.2.7.6)
==5033==    by 0x4E9B037: virDomainNetDefParseXML (domain_conf.c:3844)
==5033==    by 0x4EACB99: virDomainDefParseXML (domain_conf.c:7742)
==5033==    by 0x4EAFC13: virDomainDefParseNode (domain_conf.c:8437)
==5033==    by 0x4EB058E: virDomainDefParse (domain_conf.c:8387)
==5033==    by 0x4EB086B: virDomainObjCopyPersistentDef (domain_conf.c:13695)
==5033==    by 0x454DA0: qemuDomainModifyDeviceFlags (qemu_driver.c:5628)
==5033==    by 0x4EF2965: virDomainUpdateDeviceFlags (libvirt.c:9417)
==5033==    by 0x42AE18: remoteDispatchDomainUpdateDeviceFlagsHelper (remote_dispatch.h:6260)
==5033==    by 0x4F35394: virNetServerProgramDispatch (virnetserverprogram.c:416)
==5033==    by 0x4F36620: virNetServerHandleJob (virnetserver.c:164)
==5033== 
==5033== 15 bytes in 1 blocks are definitely lost in loss record 219 of 774
==5033==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==5033==    by 0x31AC0FEDE7: __vasprintf_chk (in /lib64/libc-2.12.so)
==5033==    by 0x4E7EB03: virVasprintf (stdio2.h:199)
==5033==    by 0x4E7EBA7: virAsprintf (util.c:1827)
==5033==    by 0x4335F5: remoteDispatchAuthListHelper (remote.c:2054)
==5033==    by 0x4F35394: virNetServerProgramDispatch (virnetserverprogram.c:416)
==5033==    by 0x4F36620: virNetServerHandleJob (virnetserver.c:164)
==5033==    by 0x4E7CAAB: virThreadPoolWorker (threadpool.c:144)
==5033==    by 0x4E7C3C8: virThreadHelper (threads-pthread.c:161)
==5033==    by 0x31AC8077F0: start_thread (in /lib64/libpthread-2.12.so)
==5033==    by 0x31AC0E570C: clone (in /lib64/libc-2.12.so)
...

Verify with libvirt-0.9.10-9.el6.x86_64 and it passed。 The steps are as follows:
1 use valgrind to start libvirtd:
# service libvirtd stop
Stopping libvirtd daemon:                                  [  OK  ]

valgrind --leak-check=full libvirtd >& /root/valgrind.log 2>&1

2 excute domif-setlink command and check valgrind log:
# virsh domif-setlink vr-rhel6-x86_64-kvm 52:54:00:2f:e1:f7 down --persistent
Device updated successfully

# service libvirtd stop
Stopping libvirtd daemon:                                  [  OK  ]

# cat /root/valgrind.log
...
==7475==    definitely lost: 0 bytes in 0 blocks
==7475==    indirectly lost: 0 bytes in 0 blocks
==7475==      possibly lost: 4,048 bytes in 11 blocks
...
Comment 10 errata-xmlrpc 2012-06-20 02:50:13 EDT
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2012-0748.html

Note You need to log in before you can comment on or make changes to this bug.