- Devel - libvirt List Archives

[libvirt] [PATCH 0/4] Fixes to some bugs encountered testing virInterface*
by Laine Stump 23 Jul '09

23 Jul '09

This series of patches fixes some problems I found while testing the virInterface implementation. Note that the 3rd patch in the series only fixes virInterface-related instances of the problem (releasing the conn lock before reporting errors). If that patch is approved, a similar thing must be done for several other functions in datatypes.c.

3 18

[libvirt] [PATCH] Release conn lock before reporting errors
by Laine Stump 23 Jul '09

23 Jul '09

This is a follow-on to 528d37. It's fixing several other cases in datatypes.c where we try to report an error while holding the conn's lock, which can't work because the error reporting also tries to lock the conn. --- src/datatypes.c | 133 ++++++++++++++++++++++++++++++++++--------------------- 1 files changed, 82 insertions(+), 51 deletions(-) diff --git a/src/datatypes.c b/src/datatypes.c index ba5401d..c2030dd 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -270,11 +270,13 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { /* TODO check the UUID */ if (ret == NULL) { if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } ret->name = strdup(name); if (ret->name == NULL) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } @@ -285,6 +287,7 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); if (virHashAddEntry(conn->domains, name, ret) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("failed to add domain to connection hash table")); goto error; @@ -299,7 +302,6 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { return(ret); error: - virMutexUnlock(&conn->lock); if (ret != NULL) { VIR_FREE(ret->name); VIR_FREE(ret); @@ -325,24 +327,28 @@ virReleaseDomain(virDomainPtr domain) { DEBUG("release domain %p %s", domain, domain->name); /* TODO search by UUID first as they are better differenciators */ - if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0) + if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("domain missing from connection hash table")); + conn = NULL; + } domain->magic = -1; domain->id = -1; VIR_FREE(domain->name); VIR_FREE(domain); - DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; + if (conn) { + DEBUG("unref connection %p %d", conn, conn->refs); + conn->refs--; + if (conn->refs == 0) { + virReleaseConnect(conn); + /* Already unlocked mutex */ + return; + } + virMutexUnlock(&conn->lock); } - - virMutexUnlock(&conn->lock); } @@ -406,11 +412,13 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { /* TODO check the UUID */ if (ret == NULL) { if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } ret->name = strdup(name); if (ret->name == NULL) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } @@ -420,6 +428,7 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); if (virHashAddEntry(conn->networks, name, ret) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("failed to add network to connection hash table")); goto error; @@ -431,7 +440,6 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { return(ret); error: - virMutexUnlock(&conn->lock); if (ret != NULL) { VIR_FREE(ret->name); VIR_FREE(ret); @@ -457,23 +465,27 @@ virReleaseNetwork(virNetworkPtr network) { DEBUG("release network %p %s", network, network->name); /* TODO search by UUID first as they are better differenciators */ - if (virHashRemoveEntry(conn->networks, network->name, NULL) < 0) + if (virHashRemoveEntry(conn->networks, network->name, NULL) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("network missing from connection hash table")); + conn = NULL; + } network->magic = -1; VIR_FREE(network->name); VIR_FREE(network); - DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; + if (conn) { + DEBUG("unref connection %p %d", conn, conn->refs); + conn->refs--; + if (conn->refs == 0) { + virReleaseConnect(conn); + /* Already unlocked mutex */ + return; + } + virMutexUnlock(&conn->lock); } - - virMutexUnlock(&conn->lock); } @@ -714,11 +726,13 @@ virGetStoragePool(virConnectPtr conn, const char *name, const unsigned char *uui /* TODO check the UUID */ if (ret == NULL) { if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } ret->name = strdup(name); if (ret->name == NULL) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } @@ -728,6 +742,7 @@ virGetStoragePool(virConnectPtr conn, const char *name, const unsigned char *uui memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); if (virHashAddEntry(conn->storagePools, name, ret) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("failed to add storage pool to connection hash table")); goto error; @@ -739,7 +754,6 @@ virGetStoragePool(virConnectPtr conn, const char *name, const unsigned char *uui return(ret); error: - virMutexUnlock(&conn->lock); if (ret != NULL) { VIR_FREE(ret->name); VIR_FREE(ret); @@ -766,23 +780,27 @@ virReleaseStoragePool(virStoragePoolPtr pool) { DEBUG("release pool %p %s", pool, pool->name); /* TODO search by UUID first as they are better differenciators */ - if (virHashRemoveEntry(conn->storagePools, pool->name, NULL) < 0) + if (virHashRemoveEntry(conn->storagePools, pool->name, NULL) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("pool missing from connection hash table")); + conn = NULL; + } pool->magic = -1; VIR_FREE(pool->name); VIR_FREE(pool); - DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; + if (conn) { + DEBUG("unref connection %p %d", conn, conn->refs); + conn->refs--; + if (conn->refs == 0) { + virReleaseConnect(conn); + /* Already unlocked mutex */ + return; + } + virMutexUnlock(&conn->lock); } - - virMutexUnlock(&conn->lock); } @@ -845,16 +863,19 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c ret = (virStorageVolPtr) virHashLookup(conn->storageVols, key); if (ret == NULL) { if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } ret->pool = strdup(pool); if (ret->pool == NULL) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } ret->name = strdup(name); if (ret->name == NULL) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } @@ -864,6 +885,7 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c ret->conn = conn; if (virHashAddEntry(conn->storageVols, key, ret) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("failed to add storage vol to connection hash table")); goto error; @@ -875,7 +897,6 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const c return(ret); error: - virMutexUnlock(&conn->lock); if (ret != NULL) { VIR_FREE(ret->name); VIR_FREE(ret->pool); @@ -903,24 +924,28 @@ virReleaseStorageVol(virStorageVolPtr vol) { DEBUG("release vol %p %s", vol, vol->name); /* TODO search by UUID first as they are better differenciators */ - if (virHashRemoveEntry(conn->storageVols, vol->key, NULL) < 0) + if (virHashRemoveEntry(conn->storageVols, vol->key, NULL) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("vol missing from connection hash table")); + conn = NULL; + } vol->magic = -1; VIR_FREE(vol->name); VIR_FREE(vol->pool); VIR_FREE(vol); - DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; + if (conn) { + DEBUG("unref connection %p %d", conn, conn->refs); + conn->refs--; + if (conn->refs == 0) { + virReleaseConnect(conn); + /* Already unlocked mutex */ + return; + } + virMutexUnlock(&conn->lock); } - - virMutexUnlock(&conn->lock); } @@ -981,7 +1006,8 @@ virGetNodeDevice(virConnectPtr conn, const char *name) ret = (virNodeDevicePtr) virHashLookup(conn->nodeDevices, name); if (ret == NULL) { - if (VIR_ALLOC(ret) < 0) { + if (VIR_ALLOC(ret) < 0) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } @@ -989,11 +1015,13 @@ virGetNodeDevice(virConnectPtr conn, const char *name) ret->conn = conn; ret->name = strdup(name); if (ret->name == NULL) { + virMutexUnlock(&conn->lock); virReportOOMError(conn); goto error; } if (virHashAddEntry(conn->nodeDevices, name, ret) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("failed to add node dev to conn hash table")); goto error; @@ -1005,7 +1033,6 @@ virGetNodeDevice(virConnectPtr conn, const char *name) return(ret); error: - virMutexUnlock(&conn->lock); if (ret != NULL) { VIR_FREE(ret->name); VIR_FREE(ret); @@ -1031,24 +1058,28 @@ virReleaseNodeDevice(virNodeDevicePtr dev) { virConnectPtr conn = dev->conn; DEBUG("release dev %p %s", dev, dev->name); - if (virHashRemoveEntry(conn->nodeDevices, dev->name, NULL) < 0) + if (virHashRemoveEntry(conn->nodeDevices, dev->name, NULL) < 0) { + virMutexUnlock(&conn->lock); virLibConnError(conn, VIR_ERR_INTERNAL_ERROR, _("dev missing from connection hash table")); + conn = NULL; + } dev->magic = -1; VIR_FREE(dev->name); VIR_FREE(dev->parent); VIR_FREE(dev); - DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; + if (conn) { + DEBUG("unref connection %p %d", conn, conn->refs); + conn->refs--; + if (conn->refs == 0) { + virReleaseConnect(conn); + /* Already unlocked mutex */ + return; + } + virMutexUnlock(&conn->lock); } - - virMutexUnlock(&conn->lock); } -- 1.6.0.6

3 2

[libvirt] [PATCH] Rename remaining occurrences of 'interface' to avoid MINGW32 build problem
by Laine Stump 23 Jul '09

23 Jul '09

--- qemud/remote.c | 2 +- src/interface_conf.c | 34 +++++++++++++++++----------------- src/interface_conf.h | 4 ++-- src/veth.c | 6 +++--- src/veth.h | 2 +- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/qemud/remote.c b/qemud/remote.c index 651e9fb..d32d513 100644 --- a/qemud/remote.c +++ b/qemud/remote.c @@ -61,7 +61,7 @@ static virDomainPtr get_nonnull_domain (virConnectPtr conn, remote_nonnull_domain domain); static virNetworkPtr get_nonnull_network (virConnectPtr conn, remote_nonnull_network network); -static virInterfacePtr get_nonnull_interface (virConnectPtr conn, remote_nonnull_interface interface); +static virInterfacePtr get_nonnull_interface (virConnectPtr conn, remote_nonnull_interface iface); static virStoragePoolPtr get_nonnull_storage_pool (virConnectPtr conn, remote_nonnull_storage_pool pool); static virStorageVolPtr get_nonnull_storage_vol (virConnectPtr conn, remote_nonnull_storage_vol vol); static void make_nonnull_domain (remote_nonnull_domain *dom_dst, virDomainPtr dom_src); diff --git a/src/interface_conf.c b/src/interface_conf.c index 1cc9f36..c2e8d07 100644 --- a/src/interface_conf.c +++ b/src/interface_conf.c @@ -1207,51 +1207,51 @@ virInterfaceObjPtr virInterfaceAssignDef(virConnectPtr conn, virInterfaceObjListPtr interfaces, const virInterfaceDefPtr def) { - virInterfaceObjPtr interface; + virInterfaceObjPtr iface; - if ((interface = virInterfaceFindByName(interfaces, def->name))) { - if (interface->def) - virInterfaceDefFree(interface->def); - interface->def = def; + if ((iface = virInterfaceFindByName(interfaces, def->name))) { + if (iface->def) + virInterfaceDefFree(iface->def); + iface->def = def; - return interface; + return iface; } - if (VIR_ALLOC(interface) < 0) { + if (VIR_ALLOC(iface) < 0) { virReportOOMError(conn); return NULL; } - if (virMutexInit(&interface->lock) < 0) { + if (virMutexInit(&iface->lock) < 0) { virInterfaceReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize mutex")); - VIR_FREE(interface); + VIR_FREE(iface); return NULL; } - virInterfaceObjLock(interface); - interface->def = def; + virInterfaceObjLock(iface); + iface->def = def; if (VIR_REALLOC_N(interfaces->objs, interfaces->count + 1) < 0) { virReportOOMError(conn); - VIR_FREE(interface); + VIR_FREE(iface); return NULL; } - interfaces->objs[interfaces->count] = interface; + interfaces->objs[interfaces->count] = iface; interfaces->count++; - return interface; + return iface; } void virInterfaceRemove(virInterfaceObjListPtr interfaces, - const virInterfaceObjPtr interface) + const virInterfaceObjPtr iface) { unsigned int i; - virInterfaceObjUnlock(interface); + virInterfaceObjUnlock(iface); for (i = 0 ; i < interfaces->count ; i++) { virInterfaceObjLock(interfaces->objs[i]); - if (interfaces->objs[i] == interface) { + if (interfaces->objs[i] == iface) { virInterfaceObjUnlock(interfaces->objs[i]); virInterfaceObjFree(interfaces->objs[i]); diff --git a/src/interface_conf.h b/src/interface_conf.h index aea1208..bb9dce4 100644 --- a/src/interface_conf.h +++ b/src/interface_conf.h @@ -186,14 +186,14 @@ virInterfaceObjPtr virInterfaceFindByName(const virInterfaceObjListPtr void virInterfaceDefFree(virInterfaceDefPtr def); -void virInterfaceObjFree(virInterfaceObjPtr interface); +void virInterfaceObjFree(virInterfaceObjPtr iface); void virInterfaceObjListFree(virInterfaceObjListPtr vms); virInterfaceObjPtr virInterfaceAssignDef(virConnectPtr conn, virInterfaceObjListPtr interfaces, const virInterfaceDefPtr def); void virInterfaceRemove(virInterfaceObjListPtr interfaces, - const virInterfaceObjPtr interface); + const virInterfaceObjPtr iface); virInterfaceDefPtr virInterfaceDefParseString(virConnectPtr conn, const char *xmlStr); diff --git a/src/veth.c b/src/veth.c index 04e68b2..71d7de6 100644 --- a/src/veth.c +++ b/src/veth.c @@ -191,16 +191,16 @@ error_out: * * Returns 0 on success or -1 in case of error */ -int moveInterfaceToNetNs(const char* interface, int pidInNs) +int moveInterfaceToNetNs(const char* iface, int pidInNs) { int rc = -1; char *pid = NULL; const char *argv[] = { - "ip", "link", "set", interface, "netns", NULL, NULL + "ip", "link", "set", iface, "netns", NULL, NULL }; int cmdResult; - if (NULL == interface) { + if (NULL == iface) { goto error_out; } diff --git a/src/veth.h b/src/veth.h index 43181cd..429eb3d 100644 --- a/src/veth.h +++ b/src/veth.h @@ -19,6 +19,6 @@ int vethCreate(char* veth1, int veth1MaxLen, char* veth2, int veth2MaxLen); int vethDelete(const char* veth); int vethInterfaceUpOrDown(const char* veth, int upOrDown); -int moveInterfaceToNetNs(const char *interface, int pidInNs); +int moveInterfaceToNetNs(const char *iface, int pidInNs); #endif /* VETH_H */ -- 1.6.0.6

3 2

[libvirt] managing libvirt users in a one-server context
by Juan Miscaro 23 Jul '09

23 Jul '09

I have a single KVM server on which I want to allow co-workers the ability to create vm's. Here are my criteria in order of importance: 1. I do not want to have to grant all users root privileges. 2. I would like the vm's to use the host's bridge (br0) by default. 3. I do not want one user to be able to start, stop, or (re)define the vm's created by another user by default. 4. I would like a user to be able grant access to another user. Mostly for viewing. Can I do all this with hardcoded usernames and passwords? If not, what is my best option? -- /jm

2 1

[libvirt] Bugfix for failing to reconnect to vbox:///session
by Pritesh Kothari 23 Jul '09

23 Jul '09

Hi All, While reconnecting to vbox:///session, since the VBoxCGlueInit() is not intialized, the g_pfnGetFunctions is NULL and thus reconnection fails with SEGV. Fixed this in the patch below. Regards, Pritesh

3 4

[libvirt] [PATCH 0/1] RFC Fix problems with TLS connections & threading
by Daniel P. Berrange 23 Jul '09

23 Jul '09

Even though we are not using gcrypt directly in any libvirt code, it turns out that we are responsible for initializing its thread support, since its used by gnutls. This really sucks in terms of library API encapsulation. It is also a little annoying if libvirt is used within an app that already uses gnutls/gcrypt, because there is no checking in gcrypt to see if threading ops are already set, so our call would override any pre-existing initialization, and an apps own call would override us. The only other option I see is to punt on the whole problem and declare apps are responsible for initializing gcrypt, but then this sucks too because it means apps have to presume we're built with gnutls, and not nss - not that we support nss currently. This bug only really impacts the libvirt client if using multiple virConnectPtr objects from separate threads with a TLS based URI. The server isn't impacted because all TLS I/O is done from the single I/O thread, and not any of the RPC workers. Daniel P. Berrange (1): Initialize gcrypt threading src/remote_internal.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 43 insertions(+), 0 deletions(-)

1 1

[libvirt] [PATCH] Support physical memory in virDomainMemoryPeek()
by Nguyen Anh Quynh 22 Jul '09

22 Jul '09

Hi, This patch provides support for physical memory in virDomainMemoryPeek(). Please consider applying. Signed-off-by: Nguyen Anh Quynh <aquynh(a)gmail.com> # diffstat pmemsave3.diff docs/libvirt-api.xml | 2 +- include/libvirt/libvirt.h.in | 1 + src/libvirt.c | 14 +++++--------- src/qemu_driver.c | 17 ++++++++--------- 4 files changed, 15 insertions(+), 19 deletions(-)

3 8

[libvirt] [PATCH 00/14] Add support for qemu NIC hotplug
by Mark McLoughlin 22 Jul '09

22 Jul '09

Hey, Here's a series of patches to (mostly) support NIC hotplug with qemu 0.10.x or later. qemu-0.10.0 introduced the host_net_add monitor command without which we cannot do NIC hotplug, so we do not try and support hotplug with older versions of KVM. With qemu-0.11.0 we will be able to pass a tap file descriptor over the monitor, but until then we cannot support 'bridge' and 'network' type interfaces. A forthcoming patch series will rectify that. Cheers, Mark.

3 60

[libvirt] fix "make distcheck" failures
by Jim Meyering 22 Jul '09

22 Jul '09

Running "make check" from a tarball would show two test failures, since some test-related files were not being distributed. [that is one of the things that "make distcheck" tests for] With these two fixes, "make distcheck" passes once again: >From 553cecaf27e86755e40bab346d4014fb0291e5cd Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 22 Jul 2009 05:07:07 -0400 Subject: [PATCH 1/2] avoid a make distcheck failure: distribute tests/interfaceschemadata/ * tests/Makefile.am (EXTRA_DIST): Add interfaceschemadata. --- tests/Makefile.am | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index efd7c6e..74e98d1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -53,6 +53,7 @@ EXTRA_DIST = \ networkschematest \ domainschematest \ domainschemadata \ + interfaceschemadata \ storagepoolschematest \ storagepoolschemadata \ storagevolschematest \ -- 1.6.2.5 >From 76f208cc5235023c594e84783af7a33383c8f649 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering(a)redhat.com> Date: Wed, 22 Jul 2009 05:10:14 -0400 Subject: [PATCH 2/2] avoid a make distcheck failure: distribute docs/schemas/interface.rng * docs/schemas/Makefile.am (schema_DATA): Add interface.rng. --- docs/schemas/Makefile.am | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/docs/schemas/Makefile.am b/docs/schemas/Makefile.am index c4ef0b1..ef41a63 100644 --- a/docs/schemas/Makefile.am +++ b/docs/schemas/Makefile.am @@ -3,6 +3,7 @@ schemadir = $(pkgdatadir)/schemas schema_DATA = \ domain.rng \ + interface.rng \ network.rng \ storagepool.rng \ storagevol.rng \ -- 1.6.2.5

2 1

[libvirt] [PATCH] libvirt-glib: import gtk instead of pygtk
by Guido Günther 22 Jul '09

22 Jul '09

Hi, the python example imports pygtk but uses gtk.main() so it fails to work. Attached patch fixes this. Cheers, -- Guido

1 0