- Devel - libvirt List Archives

[libvirt] [PATCH] doc: Fix spelling of virBufferVSprintf
by Jiri Denemark 26 Nov '10

26 Nov '10

--- Pushed under the trivial rule. HACKING | 2 +- docs/hacking.html.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/HACKING b/HACKING index cd7559d..d646709 100644 --- a/HACKING +++ b/HACKING @@ -508,7 +508,7 @@ Typical usage is as follows: ... virBufferAddLit(&buf, "<domain>\n"); - virBufferVSprint(&buf, " <memory>%d</memory>\n", memory); + virBufferVSprintf(&buf, " <memory>%d</memory>\n", memory); ... virBufferAddLit(&buf, "</domain>\n"); diff --git a/docs/hacking.html.in b/docs/hacking.html.in index ac16f41..900e242 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -606,7 +606,7 @@ ... virBufferAddLit(&buf, "<domain>\n"); - virBufferVSprint(&buf, " <memory>%d</memory>\n", memory); + virBufferVSprintf(&buf, " <memory>%d</memory>\n", memory); ... virBufferAddLit(&buf, "</domain>\n"); -- 1.7.3.2

1 0

[libvirt] Valid characters in domain names?
by Richard W.M. Jones 25 Nov '10

25 Nov '10

Does libvirt enforce any sort of validity of characters in guest names? Someone tried to create a domain called '#' (the single hash character) and noted that this caused failures in virt-tools: https://bugzilla.redhat.com/show_bug.cgi?id=639601 https://bugzilla.redhat.com/show_bug.cgi?id=639602 Had a look at the code but couldn't see anything obvious: It seems like libvirt delegates this entirely to the drivers, the drivers (probably) all call virDomainDefParseXML, and this function does no checking that I could see. If my analysis is correct, this could be dangerous. What if the name contains a character that is special to the qemu command line (','), to XML ('>'), or to C ()? As an example, the code already does: char *virDomainDefFormat(virDomainDefPtr def, int flags) { ... virBufferEscapeString(&buf, " <name>%s</name>\n", def->name); ... } Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/

8 24

[libvirt] [PATCH] Fix host CPU counting on unusual NUMA topologies
by Jiri Denemark 25 Nov '10

25 Nov '10

The nodeinfo structure includes nodes : the number of NUMA cell, 1 for uniform mem access sockets : number of CPU socket per node cores : number of core per socket threads : number of threads per core which does not work well for NUMA topologies where each node does not consist of integral number of CPU sockets. We also have VIR_NODEINFO_MAXCPUS macro in public libvirt.h which computes maximum number of CPUs as (nodes * sockets * cores * threads). As a result, we can't just change sockets to report total number of sockets instead of sockets per node. This would probably be the easiest since I doubt anyone is using the field directly. But because of the macro, some apps might be using sockets indirectly. This patch leaves sockets to be the number of CPU sockets per node (and fixes qemu driver to comply with this) on machines where sockets can be divided by nodes. If we can't divide sockets by nodes, we behave as if there was just one NUMA node containing all sockets. Apps interested in NUMA should consult capabilities XML, which is what they probably do anyway. This way, the only case in which apps that care about NUMA may break is on machines with funky NUMA topology. And there is a chance libvirt wasn't able to start any guests on those machines anyway (although it depends on the topology, total number of CPUs and kernel version). Nothing changes at all for apps that don't care about NUMA. Notes: * Testing on 4 sockets, 12 cores each, 8 NUMA nodes Xen (RHEL-5) hypervisor with numa=on: - xm info nr_cpus : 48 nr_nodes : 8 sockets_per_node : 0 cores_per_socket : 12 threads_per_core : 1 - virsh nodeinfo CPU(s): 48 CPU socket(s): 4 Core(s) per socket: 12 Thread(s) per core: 1 NUMA cell(s): 1 - virsh capabilities /capabilities/host/topology/cells@num = 8 QEMU driver: - virsh nodeinfo CPU(s): 48 CPU socket(s): 4 Core(s) per socket: 12 Thread(s) per core: 1 NUMA cell(s): 1 - virsh capabilities /capabilities/host/topology/cells@num = 8 * 2 sockets, 4 cores each, 2 NUMA nodes Xen (RHEL-5) hypervisor with numa=on: - xm info nr_cpus : 8 nr_nodes : 2 sockets_per_node : 1 cores_per_socket : 4 threads_per_core : 1 - virsh nodeinfo CPU(s): 8 CPU socket(s): 1 Core(s) per socket: 4 Thread(s) per core: 1 NUMA cell(s): 2 - virsh capabilities /capabilities/host/topology/cells@num = 2 QEMU driver: - virsh nodeinfo CPU(s): 8 CPU socket(s): 1 Core(s) per socket: 4 Thread(s) per core: 1 NUMA cell(s): 2 - virsh capabilities /capabilities/host/topology/cells@num = 2 * uniform memory architecture, 2 sockets, 4 cores each Xen (RHEL-5) hypervisor: - xm info nr_cpus : 8 nr_nodes : 1 sockets_per_node : 2 cores_per_socket : 4 threads_per_core : 1 - virsh nodeinfo CPU(s): 8 CPU socket(s): 2 Core(s) per socket: 4 Thread(s) per core: 1 NUMA cell(s): 1 - virsh capabilities /capabilities/host/topology/cells@num = 1 Xen (upstream) hypervisor: - xm info nr_cpus : 8 nr_nodes : 1 cores_per_socket : 4 threads_per_core : 1 - virsh nodeinfo CPU(s): 8 CPU socket(s): 2 Core(s) per socket: 4 Thread(s) per core: 1 NUMA cell(s): 1 - virsh capabilities /capabilities/host/topology/cells@num = 1 QEMU driver: - virsh nodeinfo CPU(s): 8 CPU socket(s): 2 Core(s) per socket: 4 Thread(s) per core: 1 NUMA cell(s): 1 - virsh capabilities /capabilities/host/topology/cells@num = 1 --- include/libvirt/libvirt.h.in | 9 ++++++--- src/nodeinfo.c | 10 ++++++++++ src/xen/xend_internal.c | 19 ++++++++++++++----- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 716f7af..395a9f8 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -219,9 +219,12 @@ struct _virNodeInfo { unsigned long memory;/* memory size in kilobytes */ unsigned int cpus; /* the number of active CPUs */ unsigned int mhz; /* expected CPU frequency */ - unsigned int nodes; /* the number of NUMA cell, 1 for uniform mem access */ - unsigned int sockets;/* number of CPU socket per node */ - unsigned int cores; /* number of core per socket */ + unsigned int nodes; /* the number of NUMA cell, 1 for unusual NUMA + topologies or uniform memory access; check + capabilities XML for the actual NUMA topology */ + unsigned int sockets;/* number of CPU sockets per node if nodes == 1, + total number of CPU sockets otherwise */ + unsigned int cores; /* number of cores per socket */ unsigned int threads;/* number of threads per core */ }; diff --git a/src/nodeinfo.c b/src/nodeinfo.c index 9be2a02..acd3188 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -305,6 +305,16 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, return -1; } + /* nodeinfo->sockets is supposed to be a number of sockets per NUMA node, + * however if NUMA nodes are not composed of whole sockets, we just lie + * about the number of NUMA nodes and force apps to check capabilities XML + * for the actual NUMA topology. + */ + if (nodeinfo->sockets % nodeinfo->nodes == 0) + nodeinfo->sockets /= nodeinfo->nodes; + else + nodeinfo->nodes = 1; + return 0; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 4450195..6ce0c3f 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2497,12 +2497,21 @@ sexpr_to_xend_node_info(const struct sexpr *root, virNodeInfoPtr info) if (procs == 0) /* Sanity check in case of Xen bugs in futures..*/ return (-1); info->sockets = nr_cpus / procs; - /* Should already be fine, but for further sanity make - * sure we have at least one socket - */ - if (info->sockets == 0) - info->sockets = 1; } + + /* On systems where NUMA nodes are not composed of whole sockets either Xen + * provided us wrong number of sockets per node or we computed the wrong + * number in the compatibility code above. In such case, we compute the + * correct number of sockets on the host, lie about the number of NUMA + * nodes, and force apps to check capabilities XML for the actual NUMA + * topology. + */ + if (info->nodes * info->sockets * info->cores * info->threads + != info->cpus) { + info->nodes = 1; + info->sockets = info->cpus / (info->cores * info->threads); + } + return (0); } -- 1.7.3.2

2 3

[libvirt] [PATCH] Fix broken XML entity for '>'
by Philipp Hahn 25 Nov '10

25 Nov '10

Add missing 'g' to '>' of '<product />' source-element. Signed-off-by: Philipp Hahn <hahn(a)univention.de> --- docs/formatstorage.html.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

2 1

[libvirt] [PATCH] qemu: mark domain as updated after config updating happened
by Osier Yang 25 Nov '10

25 Nov '10

* src/qemu/qemu_driver.c (qemudDomainDetachDevice, qemudDomainAttachDevice, qemuDomainUpdateDeviceFlags) --- src/qemu/qemu_driver.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f00d8a3..b55e189 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8864,6 +8864,9 @@ static int qemudDomainAttachDevice(virDomainPtr dom, goto endjob; } + if (ret == 0 && !vm->updated) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9074,6 +9077,9 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, break; } + if (ret == 0 && !vm->updated) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; @@ -9748,6 +9754,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom, "%s", _("This type of device cannot be hot unplugged")); } + if (ret == 0 && !vm->updated) + vm->updated = 1; + if (!ret && virDomainSaveStatus(driver->caps, driver->stateDir, vm) < 0) ret = -1; -- 1.7.3.2

2 2

[libvirt] [PATCH] Implementations of virDomainIsUpdated for drivers except qemu
by Osier Yang 25 Nov '10

25 Nov '10

Except LXC and UML driver, implementations of all other drivers return 0 simpley, because these drivers doesn't have config both in memory and on disk, no need to track if the domain of these drivers updated or not. Rename "xenUnifiedDomainisPersistent" to "xenUnifiedDomainIsPersistent" * esx/esx_driver.c * lxc/lxc_driver.c * opennebula/one_driver.c * openvz/openvz_driver.c * phyp/phyp_driver.c * test/test_driver.c * uml/uml_driver.c * vbox/vbox_tmpl.c * xen/xen_driver.c * xenapi/xenapi_driver.c --- src/esx/esx_driver.c | 8 ++++++-- src/lxc/lxc_driver.c | 25 ++++++++++++++++++++++++- src/opennebula/one_driver.c | 6 +++++- src/openvz/openvz_driver.c | 6 +++++- src/phyp/phyp_driver.c | 7 ++++++- src/test/test_driver.c | 7 ++++++- src/uml/uml_driver.c | 24 +++++++++++++++++++++--- src/vbox/vbox_tmpl.c | 6 +++++- src/xen/xen_driver.c | 12 +++++++++--- src/xenapi/xenapi_driver.c | 8 +++++++- 10 files changed, 94 insertions(+), 15 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 5c04596..8ea6219 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3765,7 +3765,11 @@ esxDomainIsPersistent(virDomainPtr domain ATTRIBUTE_UNUSED) return 1; } - +static int +esxDomainIsUpdated(virDomainPtr domain ATTRIBUTE_UNUSED) +{ + return 0; +} static virDomainSnapshotPtr esxDomainSnapshotCreateXML(virDomainPtr domain, const char *xmlDesc, @@ -4360,7 +4364,7 @@ static virDriver esxDriver = { esxIsSecure, /* isSecure */ esxDomainIsActive, /* domainIsActive */ esxDomainIsPersistent, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + esxDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 886286b..eb58086 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -325,6 +325,29 @@ cleanup: return ret; } +static int lxcDomainIsUpdated(virDomainPtr dom) +{ + lxc_driver_t *driver = dom->conn->privateData; + virDomainObjPtr obj; + int ret = -1; + + lxcDriverLock(driver); + obj = virDomainFindByUUID(&driver->domains, dom->uuid); + lxcDriverUnlock(driver); + if (!obj) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + lxcError(VIR_ERR_NO_DOMAIN, + _("No domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + ret = obj->updated; + +cleanup: + if (obj) + virDomainObjUnlock(obj); + return ret; +} static int lxcListDomains(virConnectPtr conn, int *ids, int nids) { lxc_driver_t *driver = conn->privateData; @@ -2882,7 +2905,7 @@ static virDriver lxcDriver = { lxcIsSecure, /* isSecure */ lxcDomainIsActive, /* domainIsActive */ lxcDomainIsPersistent, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + lxcDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/opennebula/one_driver.c b/src/opennebula/one_driver.c index 4fe7f9b..c31f132 100644 --- a/src/opennebula/one_driver.c +++ b/src/opennebula/one_driver.c @@ -106,6 +106,10 @@ static int oneIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) return 0; } +static int oneIsUpdated(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} static virDomainPtr oneDomainLookupByID(virConnectPtr conn, int id) @@ -800,7 +804,7 @@ static virDriver oneDriver = { oneIsSecure, /* isSecure */ NULL, /* domainIsActive */ NULL, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + oneIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 5089e37..807bb7c 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -459,6 +459,10 @@ cleanup: return ret; } +static int openvzDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) +{ + return 0; +} static char *openvzDomainDumpXML(virDomainPtr dom, int flags) { struct openvz_driver *driver = dom->conn->privateData; @@ -1670,7 +1674,7 @@ static virDriver openvzDriver = { openvzIsSecure, openvzDomainIsActive, openvzDomainIsPersistent, - NULL, /* domainIsUpdated */ + openvzDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 24b426e..08faa9b 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1277,6 +1277,11 @@ phypIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) return 1; } +static int +phypIsSecure(virConnectPtr conn ATTRIBUTE_UNUSED) +{ + return 0; +} /* return the lpar_id given a name and a managed system name */ static int @@ -4021,7 +4026,7 @@ static virDriver phypDriver = { phypIsSecure, /* isSecure */ NULL, /* domainIsActive */ NULL, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + phypIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b6838c2..ddff160 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1295,6 +1295,11 @@ cleanup: return ret; } +static int testDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) +{ + return 0; +} + static virDomainPtr testDomainCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) @@ -5413,7 +5418,7 @@ static virDriver testDriver = { testIsSecure, /* isEncrypted */ testDomainIsActive, /* domainIsActive */ testDomainIsPersistent, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + testDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 6c28c76..26b6b50 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1213,8 +1213,6 @@ cleanup: virDomainObjUnlock(obj); return ret; } - - static int umlDomainIsPersistent(virDomainPtr dom) { struct uml_driver *driver = dom->conn->privateData; @@ -1236,6 +1234,26 @@ cleanup: return ret; } +static int umlDomainIsUpdated(virDomainPtr dom) +{ + struct uml_driver *driver = dom->conn->privateData; + virDomainObjPtr obj; + int ret = -1; + + umlDriverLock(driver); + obj = virDomainFindByUUID(&driver->domains, dom->uuid); + umlDriverUnlock(driver); + if (!obj) { + umlReportError(VIR_ERR_NO_DOMAIN, NULL); + goto cleanup; + } + ret = obj->updated; + +cleanup: + if (obj) + virDomainObjUnlock(obj); + return ret; +} static int umlGetVersion(virConnectPtr conn, unsigned long *version) { struct uml_driver *driver = conn->privateData; @@ -2248,7 +2266,7 @@ static virDriver umlDriver = { umlIsSecure, /* isSecure */ umlDomainIsActive, /* domainIsActive */ umlDomainIsPersistent, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + umlDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 0a7a247..ada71b4 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1347,6 +1347,10 @@ static int vboxDomainIsPersistent(virDomainPtr dom ATTRIBUTE_UNUSED) { } +static int vboxDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) { + return 0; +} + static int vboxDomainSuspend(virDomainPtr dom) { VBOX_OBJECT_CHECK(dom->conn, int, -1); IMachine *machine = NULL; @@ -8451,7 +8455,7 @@ virDriver NAME(Driver) = { vboxIsSecure, /* isSecure */ vboxDomainIsActive, /* domainIsActive */ vboxDomainIsPersistent, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + vboxDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 959cc7d..4c11b11 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -740,7 +740,7 @@ xenUnifiedDomainIsActive(virDomainPtr dom) } static int -xenUnifiedDomainisPersistent(virDomainPtr dom) +xenUnifiedDomainIsPersistent(virDomainPtr dom) { GET_PRIVATE(dom->conn); virDomainPtr currdom = NULL; @@ -791,6 +791,12 @@ done: } static int +xenUnifiedDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) +{ + return 0; +} + +static int xenUnifiedDomainSuspend (virDomainPtr dom) { GET_PRIVATE(dom->conn); @@ -2069,8 +2075,8 @@ static virDriver xenUnifiedDriver = { xenUnifiedIsEncrypted, /* isEncrypted */ xenUnifiedIsSecure, /* isSecure */ xenUnifiedDomainIsActive, /* domainIsActive */ - xenUnifiedDomainisPersistent, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + xenUnifiedDomainIsPersistent, /* domainIsPersistent */ + xenUnifiedDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index dec2d25..6fff276 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1745,6 +1745,12 @@ xenapiNodeGetFreeMemory (virConnectPtr conn) return freeMem; } +static int +xenapiDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) +{ + return 0; +} + /* * xenapiNodeGetCellsFreeMemory * @@ -1847,7 +1853,7 @@ static virDriver xenapiDriver = { NULL, /* isSecure */ NULL, /* domainIsActive */ NULL, /* domainIsPersistent */ - NULL, /* domainIsUpdated */ + xenapiDomainIsUpdated, /* domainIsUpdated */ NULL, /* cpuCompare */ NULL, /* cpuBaseline */ NULL, /* domainGetJobInfo */ -- 1.7.3.2

2 2

[libvirt] [PATCH] build: enforce files.h usage
by Eric Blake 25 Nov '10

25 Nov '10

* cfg.mk (sc_prohibit_close): New syntax-check rule. * src/util/pci.c (pciWaitForDeviceCleanup): Fix violation. * .x-sc_prohibit_close: New exceptions. * Makefile.am (EXTRA_DIST): Distribute new file. --- As promised, here's my followup to Stefan's recent close() cleanups, which will help us avoid further regressions. .x-sc_prohibit_close | 3 +++ Makefile.am | 1 + cfg.mk | 9 +++++++++ src/util/pci.c | 2 +- 4 files changed, 14 insertions(+), 1 deletions(-) create mode 100644 .x-sc_prohibit_close diff --git a/.x-sc_prohibit_close b/.x-sc_prohibit_close new file mode 100644 index 0000000..348200c --- /dev/null +++ b/.x-sc_prohibit_close @@ -0,0 +1,3 @@ +^docs/.* +^HACKING$ +^src/util/files.c$ diff --git a/Makefile.am b/Makefile.am index d3f8876..bf1b49b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -26,6 +26,7 @@ EXTRA_DIST = \ .x-sc_bindtextdomain \ .x-sc_m4_quote_check \ .x-sc_prohibit_asprintf \ + .x-sc_prohibit_close \ .x-sc_prohibit_empty_lines_at_EOF \ .x-sc_prohibit_gethostby \ .x-sc_prohibit_gethostname \ diff --git a/cfg.mk b/cfg.mk index 0851f44..963c7db 100644 --- a/cfg.mk +++ b/cfg.mk @@ -231,6 +231,15 @@ sc_avoid_write: halt='consider using safewrite instead of write' \ $(_sc_search_regexp) +# Avoid functions that can lead to double-close bugs. +sc_prohibit_close: + @prohibit='\<[f]close *\(' \ + halt='use VIR_{FORCE_}[F]CLOSE instead of [f]close' \ + $(_sc_search_regexp) + @prohibit='\<fdopen *\(' \ + halt='use VIR_FDOPEN instead of fdopen' \ + $(_sc_search_regexp) + # Similar to the gnulib maint.mk rule for sc_prohibit_strcmp # Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0. sc_prohibit_strncmp: diff --git a/src/util/pci.c b/src/util/pci.c index bd8c6c5..d38cefa 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1095,7 +1095,7 @@ pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher) } } - fclose(fp); + VIR_FORCE_FCLOSE(fp); return ret; } -- 1.7.3.2

2 2

[libvirt] [PATCH] maint: ensure syntax check exceptions are distributed
by Eric Blake 25 Nov '10

25 Nov '10

* Makefile.am (EXTRA_DIST): Factor exceptions files... (syntax_check_excpetions): into new list. Include recently added exceptions. * cfg.mk (sc_x_sc_dist_check): New check, copied from coreutils. --- Makefile.am | 19 +++++++++++++------ cfg.mk | 13 +++++++++++++ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/Makefile.am b/Makefile.am index f3dbd6d..0a3c24f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -14,16 +14,13 @@ XML_EXAMPLES = \ $(patsubst $(srcdir)/%,%,$(wildcard $(addprefix $(srcdir)/examples/xml/, \ test/*.xml storage/*.xml))) -EXTRA_DIST = \ - ChangeLog-old \ - libvirt.spec libvirt.spec.in \ - mingw32-libvirt.spec.in \ - libvirt.pc libvirt.pc.in \ - autobuild.sh \ +syntax_check_exceptions = \ .x-sc_avoid_ctype_macros \ .x-sc_avoid_if_before_free \ .x-sc_avoid_write \ .x-sc_m4_quote_check \ + .x-sc_po_check \ + .x-sc_prohibit_always_true_header_tests \ .x-sc_prohibit_asprintf \ .x-sc_prohibit_empty_lines_at_EOF \ .x-sc_prohibit_gethostby \ @@ -32,6 +29,7 @@ EXTRA_DIST = \ .x-sc_prohibit_have_config_h \ .x-sc_prohibit_HAVE_MBRTOWC \ .x-sc_prohibit_nonreentrant \ + .x-sc_prohibit_readlink \ .x-sc_prohibit_strcmp \ .x-sc_prohibit_strcmp_and_strncmp \ .x-sc_prohibit_strncpy \ @@ -40,9 +38,18 @@ EXTRA_DIST = \ .x-sc_require_config_h \ .x-sc_require_config_h_first \ .x-sc_trailing_blank \ + .x-sc_unmarked_diagnostics + +EXTRA_DIST = \ + ChangeLog-old \ + libvirt.spec libvirt.spec.in \ + mingw32-libvirt.spec.in \ + libvirt.pc libvirt.pc.in \ + autobuild.sh \ Makefile.nonreentrant \ autogen.sh \ examples/domain-events/events-python \ + $(syntax_check_exceptions) \ $(XML_EXAMPLES) pkgconfigdir = $(libdir)/pkgconfig diff --git a/cfg.mk b/cfg.mk index 072673e..d47af80 100644 --- a/cfg.mk +++ b/cfg.mk @@ -434,6 +434,19 @@ sc_prohibit_gettext_markup: halt='do not mark these strings for translation' \ $(_sc_search_regexp) +# Ensure that the syntax_check_exceptions file list in Makefile.am +# stays in sync with corresponding files in the repository. +sce = syntax_check_exceptions +sc_x_sc_dist_check: + @test "$$( ($(VC_LIST) | sed -n '/\.x-sc_/p' \ + | sed 's|^$(_dot_escaped_srcdir)/||'; \ + sed -n '/^$(sce) =[ ]*\\$$/,/[^\]$$/p' \ + $(srcdir)/Makefile.am \ + | sed 's/^ *//;/^$(sce) =/d' \ + | tr -s '\012\\' ' ' | fmt -1 \ + ) | sort | uniq -u)" \ + && { echo 'Makefile.am: $(sce) mismatch' >&2; exit 1; } || :; + # We don't use this feature of maint.mk. prev_version_file = /dev/null -- 1.7.3.2

2 2

[libvirt] [PATCH] Always close drivers when a virConnectPtr is released
by Matthias Bolte 25 Nov '10

25 Nov '10

virConnectClose calls virUnrefConnect which in turn closes all open drivers when the refcount of that connection dropped to zero. This works fine when you free all other objects that hold a ref to the connection before you close it, because in this case virUnrefConnect is the one that removes the last ref to the connection. But it doesn't work when you close the connection first before freeing the other objects. This is because the other virUnref* functions call virReleaseConnect when they detect that the connection's refcount dropped to zero. In this case another virUnref* function (different from virUnrefConnect) removes the last ref to the connection. This results in not closing the open drivers and leaking things that should have been cleaned up in the driver close functions. To fix this move the driver close calls to virReleaseConnect. --- src/datatypes.c | 47 +++++++++++++++++++++++------------------------ 1 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index 46009ae..9817538 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -247,6 +247,29 @@ failed: static void virReleaseConnect(virConnectPtr conn) { DEBUG("release connection %p", conn); + + /* make sure to release the connection lock before we call the + * close() callbacks, otherwise we will deadlock if an error + * is raised by any of the callbacks */ + virMutexUnlock(&conn->lock); + + if (conn->networkDriver) + conn->networkDriver->close (conn); + if (conn->interfaceDriver) + conn->interfaceDriver->close (conn); + if (conn->storageDriver) + conn->storageDriver->close (conn); + if (conn->deviceMonitor) + conn->deviceMonitor->close (conn); + if (conn->secretDriver) + conn->secretDriver->close (conn); + if (conn->nwfilterDriver) + conn->nwfilterDriver->close (conn); + if (conn->driver) + conn->driver->close (conn); + + virMutexLock(&conn->lock); + if (conn->domains != NULL) virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName); if (conn->networks != NULL) @@ -295,30 +318,6 @@ virUnrefConnect(virConnectPtr conn) { conn->refs--; refs = conn->refs; if (refs == 0) { - /* make sure to release the connection lock before we call the - * close() callbacks, otherwise we will deadlock if an error - * is raised by any of the callbacks - */ - virMutexUnlock(&conn->lock); - if (conn->networkDriver) - conn->networkDriver->close (conn); - if (conn->interfaceDriver) - conn->interfaceDriver->close (conn); - if (conn->storageDriver) - conn->storageDriver->close (conn); - if (conn->deviceMonitor) - conn->deviceMonitor->close (conn); - if (conn->secretDriver) - conn->secretDriver->close (conn); - if (conn->nwfilterDriver) - conn->nwfilterDriver->close (conn); - if (conn->driver) - conn->driver->close (conn); - - /* reacquire the connection lock since virReleaseConnect expects - * it to already be held - */ - virMutexLock(&conn->lock); virReleaseConnect(conn); /* Already unlocked mutex */ return (0); -- 1.7.0.4

2 2

[libvirt] ruby-libvirt issue
by Jaromír Červenka 25 Nov '10

25 Nov '10

Good evening, could anybody explain me this, please? divinus:~ # virsh error: cannot recv data: : Connection reset by peer error: failed to connect to the hypervisor divinus:~ # I am trying to imlement Chris's ruby-libvirt to my Ruby on Rails application. First few connection to libvirt daemon (thru ruby-libvirt API) from my app are ok, but then somethings happend and it's not possible to connect anymore until I restart libvirt daemon manually by init.d script or until I restart apache (or WEBrick). And it's not possible connect to virsh from shell also! See messages above. When I try ruby-libvirt from bare Ruby script, everythig is fine - doesn't matter how many times I connect. It's strange. Does anybody have experience with ruby-libvirt in RoR application? Thank you. Jaromir.

6 11