[libvirt] [PATCH v2] Fix memory leak in virNWFilterDefParseXML()
by Nehal J Wani
While running nwfilterxml2xmltest, it was found that valgrind pointed out the
following error...
==7466== 16 bytes in 1 blocks are definitely lost in loss record 26 of 90
==7466== at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==7466== by 0x4C651AD: virAlloc (viralloc.c:142)
==7466== by 0x4D0450D: virNWFilterDefParseNode (nwfilter_conf.c:2575)
==7466== by 0x4D05D84: virNWFilterDefParse (nwfilter_conf.c:2647)
==7466== by 0x401FDE: testCompareXMLToXMLHelper (nwfilterxml2xmltest.c:39)
==7466== by 0x402DE1: virtTestRun (testutils.c:138)
==7466== by 0x4018E9: mymain (nwfilterxml2xmltest.c:111)
==7466== by 0x403482: virtTestMain (testutils.c:593)
==7466== by 0x341F421A04: (below main) (libc-start.c:225)
...21 times, which are related to 21 tests in nwfilterxml2xmltest.c which sent
EXPECT_WARN = false. There were two scenarios in virNWFilterDefParseXML(),
when the variable 'entry' was malloc'ed, but not freed.
---
src/conf/nwfilter_conf.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 793cb0e..d280df5 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2576,21 +2576,25 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
goto cleanup;
if (xmlStrEqual(curr->name, BAD_CAST "rule")) {
- if (!(entry->rule = virNWFilterRuleParse(curr)))
+ if (!(entry->rule = virNWFilterRuleParse(curr))) {
+ virNWFilterEntryFree(entry);
goto cleanup;
+ }
} else if (xmlStrEqual(curr->name, BAD_CAST "filterref")) {
- if (!(entry->include = virNWFilterIncludeParse(curr)))
+ if (!(entry->include = virNWFilterIncludeParse(curr))) {
+ virNWFilterEntryFree(entry);
goto cleanup;
+ }
}
if (entry->rule || entry->include) {
if (VIR_REALLOC_N(ret->filterEntries, ret->nentries+1) < 0) {
- VIR_FREE(entry);
+ virNWFilterEntryFree(entry);
goto cleanup;
}
ret->filterEntries[ret->nentries++] = entry;
} else
- VIR_FREE(entry);
+ virNWFilterEntryFree(entry);
}
curr = curr->next;
}
--
1.8.1.4
10 years, 11 months
[libvirt] [PATCH] Fix memory leak in virDomainDefParseXML()
by Nehal J Wani
This patch fixes the memory leaks found while running qemuxml2argvtest
==8260== 3 bytes in 1 blocks are definitely lost in loss record 1 of 129
==8260== at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260== by 0x341F485E21: strdup (strdup.c:42)
==8260== by 0x4CADCFF: virStrdup (virstring.c:554)
==8260== by 0x4CBB839: virXPathString (virxml.c:90)
==8260== by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260== by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260== by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260== by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260== by 0x427111: virtTestRun (testutils.c:138)
==8260== by 0x41D3FE: mymain (qemuxml2argvtest.c:452)
==8260== by 0x4277B2: virtTestMain (testutils.c:593)
==8260== by 0x341F421A04: (below main) (libc-start.c:225)
==8260==
==8260== 4 bytes in 1 blocks are definitely lost in loss record 5 of 129
==8260== at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==8260== by 0x341F485E21: strdup (strdup.c:42)
==8260== by 0x4CADCFF: virStrdup (virstring.c:554)
==8260== by 0x4CBB839: virXPathString (virxml.c:90)
==8260== by 0x4CE753A: virDomainDefParseXML (domain_conf.c:11478)
==8260== by 0x4CEB4FE: virDomainDefParseNode (domain_conf.c:12742)
==8260== by 0x4CEB675: virDomainDefParse (domain_conf.c:12684)
==8260== by 0x425958: testCompareXMLToArgvHelper (qemuxml2argvtest.c:107)
==8260== by 0x427111: virtTestRun (testutils.c:138)
==8260== by 0x41D39A: mymain (qemuxml2argvtest.c:451)
==8260== by 0x4277B2: virtTestMain (testutils.c:593)
==8260== by 0x341F421A04: (below main) (libc-start.c:225)
==8260==
---
src/conf/domain_conf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 140eb80..5449bd7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11486,6 +11486,7 @@ virDomainDefParseXML(xmlDocPtr xml,
def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
}
ctxt->node = node;
+ VIR_FREE(tmp);
break;
case VIR_DOMAIN_FEATURE_LAST:
--
1.8.1.4
10 years, 11 months
[libvirt] [PATCH] virsh: fix doc typos
by Nehal J Wani
Fix 6 minor spelling errors in virsh doc
---
tools/virsh.pod | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index dac9a08..f51060f 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -560,7 +560,7 @@ exits.
If I<--pass-fds> is specified, the argument is a comma separated list
of open file descriptors which should be pass on into the guest. The
-file descriptors will be re-numered in the guest, starting from 3. This
+file descriptors will be re-numbered in the guest, starting from 3. This
is only supported with container based virtualization.
B<Example>
@@ -889,7 +889,7 @@ also B<domblklist> for listing these names).
If I<--abort> is specified, the active job on the specified disk will
be aborted. If I<--async> is also specified, this command will return
-immediately, rather than waiting for the cancelation to complete. If
+immediately, rather than waiting for the cancellation to complete. If
I<--pivot> is specified, this requests that an active copy job
be pivoted over to the new copy.
If I<--info> is specified, the active job information on the specified
@@ -1158,7 +1158,7 @@ hostname which will not resolve to match one of its public IP addresses, then
libvirt will generate an incorrect URI. In this case I<migrateuri> should be
explicitly specified, using an IP address, or a correct hostname.
-=item * The host has multiple network interaces. If a host has multiple network
+=item * The host has multiple network interfaces. If a host has multiple network
interfaces, it might be desirable for the migration data stream to be sent over
a specific interface for either security or performance reasons. In this case
I<migrateuri> should be explicitly specified, using an IP address associated
@@ -1739,8 +1739,8 @@ hypervisor.
Suspend a running domain into one of these states (possible I<target>
values):
- mem equivallent of S3 ACPI state
- disk equivallent of S4 ACPI state
+ mem equivalent of S3 ACPI state
+ disk equivalent of S4 ACPI state
hybrid RAM is saved to disk but not powered off
The I<--duration> argument specifies number of seconds before the domain is
@@ -2740,7 +2740,7 @@ B<Supported algorithms>
sanitizing removable and non-removable hard disks:
random x2, 0x00, verify.
dod - 4-pass DoD 5220.22-M section 8-306 procedure for
- sanitizing removeable and non-removeable rigid
+ sanitizing removable and non-removable rigid
disks: random, 0x00, 0xff, verify.
bsi - 9-pass method recommended by the German Center of
Security in Information Technologies
--
1.8.1.4
10 years, 11 months
[libvirt] [PATCH] Fix typos in various docs
by Nehal J Wani
Fix 8 minor spelling errors in docs/*.html.in
---
docs/api.html.in | 2 +-
docs/bugs.html.in | 2 +-
docs/drvlxc.html.in | 2 +-
docs/firewall.html.in | 4 ++--
docs/internals/rpc.html.in | 2 +-
docs/remote.html.in | 2 +-
docs/storage.html.in | 2 +-
7 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/docs/api.html.in b/docs/api.html.in
index f77e9db..eae7657 100644
--- a/docs/api.html.in
+++ b/docs/api.html.in
@@ -207,7 +207,7 @@
virtualization <a href="#Functions">functions</a>. Depending upon the
driver being used, calls will be routed through the remote driver to
the libvirtd daemon. The daemon will reference the connection specific
- driver in order to retreive the requested information and then pass
+ driver in order to retrieve the requested information and then pass
back status and/or data through the connection back to the application.
The application can then decide what to do with that data, such as
display, write log data, etc. <a href="migration.html">Migration</a>
diff --git a/docs/bugs.html.in b/docs/bugs.html.in
index 2295ae9..140d1b4 100644
--- a/docs/bugs.html.in
+++ b/docs/bugs.html.in
@@ -148,7 +148,7 @@
... note the process id from the output
# gdb /usr/sbin/libvirtd
.... some information about gdb and loading debug data
-(gdb) attach $the_damon_process_id
+(gdb) attach $the_daemon_process_id
....
(gdb) thread apply all bt
.... information to attach to the bug
diff --git a/docs/drvlxc.html.in b/docs/drvlxc.html.in
index d5a4410..7494eb3 100644
--- a/docs/drvlxc.html.in
+++ b/docs/drvlxc.html.in
@@ -620,7 +620,7 @@ to PID 1 inside the container.
<p>
If the container does not respond to the graceful shutdown
-request, it can be forceably stopped using the <code>virsh destroy</code>
+request, it can be forcibly stopped using the <code>virsh destroy</code>
</p>
<pre>
diff --git a/docs/firewall.html.in b/docs/firewall.html.in
index 42642bc..5bb6dc1 100644
--- a/docs/firewall.html.in
+++ b/docs/firewall.html.in
@@ -142,7 +142,7 @@ MASQUERADE all -- * * 192.168.122.0/24 !192.168.122.0/24</pre>
<p><a href="http://www.dmtf.org/standards/cim/cim_schema_v2230/CIM_Network.pdf">http://www.dmtf.org/standards/cim/cim_schema_v2230/CIM_Network.pdf</a></p>
<p>The filters are managed in libvirt as a top level, standalone object.
This allows the filters to then be referenced by any libvirt object
- that requires their functionality, instead tieing them only to use
+ that requires their functionality, instead tying them only to use
by guest NICs. In the current implementation, filters can be associated
with individual guest NICs via the libvirt domain XML format. In the
future we might allow filters to be associated with the virtual network
@@ -272,7 +272,7 @@ f5c78134-9da4-0c60-a9f0-fb37bc21ac1f no-other-rarp-traffic
to update them. This ensures the guests have their iptables/ebtables
rules recreated.
</p>
- <p>To associate the clean-trafffic filter with a guest, edit the
+ <p>To associate the clean-traffic filter with a guest, edit the
guest XML config and change the <code><interface></code> element
to include a <code><filterref></code> and also specify the
whitelisted <code><ip address/></code> the guest is allowed to
diff --git a/docs/internals/rpc.html.in b/docs/internals/rpc.html.in
index 40fb59d..627d7aa 100644
--- a/docs/internals/rpc.html.in
+++ b/docs/internals/rpc.html.in
@@ -743,7 +743,7 @@
<p>
The main libvirt event loop thread is responsible for performing all
- socket I/O. It will read incoming packets from clients and willl
+ socket I/O. It will read incoming packets from clients and will
transmit outgoing packets to clients. It will handle the I/O to/from
streams associated with client API calls. When doing client I/O it
will also pass the data through any applicable encryption layer
diff --git a/docs/remote.html.in b/docs/remote.html.in
index 25ae30e..dc9b354 100644
--- a/docs/remote.html.in
+++ b/docs/remote.html.in
@@ -376,7 +376,7 @@ Note that parameter values must be
<td> libssh2 </td>
<td>
A comma separated list of authentication methods to use. Default (is
- "agent,privkey,keyboard-interactive". The order of the methods is perserved.
+ "agent,privkey,keyboard-interactive". The order of the methods is preserved.
Some methods may require additional parameters.
</td>
</tr>
diff --git a/docs/storage.html.in b/docs/storage.html.in
index 6a32c95..2706bc5 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -581,7 +581,7 @@
</target>
</volume></pre>
- <h3>Example disk attachement</h3>
+ <h3>Example disk attachment</h3>
<p>RBD images can be attached to Qemu guests when Qemu is built
with RBD support. Information about attaching a RBD image to a
guest can be found
--
1.8.1.4
10 years, 11 months
[libvirt] [PATCH] Fix memory leak in virNWFilterDefParseXML()
by Nehal J Wani
While running nwfilterxml2xmltest, it was found that valgrind pointed out the
following error...
==7466== 16 bytes in 1 blocks are definitely lost in loss record 26 of 90
==7466== at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==7466== by 0x4C651AD: virAlloc (viralloc.c:142)
==7466== by 0x4D0450D: virNWFilterDefParseNode (nwfilter_conf.c:2575)
==7466== by 0x4D05D84: virNWFilterDefParse (nwfilter_conf.c:2647)
==7466== by 0x401FDE: testCompareXMLToXMLHelper (nwfilterxml2xmltest.c:39)
==7466== by 0x402DE1: virtTestRun (testutils.c:138)
==7466== by 0x4018E9: mymain (nwfilterxml2xmltest.c:111)
==7466== by 0x403482: virtTestMain (testutils.c:593)
==7466== by 0x341F421A04: (below main) (libc-start.c:225)
...21 times, which are related to 21 tests in nwfilterxml2xmltest.c which sent
EXPECT_WARN = false. There were two scenarios in virNWFilterDefParseXML(),
when the variable 'entry' was malloc'ed, but not freed.
---
src/conf/nwfilter_conf.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 793cb0e..ee26a62 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2576,11 +2576,15 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) {
goto cleanup;
if (xmlStrEqual(curr->name, BAD_CAST "rule")) {
- if (!(entry->rule = virNWFilterRuleParse(curr)))
+ if (!(entry->rule = virNWFilterRuleParse(curr))) {
+ VIR_FREE(entry);
goto cleanup;
+ }
} else if (xmlStrEqual(curr->name, BAD_CAST "filterref")) {
- if (!(entry->include = virNWFilterIncludeParse(curr)))
+ if (!(entry->include = virNWFilterIncludeParse(curr))) {
+ VIR_FREE(entry);
goto cleanup;
+ }
}
if (entry->rule || entry->include) {
--
1.8.1.4
10 years, 12 months
[libvirt] [PATCH v2] virObject: Error on suspicious ref and unref
by Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1033061
Since our transformation into virObject is not complete and we must do
ref and unref ourselves there's a chance that we will get it wrong. That
is, while one thread is doing unref and subsequent dispose another
thread may come and do the ref & unref on stale pointer. This results in
dispose being called twice (and possibly simultaneously). These kind of
errors are hard to catch so we should at least throw an error into logs
if such situation occurs. In fact, I've seen a stack trace showing this
error had happen (obj = 0x7f4968018260):
Thread 2 (Thread 0x7f498596b880 (LWP 13394)):
[...]
#4 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101
__PRETTY_FUNCTION__ = "ncf_close"
#5 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262
klass = 0x7f4968019020
obj = 0x7f4968018260
lastRef = true
__func__ = "virObjectUnref"
#6 0x00007f4970159785 in netcfStateCleanup () at interface/interface_backend_netcf.c:109
No locals.
#7 0x00007f4984fc0e28 in virStateCleanup () at libvirt.c:883
i = 6
ret = 0
#8 0x00007f49859b55fd in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1549
srv = 0x7f498696ed00
remote_config_file = 0x0
statuswrite = -1
ret = <optimized out>
pid_file_fd = 5
pid_file = 0x0
sock_file = 0x0
sock_file_ro = 0x0
timeout = -1
verbose = 0
godaemon = 0
ipsock = 0
config = 0x7f498696e760
privileged = <optimized out>
implicit_conf = <optimized out>
run_dir = 0x0
old_umask = <optimized out>
opts = {{name = 0x7f49859e2b53 "verbose", has_arg = 0, flag = 0x7fff45057808, val = 118}, {name = 0x7f49859e2b5b "daemon", has_arg = 0, flag = 0x7fff4505780c, val = 100}, {name = 0x7f49859e2b62 "listen", has_arg = 0, flag = 0x7fff45057810, val = 108}, {name = 0x7f49859e2c9f "config", has_arg = 1, flag = 0x0, val = 102}, {name = 0x7f49859e2c00 "timeout", has_arg = 1, flag = 0x0, val = 116}, {name = 0x7f49859e2b69 "pid-file", has_arg = 1, flag = 0x0, val = 112}, {name = 0x7f49859e2b72 "version", has_arg = 0, flag = 0x0, val = 86}, {name = 0x7f49859e2b7a "help", has_arg = 0, flag = 0x0, val = 104}, {name = 0x0, has_arg = 0, flag = 0x0, val = 0}}
__func__ = "main"
Thread 1 (Thread 0x7f496d6a4700 (LWP 13405)):
[...]
#6 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101
__PRETTY_FUNCTION__ = "ncf_close"
#7 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262
klass = 0x7f4968019020
obj = 0x7f4968018260
lastRef = true
__func__ = "virObjectUnref"
#8 0x00007f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at interface/interface_backend_netcf.c:265
No locals.
#9 0x00007f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at datatypes.c:149
conn = 0x7f49680a3260
#10 0x00007f4984f3f31b in virObjectUnref (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262
klass = 0x7f49681d6760
obj = 0x7f49680a3260
lastRef = true
__func__ = "virObjectUnref"
#11 0x00007f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) at libvirt.c:1532
__func__ = "virConnectClose"
__FUNCTION__ = "virConnectClose"
#12 0x00007f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) at lxc/lxc_process.c:1426
conn = 0x7f49680a3260
data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260}
#13 0x00007f4984fc0d21 in virStateInitialize (privileged=true, callback=callback@entry=0x7f49859b7180 <daemonInhibitCallback>, opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864
i = 8
__func__ = "virStateInitialize"
#14 0x00007f49859b71db in daemonRunStateInit (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906
srv = 0x7f498696ed00
sysident = 0x7f4968000ae0
__func__ = "daemonRunStateInit"
#15 0x00007f4984f4efe1 in virThreadHelper (data=<optimized out>) at util/virthreadpthread.c:161
args = 0x0
local = {func = 0x7f49859b71a0 <daemonRunStateInit>, opaque = 0x7f498696ed00}
#16 0x00007f4982a40de3 in start_thread (arg=0x7f496d6a4700) at pthread_create.c:308
__res = <optimized out>
pd = 0x7f496d6a4700
now = <optimized out>
unwind_buf = {cancel_jmp_buf = {{jmp_buf = {139953345021696, 7118305506502180663, 0, 139953345022400, 139953345021696, 140734351374104, -7179978120810397897, -7180347023594422473}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
not_first_call = <optimized out>
pagesize_m1 = <optimized out>
sp = <optimized out>
freesize = <optimized out>
#17 0x00007f498236716d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
diff to v1:
- s/virReportError/VIR_ERROR/
- fix bool = int assignment
src/nwfilter/nwfilter_dhcpsnoop.c | 6 ++---
src/util/viratomic.h | 53 +++++++++++++++++++++++++++++++--------
src/util/virobject.c | 13 +++++++---
3 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index e8fcfef..71380bb 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -893,7 +893,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
skip_instantiate:
VIR_FREE(ipl);
- virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases);
+ virAtomicIntDec(&virNWFilterSnoopState.nLeases);
lease_not_found:
VIR_FREE(ipstr);
@@ -1169,7 +1169,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque)
_("Instantiation of rules failed on "
"interface '%s'"), req->ifname);
}
- virAtomicIntDecAndTest(job->qCtr);
+ virAtomicIntDec(job->qCtr);
VIR_FREE(job);
}
@@ -1562,7 +1562,7 @@ exit:
pcap_close(pcapConf[i].handle);
}
- virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads);
+ virAtomicIntDec(&virNWFilterSnoopState.nThreads);
return;
}
diff --git a/src/util/viratomic.h b/src/util/viratomic.h
index 4d7f7e5..48450b9 100644
--- a/src/util/viratomic.h
+++ b/src/util/viratomic.h
@@ -68,6 +68,18 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic)
ATTRIBUTE_NONNULL(1);
/**
+ * virAtomicIntDec:
+ * Decrements the value of atomic by 1.
+ *
+ * Think of this operation as an atomic version of
+ * { *atomic -= 1; return *atomic == 0; }
+ *
+ * This call acts as a full compiler and hardware memory barrier.
+ */
+VIR_STATIC int virAtomicIntDec(volatile int *atomic)
+ ATTRIBUTE_NONNULL(1);
+
+/**
* virAtomicIntDecAndTest:
* Decrements the value of atomic by 1.
*
@@ -75,6 +87,8 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic)
* { *atomic -= 1; return *atomic == 0; }
*
* This call acts as a full compiler and hardware memory barrier.
+ * Returns true if the resulting decremented value is zero,
+ * false otherwise.
*/
VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic)
ATTRIBUTE_NONNULL(1);
@@ -176,12 +190,15 @@ VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic,
(void)(0 ? *(atomic) ^ *(atomic) : 0); \
__sync_add_and_fetch((atomic), 1); \
}))
+# define virAtomicIntDec(atomic) \
+ (__extension__ ({ \
+ (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \
+ (void)(0 ? *(atomic) ^ *(atomic) : 0); \
+ __sync_sub_and_fetch((atomic), 1); \
+ }))
# define virAtomicIntDecAndTest(atomic) \
- (__extension__ ({ \
- (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \
- (void)(0 ? *(atomic) ^ *(atomic) : 0); \
- __sync_fetch_and_sub((atomic), 1) == 1; \
- }))
+ (virAtomicIntDec(atomic) == 0)
+
# define virAtomicIntCompareExchange(atomic, oldval, newval) \
(__extension__ ({ \
(void)verify_true(sizeof(*(atomic)) == sizeof(int)); \
@@ -252,10 +269,16 @@ virAtomicIntInc(volatile int *atomic)
return InterlockedIncrement((volatile LONG *)atomic);
}
+static inline int
+virAtomicIntDec(volatile int *atomic)
+{
+ return InterlockedDecrement((volatile LONG *)atomic);
+}
+
static inline bool
virAtomicIntDecAndTest(volatile int *atomic)
{
- return InterlockedDecrement((volatile LONG *)atomic) == 0;
+ return virAtomicIntDec(atomic) == 0;
}
static inline bool
@@ -334,16 +357,22 @@ virAtomicIntInc(volatile int *atomic)
return value;
}
-static inline bool
-virAtomicIntDecAndTest(volatile int *atomic)
+static inline int
+virAtomicIntDec(volatile int *atomic)
{
- bool is_zero;
+ int value;
pthread_mutex_lock(&virAtomicLock);
- is_zero = --(*atomic) == 0;
+ value = --(*atomic);
pthread_mutex_unlock(&virAtomicLock);
- return is_zero;
+ return value;
+}
+
+static inline bool
+virAtomicIntDecAndTest(volatile int *atomic)
+{
+ return virAtomicIntDec(atomic) == 0;
}
static inline bool
@@ -436,6 +465,8 @@ virAtomicIntXor(volatile unsigned int *atomic,
virAtomicIntSet((int *)atomic, val)
# define virAtomicIntInc(atomic) \
virAtomicIntInc((int *)atomic)
+# define virAtomicIntDec(atomic) \
+ virAtomicIntDec((int *)atomic)
# define virAtomicIntDecAndTest(atomic) \
virAtomicIntDecAndTest((int *)atomic)
# define virAtomicIntCompareExchange(atomic, oldval, newval) \
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 61b5413..a398ab5 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -252,9 +252,9 @@ bool virObjectUnref(void *anyobj)
if (!obj)
return false;
- bool lastRef = virAtomicIntDecAndTest(&obj->refs);
+ int newRefs = virAtomicIntDec(&obj->refs);
PROBE(OBJECT_UNREF, "obj=%p", obj);
- if (lastRef) {
+ if (newRefs == 0) {
PROBE(OBJECT_DISPOSE, "obj=%p", obj);
virClassPtr klass = obj->klass;
while (klass) {
@@ -268,9 +268,11 @@ bool virObjectUnref(void *anyobj)
obj->magic = 0xDEADBEEF;
obj->klass = (void*)0xDEADBEEF;
VIR_FREE(obj);
+ } else if (newRefs < 0) {
+ VIR_ERROR(_("Suspicious unrefing of object %p"), anyobj);
}
- return !lastRef;
+ return newRefs > 0;
}
@@ -289,8 +291,11 @@ void *virObjectRef(void *anyobj)
if (!obj)
return NULL;
- virAtomicIntInc(&obj->refs);
+ bool firstRef = virAtomicIntInc(&obj->refs) <= 1;
PROBE(OBJECT_REF, "obj=%p", obj);
+ if (firstRef) {
+ VIR_ERROR(_("Suspicious refing of object %p"), anyobj);
+ }
return anyobj;
}
--
1.8.4.4
10 years, 12 months
[libvirt] [PATCH] virObject: Error on suspicious ref and unref
by Michal Privoznik
https://bugzilla.redhat.com/show_bug.cgi?id=1033061
Since our transformation into virObject is not complete and we must do
ref and unref ourselves there's a chance that we will get it wrong. That
is, while one thread is doing unref and subsequent dispose another
thread may come and do the ref & unref on stale pointer. This results in
dispose being called twice (and possibly simultaneously). These kind of
errors are hard to catch so we should at least throw an error into logs
if such situation occurs. In fact, I've seen a stack trace showing this
error had happen (obj = 0x7f4968018260):
Thread 2 (Thread 0x7f498596b880 (LWP 13394)):
[...]
#4 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101
__PRETTY_FUNCTION__ = "ncf_close"
#5 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262
klass = 0x7f4968019020
obj = 0x7f4968018260
lastRef = true
__func__ = "virObjectUnref"
#6 0x00007f4970159785 in netcfStateCleanup () at interface/interface_backend_netcf.c:109
No locals.
#7 0x00007f4984fc0e28 in virStateCleanup () at libvirt.c:883
i = 6
ret = 0
#8 0x00007f49859b55fd in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1549
srv = 0x7f498696ed00
remote_config_file = 0x0
statuswrite = -1
ret = <optimized out>
pid_file_fd = 5
pid_file = 0x0
sock_file = 0x0
sock_file_ro = 0x0
timeout = -1
verbose = 0
godaemon = 0
ipsock = 0
config = 0x7f498696e760
privileged = <optimized out>
implicit_conf = <optimized out>
run_dir = 0x0
old_umask = <optimized out>
opts = {{name = 0x7f49859e2b53 "verbose", has_arg = 0, flag = 0x7fff45057808, val = 118}, {name = 0x7f49859e2b5b "daemon", has_arg = 0, flag = 0x7fff4505780c, val = 100}, {name = 0x7f49859e2b62 "listen", has_arg = 0, flag = 0x7fff45057810, val = 108}, {name = 0x7f49859e2c9f "config", has_arg = 1, flag = 0x0, val = 102}, {name = 0x7f49859e2c00 "timeout", has_arg = 1, flag = 0x0, val = 116}, {name = 0x7f49859e2b69 "pid-file", has_arg = 1, flag = 0x0, val = 112}, {name = 0x7f49859e2b72 "version", has_arg = 0, flag = 0x0, val = 86}, {name = 0x7f49859e2b7a "help", has_arg = 0, flag = 0x0, val = 104}, {name = 0x0, has_arg = 0, flag = 0x0, val = 0}}
__func__ = "main"
Thread 1 (Thread 0x7f496d6a4700 (LWP 13405)):
[...]
#6 0x00007f496ff44607 in ncf_close (ncf=0x7f496801e3d0) at netcf.c:101
__PRETTY_FUNCTION__ = "ncf_close"
#7 0x00007f4984f3f31b in virObjectUnref (anyobj=<optimized out>) at util/virobject.c:262
klass = 0x7f4968019020
obj = 0x7f4968018260
lastRef = true
__func__ = "virObjectUnref"
#8 0x00007f4970157752 in netcfInterfaceClose (conn=0x7f49680a3260) at interface/interface_backend_netcf.c:265
No locals.
#9 0x00007f4984fb7984 in virConnectDispose (obj=0x7f49680a3260) at datatypes.c:149
conn = 0x7f49680a3260
#10 0x00007f4984f3f31b in virObjectUnref (anyobj=anyobj@entry=0x7f49680a3260) at util/virobject.c:262
klass = 0x7f49681d6760
obj = 0x7f49680a3260
lastRef = true
__func__ = "virObjectUnref"
#11 0x00007f4984fc11bf in virConnectClose (conn=conn@entry=0x7f49680a3260) at libvirt.c:1532
__func__ = "virConnectClose"
__FUNCTION__ = "virConnectClose"
#12 0x00007f496eced281 in virLXCProcessAutostartAll (driver=0x7f49681ffaa0) at lxc/lxc_process.c:1426
conn = 0x7f49680a3260
data = {driver = 0x7f49681ffaa0, conn = 0x7f49680a3260}
#13 0x00007f4984fc0d21 in virStateInitialize (privileged=true, callback=callback@entry=0x7f49859b7180 <daemonInhibitCallback>, opaque=opaque@entry=0x7f498696ed00) at libvirt.c:864
i = 8
__func__ = "virStateInitialize"
#14 0x00007f49859b71db in daemonRunStateInit (opaque=opaque@entry=0x7f498696ed00) at libvirtd.c:906
srv = 0x7f498696ed00
sysident = 0x7f4968000ae0
__func__ = "daemonRunStateInit"
#15 0x00007f4984f4efe1 in virThreadHelper (data=<optimized out>) at util/virthreadpthread.c:161
args = 0x0
local = {func = 0x7f49859b71a0 <daemonRunStateInit>, opaque = 0x7f498696ed00}
#16 0x00007f4982a40de3 in start_thread (arg=0x7f496d6a4700) at pthread_create.c:308
__res = <optimized out>
pd = 0x7f496d6a4700
now = <optimized out>
unwind_buf = {cancel_jmp_buf = {{jmp_buf = {139953345021696, 7118305506502180663, 0, 139953345022400, 139953345021696, 140734351374104, -7179978120810397897, -7180347023594422473}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
not_first_call = <optimized out>
pagesize_m1 = <optimized out>
sp = <optimized out>
freesize = <optimized out>
#17 0x00007f498236716d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/nwfilter/nwfilter_dhcpsnoop.c | 6 ++---
src/util/viratomic.h | 53 +++++++++++++++++++++++++++++++--------
src/util/virobject.c | 17 ++++++++++---
3 files changed, 58 insertions(+), 18 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index e8fcfef..71380bb 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -893,7 +893,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
skip_instantiate:
VIR_FREE(ipl);
- virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases);
+ virAtomicIntDec(&virNWFilterSnoopState.nLeases);
lease_not_found:
VIR_FREE(ipstr);
@@ -1169,7 +1169,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque)
_("Instantiation of rules failed on "
"interface '%s'"), req->ifname);
}
- virAtomicIntDecAndTest(job->qCtr);
+ virAtomicIntDec(job->qCtr);
VIR_FREE(job);
}
@@ -1562,7 +1562,7 @@ exit:
pcap_close(pcapConf[i].handle);
}
- virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads);
+ virAtomicIntDec(&virNWFilterSnoopState.nThreads);
return;
}
diff --git a/src/util/viratomic.h b/src/util/viratomic.h
index 4d7f7e5..877900e 100644
--- a/src/util/viratomic.h
+++ b/src/util/viratomic.h
@@ -68,6 +68,18 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic)
ATTRIBUTE_NONNULL(1);
/**
+ * virAtomicIntDec:
+ * Decrements the value of atomic by 1.
+ *
+ * Think of this operation as an atomic version of
+ * { *atomic -= 1; return *atomic == 0; }
+ *
+ * This call acts as a full compiler and hardware memory barrier.
+ */
+VIR_STATIC int virAtomicIntDec(volatile int *atomic)
+ ATTRIBUTE_NONNULL(1);
+
+/**
* virAtomicIntDecAndTest:
* Decrements the value of atomic by 1.
*
@@ -75,6 +87,8 @@ VIR_STATIC int virAtomicIntInc(volatile int *atomic)
* { *atomic -= 1; return *atomic == 0; }
*
* This call acts as a full compiler and hardware memory barrier.
+ * Returns true if the resulting decremented value is zero,
+ * false otherwise.
*/
VIR_STATIC bool virAtomicIntDecAndTest(volatile int *atomic)
ATTRIBUTE_NONNULL(1);
@@ -176,12 +190,15 @@ VIR_STATIC unsigned int virAtomicIntXor(volatile unsigned int *atomic,
(void)(0 ? *(atomic) ^ *(atomic) : 0); \
__sync_add_and_fetch((atomic), 1); \
}))
+# define virAtomicIntDec(atomic) \
+ (__extension__ ({ \
+ (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \
+ (void)(0 ? *(atomic) ^ *(atomic) : 0); \
+ __sync_sub_and_fetch((atomic), 1); \
+ }))
# define virAtomicIntDecAndTest(atomic) \
- (__extension__ ({ \
- (void)verify_true(sizeof(*(atomic)) == sizeof(int)); \
- (void)(0 ? *(atomic) ^ *(atomic) : 0); \
- __sync_fetch_and_sub((atomic), 1) == 1; \
- }))
+ (virAtomicIntDec(atomic) == 0)
+
# define virAtomicIntCompareExchange(atomic, oldval, newval) \
(__extension__ ({ \
(void)verify_true(sizeof(*(atomic)) == sizeof(int)); \
@@ -252,10 +269,16 @@ virAtomicIntInc(volatile int *atomic)
return InterlockedIncrement((volatile LONG *)atomic);
}
+static inline int
+virAtomicIntDec(volatile int *atomic)
+{
+ return InterlockedDecrement((volatile LONG *)atomic);
+}
+
static inline bool
virAtomicIntDecAndTest(volatile int *atomic)
{
- return InterlockedDecrement((volatile LONG *)atomic) == 0;
+ return virAtomicIntDec(atomic) == 0;
}
static inline bool
@@ -334,16 +357,22 @@ virAtomicIntInc(volatile int *atomic)
return value;
}
-static inline bool
-virAtomicIntDecAndTest(volatile int *atomic)
+static inline int
+virAtomicIntDec(volatile int *atomic)
{
- bool is_zero;
+ bool value;
pthread_mutex_lock(&virAtomicLock);
- is_zero = --(*atomic) == 0;
+ value = --(*atomic);
pthread_mutex_unlock(&virAtomicLock);
- return is_zero;
+ return value;
+}
+
+static inline int
+virAtomicIntDecAndTest(volatile int *atomic)
+{
+ return virAtomicIntDec(atomic) == 0;
}
static inline bool
@@ -436,6 +465,8 @@ virAtomicIntXor(volatile unsigned int *atomic,
virAtomicIntSet((int *)atomic, val)
# define virAtomicIntInc(atomic) \
virAtomicIntInc((int *)atomic)
+# define virAtomicIntDec(atomic) \
+ virAtomicIntDec((int *)atomic)
# define virAtomicIntDecAndTest(atomic) \
virAtomicIntDecAndTest((int *)atomic)
# define virAtomicIntCompareExchange(atomic, oldval, newval) \
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 61b5413..4299432 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -252,9 +252,9 @@ bool virObjectUnref(void *anyobj)
if (!obj)
return false;
- bool lastRef = virAtomicIntDecAndTest(&obj->refs);
+ int newRefs = virAtomicIntDec(&obj->refs);
PROBE(OBJECT_UNREF, "obj=%p", obj);
- if (lastRef) {
+ if (newRefs == 0) {
PROBE(OBJECT_DISPOSE, "obj=%p", obj);
virClassPtr klass = obj->klass;
while (klass) {
@@ -268,9 +268,13 @@ bool virObjectUnref(void *anyobj)
obj->magic = 0xDEADBEEF;
obj->klass = (void*)0xDEADBEEF;
VIR_FREE(obj);
+ } else if (newRefs < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Suspicious unrefing of object %p"),
+ anyobj);
}
- return !lastRef;
+ return newRefs > 0;
}
@@ -289,8 +293,13 @@ void *virObjectRef(void *anyobj)
if (!obj)
return NULL;
- virAtomicIntInc(&obj->refs);
+ bool firstRef = virAtomicIntInc(&obj->refs) <= 1;
PROBE(OBJECT_REF, "obj=%p", obj);
+ if (firstRef) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Suspicious refing of object %p"),
+ anyobj);
+ }
return anyobj;
}
--
1.8.4.4
10 years, 12 months
[libvirt] What to do about the qemu "-boot strict" option
by Laine Stump
Awhile back a bug was filed against libvirt about the inability to
completely exclude a disk from the boot order:
https://bugzilla.redhat.com/show_bug.cgi?id=888635
In short, you can't have a domain that used PXE to boot, but also has an
un-bootable disk device *even if that disk isn't listed in the boot
order*, because if PXE times out (e.g. due to the bridge forwarding
delay), the BIOS will move on to the next target, which will be the
unbootable disk device (again - even though it wasn't given a boot
order), and get stuck at a "/BOOT DISK FAILURE, PRESS ANY KEY" message
/until a user intervenes.
It was obviously beyond the ability of libvirt to fix this (although it
can be worked around by creating a very small disk image with a
bootloader that merely instructs the system to reboot, and placing
*that* disk in the boot order just after the PXE device), so the BZ was
closed as CANTFIX.
A couple days ago I noticed that Amos Kong had later actually fixed this
problem in seabios and qemu:
https://bugzilla.redhat.com/show_bug.cgi?id=888633
https://bugzilla.redhat.com/show_bug.cgi?id=903204
Existing behavior is preserved though, and the new behavior only comes
about if "-boot strict" is specified on the qemu commandline.
It definitely seems desirable to have this ability in libvirt, but I'm
almost of the opinion that this should *always* be the behavior (if you
want all devices to be in the boot order, you can just give all of them
(or none of them, if you're feeling adventurous) a boot order ranking).
But I thought it would be prudent to ask opinions about that before
making any patch.
So what are the opinions? Should the "if any devices are given a boot
order, only attempt to boot from devices that have a boot order
specified" behavior just be the default (and only) behavior when
qemu/seabios supports it? (this would imply that the old behavior is
just a bug)? Or do we need to make it configurable? If it needs to be
configurable, the boot-related xml seems to be a bit unorganized (a flat
list of elements with mostly a single attribute for each), but I suppose
this could be added as a new attribute to the <bios> element...
10 years, 12 months
[libvirt] [PATCH python] Improve quality of sanitytest check
by Daniel P. Berrange
From: "Daniel P. Berrange" <berrange(a)redhat.com>
Validate that every public API method is mapped into the python
and that every python method has a sane C API.
Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com>
---
sanitytest.py | 309 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
setup.py | 35 +++----
2 files changed, 294 insertions(+), 50 deletions(-)
mode change 100755 => 100644 sanitytest.py
diff --git a/sanitytest.py b/sanitytest.py
old mode 100755
new mode 100644
index 517054b..9e4c261
--- a/sanitytest.py
+++ b/sanitytest.py
@@ -1,40 +1,283 @@
#!/usr/bin/python
import sys
+import lxml
+import lxml.etree
+import string
+# Munge import path to insert build location for libvirt mod
sys.path.insert(0, sys.argv[1])
-
import libvirt
+import libvirtmod
+
+# Path to the libvirt API XML file
+xml = sys.argv[2]
+
+f = open(xml, "r")
+tree = lxml.etree.parse(f)
+
+verbose = False
+
+wantenums = []
+wantfunctions = []
+
+# Phase 1: Identify all functions and enums in public API
+set = tree.xpath('/api/files/file/exports[@type="function"]/@symbol')
+for n in set:
+ wantfunctions.append(n)
+
+set = tree.xpath('/api/files/file/exports[@type="enum"]/@symbol')
+for n in set:
+ wantenums.append(n)
+
+
+# Phase 2: Identify all classes and methods in the 'libvirt' python module
+gotenums = []
+gottypes = []
+gotfunctions = { "libvirt": [] }
+
+for name in dir(libvirt):
+ if name[0] == '_':
+ continue
+ thing = getattr(libvirt, name)
+ if type(thing) == int:
+ gotenums.append(name)
+ elif type(thing) == type:
+ gottypes.append(name)
+ gotfunctions[name] = []
+ elif callable(thing):
+ gotfunctions["libvirt"].append(name)
+ else:
+ pass
+
+for klassname in gottypes:
+ klassobj = getattr(libvirt, klassname)
+ for name in dir(klassobj):
+ if name[0] == '_':
+ continue
+ thing = getattr(klassobj, name)
+ if callable(thing):
+ gotfunctions[klassname].append(name)
+ else:
+ pass
+
+
+# Phase 3: First cut at mapping of C APIs to python classes + methods
+basicklassmap = {}
+
+for cname in wantfunctions:
+ name = cname
+ # Some virConnect APIs have stupid names
+ if name[0:7] == "virNode" and name[0:13] != "virNodeDevice":
+ name = "virConnect" + name[7:]
+ if name[0:7] == "virConn" and name[0:10] != "virConnect":
+ name = "virConnect" + name[7:]
+
+ # The typed param APIs are only for internal use
+ if name[0:14] == "virTypedParams":
+ continue
+
+ # These aren't functions, they're callback signatures
+ if name in ["virConnectAuthCallbackPtr", "virConnectCloseFunc",
+ "virStreamSinkFunc", "virStreamSourceFunc", "virStreamEventCallback",
+ "virEventHandleCallback", "virEventTimeoutCallback", "virFreeCallback"]:
+ continue
+ if name[0:21] == "virConnectDomainEvent" and name[-8:] == "Callback":
+ continue
+
+
+ # virEvent APIs go into main 'libvirt' namespace not any class
+ if name[0:8] == "virEvent":
+ if name[-4:] == "Func":
+ continue
+ basicklassmap[name] = ["libvirt", name, cname]
+ else:
+ found = False
+ # To start with map APIs to classes based on the
+ # naming prefix. Mistakes will be fixed in next
+ # loop
+ for klassname in gottypes:
+ klen = len(klassname)
+ if name[0:klen] == klassname:
+ found = True
+ if name not in basicklassmap:
+ basicklassmap[name] = [klassname, name[klen:], cname]
+ elif len(basicklassmap[name]) < klassname:
+ basicklassmap[name] = [klassname, name[klen:], cname]
+
+ # Anything which can't map to a class goes into the
+ # global namespaces
+ if not found:
+ basicklassmap[name] = ["libvirt", name[3:], cname]
+
+
+# Phase 4: Deal with oh so many special cases in C -> python mapping
+finalklassmap = {}
+
+for name in sorted(basicklassmap):
+ klass = basicklassmap[name][0]
+ func = basicklassmap[name][1]
+ cname = basicklassmap[name][2]
+
+ # The object lifecycle APIs are irrelevant since they're
+ # used inside the object constructors/destructors.
+ if func in ["Ref", "Free", "New", "GetConnect", "GetDomain"]:
+ if klass == "virStream" and func == "New":
+ klass = "virConnect"
+ func = "NewStream"
+ else:
+ continue
+
+
+ # All the error handling methods need special handling
+ if klass == "libvirt":
+ if func in ["CopyLastError", "DefaultErrorFunc",
+ "ErrorFunc", "FreeError",
+ "SaveLastError", "ResetError"]:
+ continue
+ elif func in ["GetLastError", "GetLastErrorMessage", "ResetLastError", "Initialize"]:
+ func = "vir" + func
+ elif func == "SetErrorFunc":
+ func = "RegisterErrorHandler"
+ elif klass == "virConnect":
+ if func in ["CopyLastError", "SetErrorFunc"]:
+ continue
+ elif func in ["GetLastError", "ResetLastError"]:
+ func = "virConn" + func
+
+ # Remove 'Get' prefix from most APIs, except those in virConnect
+ # and virDomainSnapshot namespaces which stupidly used a different
+ # convention which we now can't fix without breaking API
+ if func[0:3] == "Get" and klass not in ["virConnect", "virDomainSnapshot", "libvirt"]:
+ if func not in ["GetCPUStats"]:
+ func = func[3:]
+
+ # The object creation and lookup APIs all have to get re-mapped
+ # into the parent class
+ if func in ["CreateXML", "CreateLinux", "CreateXMLWithFiles",
+ "DefineXML", "CreateXMLFrom", "LookupByUUID",
+ "LookupByUUIDString", "LookupByVolume" "LookupByName",
+ "LookupByID", "LookupByName", "LookupByKey", "LookupByPath",
+ "LookupByMACString", "LookupByUsage", "LookupByVolume",
+ "LookupSCSIHostByWWN", "Restore", "RestoreFlags",
+ "SaveImageDefineXML", "SaveImageGetXMLDesc"]:
+ if klass != "virDomain":
+ func = klass[3:] + func
+
+ if klass == "virDomainSnapshot":
+ klass = "virDomain"
+ func = func[6:]
+ elif klass == "virStorageVol" and func in ["StorageVolCreateXMLFrom", "StorageVolCreateXML"]:
+ klass = "virStoragePool"
+ func = func[10:]
+ elif func == "StoragePoolLookupByVolume":
+ klass = "virStorageVol"
+ elif func == "StorageVolLookupByName":
+ klass = "virStoragePool"
+ else:
+ klass = "virConnect"
+
+ # The open methods get remapped to primary namespace
+ if klass == "virConnect" and func in ["Open", "OpenAuth", "OpenReadOnly"]:
+ klass = "libvirt"
+
+ # These are inexplicably renamed in the python API
+ if func == "ListDomains":
+ func = "ListDomainsID"
+ elif func == "ListAllNodeDevices":
+ func = "ListAllDevices"
+ elif func == "ListNodeDevices":
+ func = "ListDevices"
+
+ # The virInterfaceChangeXXXX APIs go into virConnect. Stupidly
+ # they have lost their 'interface' prefix in names, but we can't
+ # fix this name
+ if func[0:6] == "Change":
+ klass = "virConnect"
+
+ # Need to special case the snapshot APIs
+ if klass == "virDomainSnapshot" and func in ["Current", "ListNames", "Num"]:
+ klass = "virDomain"
+ func = "snapshot" + func
+
+ # Names should stsart with lowercase letter...
+ func = string.lower(func[0:1]) + func[1:]
+ if func[0:8] == "nWFilter":
+ func = "nwfilter" + func[8:]
+
+ # ...except when they don't. More stupid naming
+ # decisions we can't fix
+ if func == "iD":
+ func = "ID"
+ if func == "uUID":
+ func = "UUID"
+ if func == "uUIDString":
+ func = "UUIDString"
+ if func == "oSType":
+ func = "OSType"
+ if func == "xMLDesc":
+ func = "XMLDesc"
+ if func == "mACString":
+ func = "MACString"
+
+ finalklassmap[name] = [klass, func, cname]
+
+
+# Phase 5: Validate sure that every C API is mapped to a python API
+fail = False
+usedfunctions = {}
+for name in sorted(finalklassmap):
+ klass = finalklassmap[name][0]
+ func = finalklassmap[name][1]
+
+ if func in gotfunctions[klass]:
+ usedfunctions["%s.%s" % (klass, func)] = 1
+ if verbose:
+ print "PASS %s -> %s.%s" % (name, klass, func)
+ else:
+ print "FAIL %s -> %s.%s (C API not mapped to python)" % (name, klass, func)
+ fail = True
+
+
+# Phase 6: Validate that every python API has a corresponding C API
+for klass in gotfunctions:
+ if klass == "libvirtError":
+ continue
+ for func in sorted(gotfunctions[klass]):
+ # These are pure python methods with no C APi
+ if func in ["connect", "getConnect", "domain", "getDomain"]:
+ continue
+
+ key = "%s.%s" % (klass, func)
+ if not key in usedfunctions:
+ print "FAIL %s.%s (Python API not mapped to C)" % (klass, func)
+ fail = True
+ else:
+ if verbose:
+ print "PASS %s.%s" % (klass, func)
+
+# Phase 7: Validate that all the low level C APIs have binding
+for name in sorted(finalklassmap):
+ cname = finalklassmap[name][2]
+
+ pyname = cname
+ if pyname == "virSetErrorFunc":
+ pyname = "virRegisterErrorHandler"
+ elif pyname == "virConnectListDomains":
+ pyname = "virConnectListDomainsID"
+
+ # These exist in C and exist in python, but we've got
+ # a pure-python impl so don't check them
+ if name in ["virStreamRecvAll", "virStreamSendAll"]:
+ continue
+
+ try:
+ thing = getattr(libvirtmod, pyname)
+ except AttributeError:
+ print "FAIL libvirtmod.%s (C binding does not exist)" % pyname
+ fail = True
-globals = dir(libvirt)
-
-# Sanity test that the generator hasn't gone wrong
-
-# Look for core classes
-for clsname in ["virConnect",
- "virDomain",
- "virDomainSnapshot",
- "virInterface",
- "virNWFilter",
- "virNodeDevice",
- "virNetwork",
- "virSecret",
- "virStoragePool",
- "virStorageVol",
- "virStream",
- ]:
- assert(clsname in globals)
- assert(object in getattr(libvirt, clsname).__bases__)
-
-# Constants
-assert("VIR_CONNECT_RO" in globals)
-
-# Error related bits
-assert("libvirtError" in globals)
-assert("VIR_ERR_AUTH_FAILED" in globals)
-assert("virGetLastError" in globals)
-
-# Some misc methods
-assert("virInitialize" in globals)
-assert("virEventAddHandle" in globals)
-assert("virEventRegisterDefaultImpl" in globals)
+if fail:
+ sys.exit(1)
+else:
+ sys.exit(0)
diff --git a/setup.py b/setup.py
index 17b4722..bf222f8 100755
--- a/setup.py
+++ b/setup.py
@@ -59,6 +59,20 @@ def get_pkgconfig_data(args, mod, required=True):
return line
+def get_api_xml_files():
+ """Check with pkg-config that libvirt is present and extract
+ the API XML file paths we need from it"""
+
+ libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt")
+
+ offset = libvirt_api.index("-api.xml")
+ libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml"
+
+ offset = libvirt_api.index("-api.xml")
+ libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml"
+
+ return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api)
+
ldflags = get_pkgconfig_data(["--libs-only-L"], "libvirt", False)
cflags = get_pkgconfig_data(["--cflags"], "libvirt", False)
@@ -105,23 +119,8 @@ if have_libvirt_lxc:
class my_build(build):
- def get_api_xml_files(self):
- """Check with pkg-config that libvirt is present and extract
- the API XML file paths we need from it"""
-
- libvirt_api = get_pkgconfig_data(["--variable", "libvirt_api"], "libvirt")
-
- offset = libvirt_api.index("-api.xml")
- libvirt_qemu_api = libvirt_api[0:offset] + "-qemu-api.xml"
-
- offset = libvirt_api.index("-api.xml")
- libvirt_lxc_api = libvirt_api[0:offset] + "-lxc-api.xml"
-
- return (libvirt_api, libvirt_qemu_api, libvirt_lxc_api)
-
-
def run(self):
- apis = self.get_api_xml_files()
+ apis = get_api_xml_files()
self.spawn(["python", "generator.py", "libvirt", apis[0]])
self.spawn(["python", "generator.py", "libvirt-qemu", apis[1]])
@@ -266,7 +265,9 @@ class my_test(Command):
Run test suite
"""
- self.spawn(["python", "sanitytest.py", self.build_platlib])
+ apis = get_api_xml_files()
+
+ self.spawn(["python", "sanitytest.py", self.build_platlib, apis[0]])
class my_clean(clean):
--
1.8.3.1
10 years, 12 months