Bug 725322 - memory leak when setting blkiotune weight value of guest
memory leak when setting blkiotune weight value of guest
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt (Show other bugs)
6.2
x86_64 Linux
high Severity high
: rc
: ---
Assigned To: Osier Yang
Virtualization Bugs
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-25 01:56 EDT by Alex Jia
Modified: 2011-12-06 06:17 EST (History)
3 users (show)

See Also:
Fixed In Version: libvirt-0.9.4-rc2-1.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-12-06 06:17:37 EST
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 Alex Jia 2011-07-25 01:56:57 EDT
Description of problem:
Memory leak on remoteSerializeTypedParameters function from remote_driver.c.

Version-Release number of selected component (if applicable):
# uname -r
2.6.32-160.el6.x86_64

# rpm -q libvirt
libvirt-0.9.3-7.el6.x86_64

# rpm -q qemu-kvm
qemu-kvm-0.12.1.2-2.169.el6.x86_64 

How reproducible:
always

Steps to Reproduce:
1. valgrind -v --leak-check=full --show-reachable=yes virsh blkiotune ${guestname} --weight ${number}
Note: number is in range [100, 1000]
2. check 'LEAK SUMMARY'
  
Actual results:
==15803== 7 bytes in 1 blocks are indirectly lost in loss record 2 of 24
==15803==    at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==15803==    by 0x386A27F8A1: strdup (in /lib64/libc-2.12.so)
==15803==    by 0x3FF74C5DE0: remoteSerializeTypedParameters (remote_driver.c:1211)
==15803==    by 0x3FF74C6208: remoteDomainSetBlkioParameters (remote_client_bodies.h:1628)
==15803==    by 0x3FF74AFC52: virDomainSetBlkioParameters (libvirt.c:3134)
==15803==    by 0x41C086: cmdBlkiotune (virsh.c:3659)
==15803==    by 0x410CC2: vshCommandRun (virsh.c:12758)
==15803==    by 0x41F286: main (virsh.c:14110)

==15803== 31 (24 direct, 7 indirect) bytes in 1 blocks are definitely lost in loss record 8 of 24
==15803==    at 0x4A04A28: calloc (vg_replace_malloc.c:467)
==15803==    by 0x3FF7443A2B: virAllocN (memory.c:129)
==15803==    by 0x3FF74C5DAC: remoteSerializeTypedParameters (remote_driver.c:1204)
==15803==    by 0x3FF74C6208: remoteDomainSetBlkioParameters (remote_client_bodies.h:1628)
==15803==    by 0x3FF74AFC52: virDomainSetBlkioParameters (libvirt.c:3134)
==15803==    by 0x41C086: cmdBlkiotune (virsh.c:3659)
==15803==    by 0x410CC2: vshCommandRun (virsh.c:12758)
==15803==    by 0x41F286: main (virsh.c:14110)

==15803== LEAK SUMMARY:
==15803==    definitely lost: 24 bytes in 1 blocks
==15803==    indirectly lost: 7 bytes in 1 blocks
==15803==      possibly lost: 0 bytes in 0 blocks
==15803==    still reachable: 331,417 bytes in 55 blocks
==15803==         suppressed: 0 bytes in 0 blocks 

Expected results:
Fix these 2 memory leak.

Additional info:
I tried to free strdup and VIR_ALLOC_N allocated memory in remoteSerializeTypedParameters function from src/remote/remote_driver.c,
but it seems cleanup section is doing this, however, if the everything is
okay, the programming will execute the following codes:
1244     *args_params_val = val;
1245     val = NULL;

And 'val' point to NULL, the following codes can't free allocated memory for ever:
1248 cleanup:
1249     if (val) {
1250         for (i = 0; i < nparams; i++)
1251             VIR_FREE(val[i].field);
1252         VIR_FREE(val);
1253     }
1254     return rv;
1255 }

However, it seems this is a expected result, we don't want to free 'val', so assign 'NULL' to 'val', I saw the following codes in this function:
1210     for (i = 0; i < nparams; ++i) {
1211         /* call() will free this: */
1212         val[i].field = strdup (params[i].field);

"call() will free this", so change my original mind and check related codes on remoteDomainSetBlkioParameters function from src/remote/remote_client_bodies.h:

1763     if (remoteSerializeTypedParameters(params, nparams, &args.params.params_val, &args.params.params_len) < 0) {
1764         xdr_free((xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char *)&args);
1765         goto done;
1766     }
1767 
1768     if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS,
1769              (xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char *)&args,
1770              (xdrproc_t)xdr_void, (char *)NULL) == -1) {
1771         goto done;
1772     }

It seems we haven't missed xdr_free in here, because this file is automatically generated by src/rpc/gendispatch.pl, I tried to add xdr_free((xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char *)&args); followed by 1772 line in src/rpc/gendispatch.pl, it is an thankless effort. So I think we should back to original mind and should fix this leak in remoteSerializeTypedParameters function from src/remote/remote_driver.c, I hope these information are helpful for you.


Alex
Comment 1 Osier Yang 2011-07-27 09:26:39 EDT
> It seems we haven't missed xdr_free in here, because this file is automatically
> generated by src/rpc/gendispatch.pl, I tried to add
> xdr_free((xdrproc_t)xdr_remote_domain_set_blkio_parameters_args, (char
> *)&args); followed by 1772 line in src/rpc/gendispatch.pl, it is an thankless
> effort. So I think we should back to original mind and should fix this leak in
> remoteSerializeTypedParameters function from src/remote/remote_driver.c,

The leak is not from remoteSerializeTypedParameters, (the "val" never should be freed if no failure). but from the generated remoteDomainSet* functions, such as remoteDomainSetMemoryFlags, remoteDomainSetSchedulerparameters, etc, of course includes remoteDomainSetBlkioParameters.
Comment 2 Eric Blake 2011-07-27 17:20:08 EDT
Fixed upstream:

commit 8d2319cb9038bbb71dcb8079439bd5e506995e52
Author: Matthias Bolte <matthias.bolte@googlemail.com>
Date:   Wed Jul 27 20:32:25 2011 +0200

    rpc: Fix memory leak in remoteDomainSet*Parameters functions
    
    Add a new helper remoteFreeTypedParameters and teach the generator
    to add it to the cleanup section.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=725322
Comment 4 Alex Jia 2011-07-31 22:37:53 EDT
This issue has been fixed on rhel6.2(2.6.32-160.el6.x86_64) with
libvirt-0.9.4-0rc2.el6.x86_64 and qemu-kvm-0.12.1.2-2.169.el6.x86_64, the
following is actual test result by valgrind, so change the bug status to
VERIFIED.


Actual result:

Detected in valgrind run:

==1963== 
==1963== HEAP SUMMARY:
==1963==     in use at exit: 331,419 bytes in 55 blocks
==1963==   total heap usage: 897 allocs, 842 frees, 2,084,107 bytes allocated
==1963== 
==1963== Searching for pointers to 55 not-freed blocks
==1963== Checked 1,518,568 bytes
==1963== 
==1963== LEAK SUMMARY:
==1963==    definitely lost: 0 bytes in 0 blocks
==1963==    indirectly lost: 0 bytes in 0 blocks
==1963==      possibly lost: 0 bytes in 0 blocks
==1963==    still reachable: 331,419 bytes in 55 blocks
Comment 5 errata-xmlrpc 2011-12-06 06:17:37 EST
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/RHBA-2011-1513.html

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