[Libvir] avoid used-uninitialized errors in test driver
by Jim Meyering
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
16 years, 3 months
[Libvir] libvirt.c: avoid a double-free upon do_open failure
by Jim Meyering
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.
16 years, 3 months
[Libvir] [PATCH] test.c: Avoid segfault upon malloc failure, and plug a leak.
by Jim Meyering
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
16 years, 3 months
[Libvir] Re: libvirt on mingw
by Richard W.M. Jones
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
16 years, 3 months
[Libvir] virsh capabilities cmd: plug a leak
by Jim Meyering
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
16 years, 3 months
Re: [Libvir] Libvirt on windows
by Richard W.M. Jones
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
16 years, 3 months
[Libvir] Re: libvirt on mingw
by Richard W.M. Jones
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
16 years, 3 months
[Libvir] [PATCH] avoid virsh hang due to missing virDomainFree(dom) call
by Jim Meyering
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
16 years, 3 months
[Libvir] plug small leaks upon failure
by Jim Meyering
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
16 years, 3 months
