[libvirt] [PATCH] network: avoid memory leak on cleanup
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; } static int networkGetAutostart(virNetworkPtr net, -- 1.7.1
2011/7/15 <ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net,
NACK. Now networkGetBridgeName returns NULL always, that's wrong. Why do you think that there is a leak? -- Matthias Bolte http://photron.blogspot.com
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15<ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net, NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) Note: remote memory leak are a known bug:https://bugzilla.redhat.com/show_bug.cgi?id=690734, so this patch only fix network memory leak. The following codes leads memory leak: 2448 static char *networkGetBridgeName(virNetworkPtr net) { 2449 struct network_driver *driver = net->conn->networkPrivateData; 2450 virNetworkObjPtr network; 2451*char *bridge = NULL;* ...... 2470*bridge = strdup(network->def->bridge);* Cleanup section hasn't released allocation free, and the programming will return a NULL if only and only error condition happen, if everything is okay, 'bridge' will be returned, right? Regards, Alex
On 07/15/2011 03:03 PM, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15<ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net, NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084)
Note: remote memory leak are a known bug:https://bugzilla.redhat.com/show_bug.cgi?id=690734, so this patch only fix network memory leak.
The following codes leads memory leak: 2448 static char *networkGetBridgeName(virNetworkPtr net) { 2449 struct network_driver *driver = net->conn->networkPrivateData; 2450 virNetworkObjPtr network; 2451*char *bridge = NULL;* ...... 2470*bridge = strdup(network->def->bridge);*
Cleanup section hasn't released allocation free, and the programming will return a NULL if only and only error condition happen, if everything is okay, 'bridge' will be returned, right?
Regards, Alex
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Yeah, we need to return 'bridge' pointer variable, so shouldn't free it, I should fix other function memory leak.
Thanks, Alex Alex
2011/7/15 Alex Jia <ajia@redhat.com>:
On 07/15/2011 03:03 PM, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15 <ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net,
NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084)
The caller of virNetworkGetBridgeName has to free the allocated string: diff --git a/tools/virsh.c b/tools/virsh.c index bd6fea7..5c48f0c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5011,6 +5011,7 @@ cmdNetworkInfo(vshControl *ctl, const vshCmd *cmd) if (bridge) vshPrint(ctl, "%-15s %s\n", _("Bridge:"), bridge); + VIR_FREE(bridge); virNetworkFree(network); return true; }
==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084)
Note: remote memory leak are a known bug: https://bugzilla.redhat.com/show_bug.cgi?id=690734, so this patch only fix network memory leak.
Judging by a quick look at the code this two leaks should not be possible? Are you working on current git? Anyway, I'll leave those to you. -- Matthias Bolte http://photron.blogspot.com
On 07/15/2011 03:35 PM, Matthias Bolte wrote:
2011/7/15 Alex Jia<ajia@redhat.com>:
On 07/15/2011 03:03 PM, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15<ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net,
NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084) The caller of virNetworkGetBridgeName has to free the allocated string:
diff --git a/tools/virsh.c b/tools/virsh.c index bd6fea7..5c48f0c 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -5011,6 +5011,7 @@ cmdNetworkInfo(vshControl *ctl, const vshCmd *cmd) if (bridge) vshPrint(ctl, "%-15s %s\n", _("Bridge:"), bridge);
+ VIR_FREE(bridge); Yeah, I have also found the root reason of leak in cmdNetworkInfo function.
virNetworkFree(network); return true; }
==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084)
Note: remote memory leak are a known bug: https://bugzilla.redhat.com/show_bug.cgi?id=690734, so this patch only fix network memory leak. Judging by a quick look at the code this two leaks should not be possible? Are you working on current git? Anyway, I'll leave those to you.
Upstream is okay, I use libvirt-0.9.3-3.el6.x86_64 rpm installation on my another machine, it seems we haven't picked up some patches into libvirt for rhelx.y. Regards, Alex
On Fri, Jul 15, 2011 at 03:03:07PM +0800, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15<ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net, NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084)
What version of libvirt did you test this on ? These were leaks in the 0.9.3 release, but current GIT has fixed them Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Hi Daniel, # rpm -q libvirt libvirt-0.9.3-3.el6.x86_64 Regards, Alex ----- Original Message ----- From: "Daniel P. Berrange" <berrange@redhat.com> To: "Alex Jia" <ajia@redhat.com> Cc: "Matthias Bolte" <matthias.bolte@googlemail.com>, libvir-list@redhat.com Sent: Friday, July 15, 2011 6:12:17 PM Subject: Re: [libvirt] [PATCH] network: avoid memory leak on cleanup On Fri, Jul 15, 2011 at 03:03:07PM +0800, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15<ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net, NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084)
What version of libvirt did you test this on ? These were leaks in the 0.9.3 release, but current GIT has fixed them Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 07/18/2011 12:35 AM, Alex Jia wrote:
Hi Daniel, # rpm -q libvirt libvirt-0.9.3-3.el6.x86_64
Regards, Alex
----- Original Message ----- From: "Daniel P. Berrange"<berrange@redhat.com> To: "Alex Jia"<ajia@redhat.com> Cc: "Matthias Bolte"<matthias.bolte@googlemail.com>, libvir-list@redhat.com Sent: Friday, July 15, 2011 6:12:17 PM Subject: Re: [libvirt] [PATCH] network: avoid memory leak on cleanup
On Fri, Jul 15, 2011 at 03:03:07PM +0800, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15<ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net, NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) What version of libvirt did you test this on ? These were leaks in the 0.9.3 release, but current GIT has fixed them
Daniel Now, it's fine for libvirt-0.9.3-5.el6.x86_64, I haven't found remote leak again.
Regards, Alex
于 2011年07月18日 11:01, Alex Jia 写道:
On 07/18/2011 12:35 AM, Alex Jia wrote:
Hi Daniel, # rpm -q libvirt libvirt-0.9.3-3.el6.x86_64
Regards, Alex
----- Original Message ----- From: "Daniel P. Berrange"<berrange@redhat.com> To: "Alex Jia"<ajia@redhat.com> Cc: "Matthias Bolte"<matthias.bolte@googlemail.com>, libvir-list@redhat.com Sent: Friday, July 15, 2011 6:12:17 PM Subject: Re: [libvirt] [PATCH] network: avoid memory leak on cleanup
On Fri, Jul 15, 2011 at 03:03:07PM +0800, Alex Jia wrote:
On 07/15/2011 02:49 PM, Matthias Bolte wrote:
2011/7/15<ajia@redhat.com>:
* src/network/bridge_driver.c: Fix memory leak on cleanup section from networkGetBridgeName function. --- src/network/bridge_driver.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0a12bc0..59e780d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2474,7 +2474,8 @@ static char *networkGetBridgeName(virNetworkPtr net) { cleanup: if (network) virNetworkObjUnlock(network); - return bridge; + VIR_FREE(bridge); + return NULL; }
static int networkGetAutostart(virNetworkPtr net, NACK. Now networkGetBridgeName returns NULL always, that's wrong.
Why do you think that there is a leak?
Detected in valgrind run: ==9020== 7 bytes in 1 blocks are definitely lost in loss record 1 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A314B3D: xdr_string (in /lib64/libc-2.12.so) ==9020== by 0x4CFC0AD: xdr_remote_nonnull_string (remote_protocol.c:30) ==9020== by 0x4CFCC08: xdr_remote_network_get_bridge_name_ret (remote_protocol.c:1999) ==9020== by 0x4D06FC1: virNetMessageDecodePayload (virnetmessage.c:286) ==9020== by 0x4D03235: virNetClientProgramCall (virnetclientprogram.c:318) ==9020== by 0x4CE7262: call (remote_driver.c:3925) ==9020== by 0x4CED8D2: remoteNetworkGetBridgeName (remote_client_bodies.h:3384) ==9020== by 0x4CC494E: virNetworkGetBridgeName (libvirt.c:8503) ==9020== by 0x40F654: cmdNetworkInfo (virsh.c:5015) ==9020== by 0x410D02: vshCommandRun (virsh.c:12738) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 10 bytes in 1 blocks are definitely lost in loss record 3 of 26 ==9020== at 0x4A05FDE: malloc (vg_replace_malloc.c:236) ==9020== by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so) ==9020== by 0x4CF508B: doRemoteOpen (remote_driver.c:364) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) ==9020== ==9020== 56 (24 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 26 ==9020== at 0x4A04A28: calloc (vg_replace_malloc.c:467) ==9020== by 0x4C686ED: virAlloc (memory.c:101) ==9020== by 0x4C96870: virDomainEventStateNew (domain_event.c:578) ==9020== by 0x4CF5A8E: doRemoteOpen (remote_driver.c:658) ==9020== by 0x4CF6E8F: remoteOpen (remote_driver.c:800) ==9020== by 0x4CCB384: do_open (libvirt.c:1054) ==9020== by 0x4CCBEB5: virConnectOpenAuth (libvirt.c:1280) ==9020== by 0x410BC0: vshReconnect (virsh.c:589) ==9020== by 0x410DCF: vshCommandRun (virsh.c:12733) ==9020== by 0x41F2D5: main (virsh.c:14084) What version of libvirt did you test this on ? These were leaks in the 0.9.3 release, but current GIT has fixed them
Daniel Now, it's fine for libvirt-0.9.3-5.el6.x86_64, I haven't found remote leak again.
Regards, Alex
You should always use upstream libvirt to make patches for upstream.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (5)
-
ajia＠redhat.com -
Alex Jia
-
Daniel P. Berrange -
Matthias Bolte -
Osier Yang