[libvirt] [PATCH] Fix valgrind complaints when using kill -SIGHUP on libvirtd
This patch fixes a couple of complaints from valgrind when tickling libvirtd with SIGHUP. The first two files contain fixes for memory leaks. The 3rd one initializes an uninitialized variable. The 4th one is another memory leak. Here are some example valgrind outputs (not in order of patch addressing it): ==5261== 43 bytes in 1 blocks are definitely lost in loss record 347 of 573 ==5261== at 0x4A0515D: malloc (vg_replace_malloc.c:195) ==5261== by 0x327607FD51: strdup (in /lib64/libc-2.12.so) ==5261== by 0x4E865DF: virStoragePoolLoadAllConfigs (storage_conf.c:1413) ==5261== by 0x48A759: storageDriverStartup (storage_driver.c:168) ==5261== by 0x4EA4D2F: virStateInitialize (libvirt.c:978) ==5261== by 0x41E190: main (libvirtd.c:3241) ==5261== ==5261== 43 bytes in 1 blocks are definitely lost in loss record 348 of 573 ==5261== at 0x4A0515D: malloc (vg_replace_malloc.c:195) ==5261== by 0x327607FD51: strdup (in /lib64/libc-2.12.so) ==5261== by 0x4E865DF: virStoragePoolLoadAllConfigs (storage_conf.c:1413) ==5261== by 0x48A551: storageDriverReload (storage_driver.c:198) ==5261== by 0x4E9566D: virStateReload (libvirt.c:1017) ==5261== by 0x418870: qemudDispatchSignalEvent (libvirtd.c:388) ==5261== by 0x418538: virEventRunOnce (event.c:479) ==5261== by 0x41A1D5: qemudOneLoop (libvirtd.c:2215) ==5261== by 0x41A4A2: qemudRunLoop (libvirtd.c:2324) ==5261== by 0x3276807760: start_thread (in /lib64/libpthread-2.12.so) ==5261== by 0x32760E14EC: clone (in /lib64/libc-2.12.so) [...] ==5261== Conditional jump or move depends on uninitialised value(s) ==5261== at 0x437E66: virPipeReadUntilEOF (util.c:966) ==5261== by 0x439467: virRunWithHook (util.c:829) ==5261== by 0x4A821F: ebiptablesExecCLI (nwfilter_ebiptables_driver.c:2312) ==5261== by 0x4A8728: ebiptablesTearOldRules (nwfilter_ebiptables_driver.c:3218) [...] ==21871== 841 bytes in 20 blocks are definitely lost in loss record 552 of 577 ==21871== at 0x4A0515D: malloc (vg_replace_malloc.c:195) ==21871== by 0x327607FD51: strdup (in /lib64/libc-2.12.so) ==21871== by 0x4E7FD34: virNWFilterPoolLoadAllConfigs (nwfilter_conf.c:2242) ==21871== by 0x4A5406: nwfilterDriverStartup (nwfilter_driver.c:111) ==21871== by 0x4EA4D5F: virStateInitialize (libvirt.c:978) ==21871== by 0x41E190: main (libvirtd.c:3241) [...] ==12889== at 0x4A04481: calloc (vg_replace_malloc.c:418) ==12889== by 0x434D2D: virAlloc (memory.c:100) ==12889== by 0x4E8E0A1: x86MapLoadCallback (cpu_x86.c:473) ==12889== by 0x4E90ED0: cpuMapLoad (cpu_map.c:58) ==12889== by 0x4E8DACE: x86LoadMap (cpu_x86.c:1131) ==12889== by 0x4E901DB: x86Decode (cpu_x86.c:1313) ==12889== by 0x45F4F8: qemudCapsInit (qemu_conf.c:1056) ==12889== by 0x440FBF: qemuCreateCapabilities (qemu_driver.c:1553) ==12889== by 0x44165F: qemudStartup (qemu_driver.c:1856) ==12889== by 0x4EA4D3F: virStateInitialize (libvirt.c:978) ==12889== by 0x41E190: main (libvirtd.c:3241) Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- src/conf/nwfilter_conf.c | 1 + src/conf/storage_conf.c | 2 ++ src/util/util.c | 2 ++ 3 files changed, 5 insertions(+) Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -2239,6 +2239,7 @@ virNWFilterPoolObjLoad(virConnectPtr con return NULL; } + VIR_FREE(pool->configFile); // for driver reload pool->configFile = strdup(path); if (pool->configFile == NULL) { virReportOOMError(); Index: libvirt-acl/src/conf/storage_conf.c =================================================================== --- libvirt-acl.orig/src/conf/storage_conf.c +++ libvirt-acl/src/conf/storage_conf.c @@ -1403,12 +1403,14 @@ virStoragePoolObjLoad(virStoragePoolObjL return NULL; } + VIR_FREE(pool->configFile); // for driver reload pool->configFile = strdup(path); if (pool->configFile == NULL) { virReportOOMError(); virStoragePoolDefFree(def); return NULL; } + VIR_FREE(pool->autostartLink); // for driver reload pool->autostartLink = strdup(autostartLink); if (pool->autostartLink == NULL) { virReportOOMError(); Index: libvirt-acl/src/util/util.c =================================================================== --- libvirt-acl.orig/src/util/util.c +++ libvirt-acl/src/util/util.c @@ -941,9 +941,11 @@ virPipeReadUntilEOF(int outfd, int errfd fds[0].fd = outfd; fds[0].events = POLLIN; + fds[0].revents = 0; finished[0] = 0; fds[1].fd = errfd; fds[1].events = POLLIN; + fds[1].revents = 0; finished[1] = 0; while(!(finished[0] && finished[1])) { Index: libvirt-acl/src/cpu/cpu_x86.c =================================================================== --- libvirt-acl.orig/src/cpu/cpu_x86.c +++ libvirt-acl/src/cpu/cpu_x86.c @@ -1092,6 +1092,12 @@ x86MapFree(struct x86_map *map) x86ModelFree(model); } + while (map->vendors != NULL) { + struct x86_vendor *vendor = map->vendors; + map->vendors = vendor->next; + x86VendorFree(vendor); + } + VIR_FREE(map); }
On Thu, Aug 12, 2010 at 02:58:15PM -0400, Stefan Berger wrote:
This patch fixes a couple of complaints from valgrind when tickling libvirtd with SIGHUP.
Index: libvirt-acl/src/conf/nwfilter_conf.c =================================================================== --- libvirt-acl.orig/src/conf/nwfilter_conf.c +++ libvirt-acl/src/conf/nwfilter_conf.c @@ -2239,6 +2239,7 @@ virNWFilterPoolObjLoad(virConnectPtr con return NULL; }
+ VIR_FREE(pool->configFile); // for driver reload pool->configFile = strdup(path); if (pool->configFile == NULL) { virReportOOMError(); Index: libvirt-acl/src/conf/storage_conf.c =================================================================== --- libvirt-acl.orig/src/conf/storage_conf.c +++ libvirt-acl/src/conf/storage_conf.c @@ -1403,12 +1403,14 @@ virStoragePoolObjLoad(virStoragePoolObjL return NULL; }
+ VIR_FREE(pool->configFile); // for driver reload pool->configFile = strdup(path); if (pool->configFile == NULL) { virReportOOMError(); virStoragePoolDefFree(def); return NULL; } + VIR_FREE(pool->autostartLink); // for driver reload pool->autostartLink = strdup(autostartLink); if (pool->autostartLink == NULL) { virReportOOMError(); Index: libvirt-acl/src/util/util.c =================================================================== --- libvirt-acl.orig/src/util/util.c +++ libvirt-acl/src/util/util.c @@ -941,9 +941,11 @@ virPipeReadUntilEOF(int outfd, int errfd
fds[0].fd = outfd; fds[0].events = POLLIN; + fds[0].revents = 0; finished[0] = 0; fds[1].fd = errfd; fds[1].events = POLLIN; + fds[1].revents = 0; finished[1] = 0;
while(!(finished[0] && finished[1])) { Index: libvirt-acl/src/cpu/cpu_x86.c =================================================================== --- libvirt-acl.orig/src/cpu/cpu_x86.c +++ libvirt-acl/src/cpu/cpu_x86.c @@ -1092,6 +1092,12 @@ x86MapFree(struct x86_map *map) x86ModelFree(model); }
+ while (map->vendors != NULL) { + struct x86_vendor *vendor = map->vendors; + map->vendors = vendor->next; + x86VendorFree(vendor); + } + VIR_FREE(map); }
ACK, all looks good Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
libvir-list-bounces@redhat.com wrote on 08/12/2010 04:26:26 PM:
Please respond to "Daniel P. Berrange"
On Thu, Aug 12, 2010 at 02:58:15PM -0400, Stefan Berger wrote:
This patch fixes a couple of complaints from valgrind when tickling libvirtd with SIGHUP.
[...]
ACK, all looks good
Daniel
Pushed. Stefan
participants (3)
-
Daniel P. Berrange -
Stefan Berger -
Stefan Berger