- Devel - libvirt List Archives

[Libvir] avoid used-uninitialized errors in test driver
by Jim Meyering 01 Feb '08

01 Feb '08

Here's another distracting and easily avoided error: $ valgrind --leak-check=full ./virsh --connect test://$PWD/../docs/testnode.xml list ==13150== Conditional jump or move depends on uninitialised value(s) ==13150== at 0x4177DE: testLoadDomain (test.c:324) ==13150== by 0x417BA4: testLoadDomainFromFile (test.c:401) ==13150== by 0x418E52: testOpenFromFile (test.c:797) ==13150== by 0x4192C9: testOpen (test.c:913) ==13150== by 0x40FA20: do_open (libvirt.c:572) ==13150== by 0x40FDE4: virConnectOpenAuth (libvirt.c:681) ==13150== by 0x40D5B1: vshInit (virsh.c:4464) ==13150== by 0x40E67C: main (virsh.c:4985) ==13150== ==13150== Conditional jump or move depends on uninitialised value(s) ==13150== at 0x417F0D: testLoadNetwork (test.c:486) ==13150== by 0x4183AA: testLoadNetworkFromFile (test.c:577) ==13150== by 0x418F81: testOpenFromFile (test.c:822) ==13150== by 0x4192C9: testOpen (test.c:913) ==13150== by 0x40FA20: do_open (libvirt.c:572) ==13150== by 0x40FDE4: virConnectOpenAuth (libvirt.c:681) ==13150== by 0x40D5B1: vshInit (virsh.c:4464) ==13150== by 0x40E67C: main (virsh.c:4985) ==13150== ==13150== Conditional jump or move depends on uninitialised value(s) ==13150== at 0x41955B: testNumOfDomains (test.c:1020) ==13150== by 0x4104B9: virConnectNumOfDomains (libvirt.c:940) ==13150== by 0x405363: cmdList (virsh.c:557) ==13150== by 0x40C5F9: vshCommandRun (virsh.c:4032) ==13150== by 0x40E6AF: main (virsh.c:4991) Id Name State ---------------------------------- 1 fv0 running 2 fc4 running This patch fixes it: * src/test.c (testOpenFromFile): avoid used-uninitialized errors in test driver --- src/test.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/test.c b/src/test.c index 85170d9..fe5da41 100644 --- a/src/test.c +++ b/src/test.c @@ -672,7 +672,7 @@ static int testOpenFromFile(virConnectPtr conn, xmlNodePtr *domains, *networks = NULL; xmlXPathContextPtr ctxt = NULL; virNodeInfoPtr nodeInfo; - testConnPtr privconn = malloc(sizeof(*privconn)); + testConnPtr privconn = calloc(1, sizeof(*privconn)); if (!privconn) { testError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, "testConn"); return VIR_DRV_OPEN_ERROR; -- 1.5.4.rc5.1.g0fa73

2 1

[Libvir] libvirt.c: avoid a double-free upon do_open failure
by Jim Meyering 01 Feb '08

01 Feb '08

With a contrived example using more than 20 (the max permitted by the testing framework) domains, I got a double-free error: ==4821== Invalid free() / delete / delete[] ==4821== at 0x4A0560B: free (vg_replace_malloc.c:233) ==4821== by 0x4167F1: virReleaseConnect (hash.c:717) ==4821== by 0x4168E7: virUnrefConnect (hash.c:746) ==4821== by 0x40FCEB: do_open (libvirt.c:621) ==4821== by 0x40FDF1: virConnectOpenAuth (libvirt.c:682) ==4821== by 0x40D5B1: vshInit (virsh.c:4464) ==4821== by 0x40E67C: main (virsh.c:4985) ==4821== Address 0x4C3BA68 is 0 bytes inside a block of size 60 free'd ==4821== at 0x4A0560B: free (vg_replace_malloc.c:233) ==4821== by 0x40FCB3: do_open (libvirt.c:618) ==4821== by 0x40FDF1: virConnectOpenAuth (libvirt.c:682) ==4821== by 0x40D5B1: vshInit (virsh.c:4464) ==4821== by 0x40E67C: main (virsh.c:4985) error: failed to connect to the hypervisor ... here's one way to fix it: diff --git a/src/libvirt.c b/src/libvirt.c index defadc1..c19565f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -615,7 +615,6 @@ do_open (const char *name, return ret; failed: - free (ret->name); if (ret->driver) ret->driver->close (ret); if (uri) xmlFreeURI(uri); virUnrefConnect(ret); At first, rather than removing the offending free, I inserted this line just after it: ret->name = NULL; which avoids leaking ->name even if some driver-specific close function fails to clean up properly. But IMHO if such a function doesn't clean up properly then *it* should be fixed, not all callers.

2 1

[Libvir] [PATCH] test.c: Avoid segfault upon malloc failure, and plug a leak.
by Jim Meyering 01 Feb '08

01 Feb '08

Testing exposed a tiny leak: $ valgrind --leak-check=full ./virsh --connect \ test://$PWD/../docs/testnode.xml save fc4 /dev/null ... ==11077== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 1) ==11077== malloc/free: in use at exit: 24,677 bytes in 367 blocks. ==11077== malloc/free: 23,521 allocs, 23,154 frees, 3,424,316 bytes allocated. ==11077== For counts of detected errors, rerun with: -v ==11077== searching for pointers to 367 not-freed blocks. ==11077== checked 538,400 bytes. ==11077== ==11077== 4,000 bytes in 1 blocks are definitely lost in loss record 26 of 27 ==11077== at 0x4A059F6: malloc (vg_replace_malloc.c:149) ==11077== by 0x41C53A: virBufferNew (buf.c:132) ==11077== by 0x41AB22: testDomainDumpXML (test.c:1473) ==11077== by 0x41A1B5: testDomainSave (test.c:1284) ==11077== by 0x41120B: virDomainSave (libvirt.c:1342) ==11077== by 0x406169: cmdSave (virsh.c:1057) ==11077== by 0x40C5F9: vshCommandRun (virsh.c:4032) ==11077== by 0x40E6AF: main (virsh.c:4991) ==11077== ==11077== LEAK SUMMARY: ==11077== definitely lost: 4,000 bytes in 1 blocks. ==11077== possibly lost: 0 bytes in 0 blocks. ==11077== still reachable: 20,677 bytes in 366 blocks. ==11077== suppressed: 0 bytes in 0 blocks. Here's the fix: test.c: Avoid segfault upon malloc failure, and plug a leak. * src/test.c (testDomainSave): Time permitting, I will add tests to exercise as many of the commands as possible. --- src/test.c | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/src/test.c b/src/test.c index 35e41a3..85170d9 100644 --- a/src/test.c +++ b/src/test.c @@ -1,7 +1,7 @@ /* * test.c: A "mock" hypervisor for use by application unit tests * - * Copyright (C) 2006-2007 Red Hat, Inc. + * Copyright (C) 2006-2008 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1281,6 +1281,11 @@ static int testDomainSave(virDomainPtr domain, GET_DOMAIN(domain, -1); xml = testDomainDumpXML(domain, 0); + if (xml == NULL) { + testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, + "cannot allocate space for metadata"); + return (-1); + } if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR)) < 0) { testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, @@ -1303,9 +1308,11 @@ static int testDomainSave(virDomainPtr domain, if (write(fd, xml, len) != len) { testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "cannot write metadata"); + free(xml); close(fd); return (-1); } + free(xml); if (close(fd) < 0) { testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "cannot save domain data"); -- 1.5.4.rc5.1.g0fa73

2 1

[Libvir] [PATCH] fix compilation with USE_XEN == 0
by Guido Guenther 31 Jan '08

31 Jan '08

Hi, statstest.c needs to check if USE_XEN > 0, otherwise compilation breaks on --without-xen builds. Please apply if appropriate. Patch is against current cvs. Cheers, -- Guido

3 4

[Libvir] Re: libvirt on mingw
by Richard W.M. Jones 31 Jan '08

31 Jan '08

Brecht Sanders wrote: > I wasn't able to run virt-manager yet because of Python dependancy > issues, but I'm sure I'll be able to work that out. > Did you have virt-manager running on win32 yet? No, I wasn't able to get virt-manager running. Probably the same problems as you are having: building Python modules (specifically, libvirtmod.dll) which can work from the Win32 version of python, with gcc/MinGW, proves to be very hard. If you get it working, please let me know. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903

1 0

[Libvir] virsh capabilities cmd: plug a leak
by Jim Meyering 31 Jan '08

31 Jan '08

Here's another: $ valgrind --leak-check=full \ ./virsh --connect test://$PWD/../docs/testnode.xml capabilities ... ==16392== 368 bytes in 1 blocks are definitely lost in loss record 13 of 27 ==16392== at 0x4A059F6: malloc (vg_replace_malloc.c:149) ==16392== by 0x387BC7CC11: strdup (strdup.c:43) ==16392== by 0x4194D2: testGetCapabilities (test.c:1006) ==16392== by 0x412B43: virConnectGetCapabilities (libvirt.c:2119) ==16392== by 0x408047: cmdCapabilities (virsh.c:2018) ==16392== by 0x40C5F9: vshCommandRun (virsh.c:4032) ==16392== by 0x40E6AF: main (virsh.c:4991) ==16392== Here's the patch: * src/virsh.c (cmdCapabilities): Plug a small leak. diff --git a/src/virsh.c b/src/virsh.c index c09dc8d..0d93fb6 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -2020,6 +2020,7 @@ cmdCapabilities (vshControl * ctl, vshCmd * cmd ATTRIBUTE_UNUSED) return FALSE; } vshPrint (ctl, "%s\n", caps); + free (caps); return TRUE; } -- 1.5.4.rc5.1.g0fa73

3 2

Re: [Libvir] Libvirt on windows
by Richard W.M. Jones 31 Jan '08

31 Jan '08

Gabriel Kaufmann wrote: > I tool libvirt-0.dll and I am using it in VC. > In order to do this, I had to dynamically load the DLL and map all its functions. This is somewhat outside my experience. My best suggestion (guess) is to try adding __declspec(dllexport) around some libvirt functions to see if this causes them to be exported to VC++-compiled programs. On gcc declspec doesn't appear to be necessary. See also: http://www.cygwin.com/ml/cygwin/2000-05/msg00430.html Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903

2 2

[Libvir] Re: libvirt on mingw
by Richard W.M. Jones 30 Jan '08

30 Jan '08

Brecht Sanders wrote: > Hello again, > Note that I am using gcc (mingw) 3.4.5, and not 4 as you recommend. > The attached patch got rid of the warnings though. > Maybe there is a compiler switch to fix it in a cleaner way though. > Anyway, with the attached patch it does compile. > Basically it casts to void* before converting to a different type. Cheers for the patch, I'll take a look at it tomorrow ... There is a reason why you need to use gcc 4 although it's been so long since I did this that I can't remember exactly why now. Possibly it was some compiler feature that we use in libvirt. Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903

1 0

[Libvir] [PATCH] avoid virsh hang due to missing virDomainFree(dom) call
by Jim Meyering 30 Jan '08

30 Jan '08

Without the following patch, this command would hang printf 'domuuid fc4\ndomstate fc4\n' \ | ./virsh --connect test://$PWD/../docs/testnode.xml with this stack trace: __lll_lock_wait ... _L_lock_105 ... __pthread_mutex_lock ... virUnrefDomain (domain=0x6a8b30) at hash.c:884 virDomainFree (domain=0x6a8b30) at libvirt.c:1211 cmdDomstate (ctl=0x7fffea723390, cmd=0x6a8b10) at virsh.c:677 vshCommandRun (ctl=0x7fffea723390, cmd=0x6a8b10) at virsh.c:4033 main (argc=3, argv=0x7fffea7234e8) at virsh.c:5015 The problem is that cmdDomuuid didn't call virDomainFree(dom), so cmdDomstate hangs trying to do the Unref. I then did a mind-numbing audit of all of the other cmd* functions in virsh.c and found two other cases in which a "dom" may not be released. Here's the patch: Avoid virsh hang due to missing virDomainFree(dom) calls * src/virsh.c (cmdDomuuid): Add missing virDomainFree call. (cmdAttachDevice): Likewise. (cmdDetachDevice): Likewise. --- src/virsh.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/virsh.c b/src/virsh.c index 0d93fb6..487f256 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -2161,6 +2161,7 @@ cmdDomuuid(vshControl * ctl, vshCmd * cmd) else vshError(ctl, FALSE, "%s", _("failed to get domain UUID")); + virDomainFree(dom); return TRUE; } @@ -3038,8 +3039,10 @@ cmdAttachDevice(vshControl * ctl, vshCmd * cmd) return FALSE; } - if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) { + virDomainFree(dom); return FALSE; + } ret = virDomainAttachDevice(dom, buffer); free (buffer); @@ -3092,8 +3095,10 @@ cmdDetachDevice(vshControl * ctl, vshCmd * cmd) return FALSE; } - if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) + if (virFileReadAll(from, VIRSH_MAX_XML_FILE, &buffer) < 0) { + virDomainFree(dom); return FALSE; + } ret = virDomainDetachDevice(dom, buffer); free (buffer); -- 1.5.4.rc5.1.g0fa73

2 5

[Libvir] plug small leaks upon failure
by Jim Meyering 30 Jan '08

30 Jan '08

with more than 20 domains, I get this: $ valgrind --leak-check=full ./virsh --connect test://$PWD/../docs/testnode.xml list ... error: failed to connect to the hypervisor ... ==13756== 5 bytes in 1 blocks are definitely lost in loss record 1 of 32 ==13756== at 0x4A059F6: malloc (vg_replace_malloc.c:149) ==13756== by 0x387BC7CC11: strdup (strdup.c:43) ==13756== by 0x41D9F2: virXPathString (xml.c:486) ==13756== by 0x4174D3: testLoadDomain (test.c:248) ==13756== by 0x417BA4: testLoadDomainFromFile (test.c:401) ==13756== by 0x418E57: testOpenFromFile (test.c:797) ==13756== by 0x4192CE: testOpen (test.c:913) ==13756== by 0x40FA20: do_open (libvirt.c:572) ==13756== by 0x40FDE4: virConnectOpenAuth (libvirt.c:681) ==13756== by 0x40D5B1: vshInit (virsh.c:4464) ==13756== by 0x40E67C: main (virsh.c:4985) ==13756== ==13756== ==13756== 12,996 (352 direct, 12,644 indirect) bytes in 1 blocks are definitely lost in loss record 31 of 32 ==13756== at 0x4A059F6: malloc (vg_replace_malloc.c:149) ==13756== by 0x38864880E6: xmlXPathNewContext (in /usr/lib64/libxml2.so.2.6.31) ==13756== by 0x417494: testLoadDomain (test.c:242) ==13756== by 0x417BA4: testLoadDomainFromFile (test.c:401) ==13756== by 0x418E57: testOpenFromFile (test.c:797) ==13756== by 0x4192CE: testOpen (test.c:913) ==13756== by 0x40FA20: do_open (libvirt.c:572) ==13756== by 0x40FDE4: virConnectOpenAuth (libvirt.c:681) ==13756== by 0x40D5B1: vshInit (virsh.c:4464) ==13756== by 0x40E67C: main (virsh.c:4985) ==13756== ==13756== LEAK SUMMARY: ==13756== definitely lost: 357 bytes in 2 blocks. ==13756== indirectly lost: 12,644 bytes in 32 blocks. ==13756== possibly lost: 0 bytes in 0 blocks. ==13756== still reachable: 20,614 bytes in 362 blocks. ==13756== suppressed: 0 bytes in 0 blocks. ==13756== Reachable blocks (those to which a pointer was found) are not shown. ==13756== To see them, rerun with: --leak-check=full --show-reachable Here's the fix: * test.c (testLoadDomain): Avoid leaks upon failure. --- src/test.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/test.c b/src/test.c index fe5da41..ab83f65 100644 --- a/src/test.c +++ b/src/test.c @@ -327,7 +327,7 @@ static int testLoadDomain(virConnectPtr conn, } } if (handle < 0) - return (-1); + goto error; privconn->domains[handle].active = 1; privconn->domains[handle].id = domid; -- 1.5.4.rc5.1.g0fa73

2 1