Re: [libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
Hi,
> 2. missing virConnectPtr during autostart of the vm:
>
> in src/qemu_driver.c:145, qemudAutostartConfigs() you can find this call:
> int ret = qemudStartVMDaemon(NULL, driver, vm, NULL);
>
> This means virConnectPtr conn is NULL within qemudStartVMDaemon().
> Following the calls:
> qemudStartVMDaemon()
> -> qemudBuildCommandLine()
> -> qemudNetworkIfaceConnect()
> -> virNetworkLookupByName()
>
> virNetworkLookupByName bails out if called with invalid virConnectPtr. This
> means starting this vm fails.
I came up with the attached patch to fix this for me, but
a) I'm not sure if it is a clean use of the api to call virConnectOpen() from
within a state-initializer function
b) This is just for qemu/kvm, I haven't looked at any other drivers
It would be nice if an experienced libvirt-developer could take a look at
this. Thank you very much.
Kind regards,
Gerd
diff -r -u libvirt-0.5.0.orig/src/qemu_driver.c libvirt-0.5.0/src/qemu_driver.c
--- libvirt-0.5.0.orig/src/qemu_driver.c2008-11-21 13:47:32.0
+0100
+++ libvirt-0.5.0/src/qemu_driver.c 2008-12-01 01:49:16.0 +0100
@@ -138,11 +138,14 @@
qemudAutostartConfigs(struct qemud_driver *driver) {
unsigned int i;
+/* we need a valid virConnectPtr for qemudStartVMDaemon to be able to
connect to other drivers */
+virConnectPtr conn=virConnectOpen(getuid() ? "qemu:///session" :
"qemu:///system");
+
for (i = 0 ; i < driver->domains.count ; i++) {
virDomainObjPtr vm = driver->domains.objs[i];
if (vm->autostart &&
!virDomainIsActive(vm)) {
-int ret = qemudStartVMDaemon(NULL, driver, vm, NULL);
+int ret = qemudStartVMDaemon(conn, driver, vm, NULL);
if (ret < 0) {
virErrorPtr err = virGetLastError();
qemudLog(QEMUD_ERR, _("Failed to autostart VM '%s': %s\n"),
@@ -154,6 +157,8 @@
}
}
}
+
+virConnectClose(conn);
}
/**
--
Address (better: trap) for people I really don't want to get mail from:
[EMAIL PROTECTED]
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 0/28: Thread safety for libvirtd daemon and drivers
This is a diffstat summary for the combined series of 28 patches b/Makefile.maint |1 b/qemud/Makefile.am | 24 b/qemud/THREADING.txt| 63 b/qemud/event.c | 168 + b/qemud/event.h | 14 b/qemud/libvirtd.aug |6 b/qemud/libvirtd.conf| 19 b/qemud/qemud.c | 273 ++- b/qemud/qemud.h | 34 b/qemud/remote.c | 3030 ++- b/qemud/remote_dispatch_args.h | 101 + b/qemud/remote_dispatch_prototypes.h | 936 +- b/qemud/remote_dispatch_ret.h| 88 + b/qemud/remote_dispatch_table.h | 594 ++ b/qemud/remote_generate_stubs.pl | 197 -- b/qemud/test_libvirtd.aug| 38 b/src/datatypes.c|1 b/src/datatypes.h|1 b/src/domain_conf.c | 56 b/src/domain_conf.h |4 b/src/domain_event.c | 191 +- b/src/domain_event.h | 51 b/src/libvirt.c | 14 b/src/libvirt_sym.version.in | 25 b/src/lxc_conf.h |2 b/src/lxc_driver.c | 453 +++-- b/src/network_conf.c | 54 b/src/network_conf.h |5 b/src/network_driver.c | 439 +++-- b/src/node_device.c | 118 + b/src/node_device.h |4 b/src/node_device_conf.c | 36 b/src/node_device_conf.h |8 b/src/node_device_devkit.c | 41 b/src/node_device_hal.c | 166 + b/src/openvz_conf.h |2 b/src/openvz_driver.c| 493 +++-- b/src/qemu_conf.h|5 b/src/qemu_driver.c | 2321 +++--- b/src/remote_internal.c | 93 - b/src/storage_conf.c | 44 b/src/storage_conf.h |7 b/src/storage_driver.c | 860 ++--- b/src/test.c | 2016 +-- b/src/uml_conf.h |2 b/src/uml_driver.c | 617 --- b/src/xen_inotify.c | 135 - b/src/xen_unified.c | 30 b/src/xen_unified.h |4 b/src/xs_internal.c | 56 b/tests/virsh-all|1 qemud/remote_dispatch_localvars.h| 185 -- qemud/remote_dispatch_proc_switch.h | 898 -- 54 files changed, 9513 insertions(+), 5594 deletions(-) Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 28/28: Remove unused param in QEMU monitor APIs
The QEMU driver internal API for interacting with the monitor console takes
a 'struct qemud_driver *' parameter. This is a problem because any access
to this struct must be protected by a lock, but we do not want to hold the
global driver lock for all monitor operations since it would destroy any
hope of concurrency. Fortunately tracing down many levels of call reveals
this parameter is totally unused, so this patch removes it from a tonne of
internal APIs
qemu_driver.c | 109 ++
1 file changed, 49 insertions(+), 60 deletions(-)
Daniel
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -136,8 +136,7 @@ static void qemudShutdownVMDaemon(virCon
static int qemudDomainGetMaxVcpus(virDomainPtr dom);
-static int qemudMonitorCommand (const struct qemud_driver *driver,
-const virDomainObjPtr vm,
+static int qemudMonitorCommand (const virDomainObjPtr vm,
const char *cmd,
char **reply);
@@ -386,14 +385,12 @@ qemudShutdown(void) {
/* Return -1 for error, 1 to continue reading and 0 for success */
typedef int qemudHandlerMonitorOutput(virConnectPtr conn,
- struct qemud_driver *driver,
virDomainObjPtr vm,
const char *output,
int fd);
static int
qemudReadMonitorOutput(virConnectPtr conn,
- struct qemud_driver *driver,
virDomainObjPtr vm,
int fd,
char *buf,
@@ -452,7 +449,7 @@ qemudReadMonitorOutput(virConnectPtr con
} else {
got += ret;
buf[got] = '\0';
-if ((ret = func(conn, driver, vm, buf, fd)) != 1)
+if ((ret = func(conn, vm, buf, fd)) != 1)
return ret;
}
}
@@ -466,7 +463,6 @@ qemudReadMonitorOutput(virConnectPtr con
static int
qemudCheckMonitorPrompt(virConnectPtr conn ATTRIBUTE_UNUSED,
-struct qemud_driver *driver ATTRIBUTE_UNUSED,
virDomainObjPtr vm,
const char *output,
int fd)
@@ -480,7 +476,6 @@ qemudCheckMonitorPrompt(virConnectPtr co
}
static int qemudOpenMonitor(virConnectPtr conn,
-struct qemud_driver *driver,
virDomainObjPtr vm,
const char *monitor) {
int monfd;
@@ -504,7 +499,7 @@ static int qemudOpenMonitor(virConnectPt
}
ret = qemudReadMonitorOutput(conn,
- driver, vm, monfd,
+ vm, monfd,
buf, sizeof(buf),
qemudCheckMonitorPrompt,
"monitor");
@@ -564,7 +559,6 @@ static int qemudExtractMonitorPath(virCo
static int
qemudFindCharDevicePTYs(virConnectPtr conn,
-struct qemud_driver *driver,
virDomainObjPtr vm,
const char *output,
int fd ATTRIBUTE_UNUSED)
@@ -603,7 +597,7 @@ qemudFindCharDevicePTYs(virConnectPtr co
}
/* Got them all, so now open the monitor console */
-ret = qemudOpenMonitor(conn, driver, vm, monitor);
+ret = qemudOpenMonitor(conn, vm, monitor);
cleanup:
VIR_FREE(monitor);
@@ -611,11 +605,10 @@ cleanup:
}
static int qemudWaitForMonitor(virConnectPtr conn,
- struct qemud_driver *driver,
virDomainObjPtr vm) {
char buf[1024]; /* Plenty of space to get startup greeting */
int ret = qemudReadMonitorOutput(conn,
- driver, vm, vm->stderr_fd,
+ vm, vm->stderr_fd,
buf, sizeof(buf),
qemudFindCharDevicePTYs,
"console");
@@ -632,7 +625,6 @@ static int qemudWaitForMonitor(virConnec
static int
qemudDetectVcpuPIDs(virConnectPtr conn,
-struct qemud_driver *driver,
virDomainObjPtr vm) {
char *qemucpus = NULL;
char *line;
@@ -656,7 +648,7 @@ qemudDetectVcpuPIDs(virConnectPtr conn,
return 0;
}
-if (qemudMonitorCommand(driver, vm, "info cpus", &qemucpus) < 0) {
+if (qemudMonitorCommand(vm, "info cpus", &qemucpus) < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot run monitor command to fetch CPU
thread info"));
VIR_FREE(vm->vcpupids);
@@ -732,7 +724,6 @@ error:
static int
qemudInitCpus(virConnectPtr conn,
- struct qemud_driver *driver,
Re: [libvirt] PATCH: 27/28: Configurable threading in libvirtd
This patch extends the libvirtd configuration file to allow various thread
related parameters to be tuned. It introduces a limit on the number of
client connections allowed over TCP socket to prevent DOS attacks on the
machine resources. It also adds a min/max worker thread pool size for
handling RPC calls.
libvirtd.aug |6 ++
libvirtd.conf | 19 +++
qemud.c | 17 -
test_libvirtd.aug | 38 ++
4 files changed, 79 insertions(+), 1 deletion(-)
Daniel
diff --git a/qemud/libvirtd.aug b/qemud/libvirtd.aug
--- a/qemud/libvirtd.aug
+++ b/qemud/libvirtd.aug
@@ -13,11 +13,13 @@ module Libvirtd =
let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\""
let bool_val = store /0|1/
+ let int_val = store /[0-9]+/
let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ ""
let str_array_val = counter "el" . array_start . ( str_array_element . (
array_sep . str_array_element ) * ) ? . array_end
let str_entry (kw:string) = [ key kw . value_sep . str_val ]
let bool_entry (kw:string) = [ key kw . value_sep . bool_val ]
+ let int_entry (kw:string) = [ key kw . value_sep . int_val ]
let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ]
@@ -48,6 +50,9 @@ module Libvirtd =
| str_array_entry "tls_allowed_dn_list"
| str_array_entry "sasl_allowed_username_list"
+ let processing_entry = int_entry "min_workers"
+| int_entry "max_workers"
+| int_entry "max_clients"
(* Each enty in the config is one of the following three ... *)
let entry = network_entry
@@ -55,6 +60,7 @@ module Libvirtd =
| authentication_entry
| certificate_entry
| authorization_entry
+ | processing_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^
\t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
diff --git a/qemud/libvirtd.conf b/qemud/libvirtd.conf
--- a/qemud/libvirtd.conf
+++ b/qemud/libvirtd.conf
@@ -225,3 +225,22 @@
#sasl_allowed_username_list = ["[EMAIL PROTECTED]", "[EMAIL PROTECTED]" ]
+
+#
+#
+# Processing controls
+#
+
+# The maximum number of concurrent client connections to allow
+# over all sockets combined.
+# max_clients = 20
+
+
+# The minimum limit sets the number of workers to start up
+# initially. If the number of active clients exceeds this,
+# then more threads are spawned, upto max_workers limit.
+# Typically you'd want max_workers to equal maximum number
+# of clients allowed
+#min_workers = 5
+#max_workers = 20
+
diff --git a/qemud/qemud.c b/qemud/qemud.c
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -129,6 +129,10 @@ static char *crl_file = (char *) "";
static gnutls_certificate_credentials_t x509_cred;
static gnutls_dh_params_t dh_params;
+
+static int min_workers = 5;
+static int max_workers = 20;
+static int max_clients = 20;
#define DH_BITS 1024
@@ -1167,6 +1171,12 @@ static int qemudDispatchServer(struct qe
return -1;
}
+if (server->nclients >= max_clients) {
+qemudLog(QEMUD_ERR, "%s", _("Too many active clients, dropping
connection"));
+close(fd);
+return -1;
+}
+
if (VIR_REALLOC_N(server->clients, server->nclients+1) < 0) {
qemudLog(QEMUD_ERR, "%s", _("Out of memory allocating clients"));
close(fd);
@@ -1825,7 +1835,7 @@ static int qemudRunLoop(struct qemud_ser
pthread_mutex_lock(&server->lock);
-server->nworkers = 10;
+server->nworkers = min_workers;
if (VIR_ALLOC_N(server->workers, server->nworkers) < 0) {
qemudLog(QEMUD_ERR, "%s", _("Failed to allocate workers"));
return -1;
@@ -2221,6 +2231,11 @@ remoteReadConfigFile (struct qemud_serve
if (remoteReadSaslAllowedUsernameList (conf, server, filename) < 0)
goto free_and_fail;
+
+
+GET_CONF_INT (conf, filename, min_workers);
+GET_CONF_INT (conf, filename, max_workers);
+GET_CONF_INT (conf, filename, max_clients);
virConfFree (conf);
return 0;
diff --git a/qemud/test_libvirtd.aug b/qemud/test_libvirtd.aug
--- a/qemud/test_libvirtd.aug
+++ b/qemud/test_libvirtd.aug
@@ -227,6 +227,25 @@ sasl_allowed_username_list = [
\"[EMAIL PROTECTED]",
\"[EMAIL PROTECTED]"
]
+
+
+#
+#
+# Processing controls
+#
+
+# The maximum number of concurrent client connections to allow
+# over all sockets combined.
+max_clients = 20
+
+
+# The minimum limit sets the number of workers to start up
+# initially. If the number of active clients exceeds this,
+# then more threads are spawned, upto max_workers limit.
+# Typically you'd want max_workers to equal maximum number
+# of clients allowed
+min_workers =
Re: [libvirt] PATCH: 25/28: Thread safety for libvirtd event loop
This patch makes the event loop in the libvirtd daemon thread safe, and
re-entrant. This allows one thread to add/remove/update timers/file handle
watches while other thread is doing the poll. This sometimes requires that
we wakeup the main thread to make it see changes to the poll FD list. We
use the traditional self-pipe trick for this task.
event.c | 168 ++--
event.h | 14 +
qemud.c | 10 +++
qemud.h |3 +
4 files changed, 167 insertions(+), 28 deletions(-)
Daniel
diff --git a/qemud/event.c b/qemud/event.c
--- a/qemud/event.c
+++ b/qemud/event.c
@@ -28,12 +28,16 @@
#include
#include
#include
+#include
#include "qemud.h"
#include "event.h"
#include "memory.h"
+#include "util.h"
#define EVENT_DEBUG(fmt, ...) qemudDebug("EVENT: " fmt, __VA_ARGS__)
+
+static int virEventInterruptLocked(void);
/* State for a single file handle being monitored */
struct virEventHandle {
@@ -63,6 +67,9 @@ struct virEventTimeout {
/* State for the main event loop */
struct virEventLoop {
+pthread_mutex_t lock;
+pthread_t leader;
+int wakeupfd[2];
int handlesCount;
int handlesAlloc;
struct virEventHandle *handles;
@@ -80,6 +87,16 @@ static int nextWatch = 0;
/* Unique ID for the next timer to be registered */
static int nextTimer = 0;
+static void virEventLock(void)
+{
+pthread_mutex_lock(&eventLoop.lock);
+}
+
+static void virEventUnlock(void)
+{
+pthread_mutex_unlock(&eventLoop.lock);
+}
+
/*
* Register a callback for monitoring file handle events.
* NB, it *must* be safe to call this from within a callback
@@ -89,17 +106,23 @@ int virEventAddHandleImpl(int fd, int ev
virEventHandleCallback cb,
void *opaque,
virFreeCallback ff) {
+int watch;
EVENT_DEBUG("Add handle %d %d %p %p", fd, events, cb, opaque);
+virEventLock();
if (eventLoop.handlesCount == eventLoop.handlesAlloc) {
EVENT_DEBUG("Used %d handle slots, adding %d more",
eventLoop.handlesAlloc, EVENT_ALLOC_EXTENT);
if (VIR_REALLOC_N(eventLoop.handles,
- (eventLoop.handlesAlloc + EVENT_ALLOC_EXTENT)) < 0)
+ (eventLoop.handlesAlloc + EVENT_ALLOC_EXTENT)) < 0) {
+virEventUnlock();
return -1;
+}
eventLoop.handlesAlloc += EVENT_ALLOC_EXTENT;
}
-eventLoop.handles[eventLoop.handlesCount].watch = nextWatch++;
+watch = nextWatch++;
+
+eventLoop.handles[eventLoop.handlesCount].watch = watch;
eventLoop.handles[eventLoop.handlesCount].fd = fd;
eventLoop.handles[eventLoop.handlesCount].events =
virEventHandleTypeToPollEvent(events);
@@ -110,11 +133,15 @@ int virEventAddHandleImpl(int fd, int ev
eventLoop.handlesCount++;
-return nextWatch-1;
+virEventInterruptLocked();
+virEventUnlock();
+
+return watch;
}
void virEventUpdateHandleImpl(int watch, int events) {
int i;
+virEventLock();
for (i = 0 ; i < eventLoop.handlesCount ; i++) {
if (eventLoop.handles[i].watch == watch) {
eventLoop.handles[i].events =
@@ -122,6 +149,8 @@ void virEventUpdateHandleImpl(int watch,
break;
}
}
+virEventInterruptLocked();
+virEventUnlock();
}
/*
@@ -133,6 +162,7 @@ int virEventRemoveHandleImpl(int watch)
int virEventRemoveHandleImpl(int watch) {
int i;
EVENT_DEBUG("Remove handle %d", watch);
+virEventLock();
for (i = 0 ; i < eventLoop.handlesCount ; i++) {
if (eventLoop.handles[i].deleted)
continue;
@@ -140,9 +170,12 @@ int virEventRemoveHandleImpl(int watch)
if (eventLoop.handles[i].watch == watch) {
EVENT_DEBUG("mark delete %d %d", i, eventLoop.handles[i].fd);
eventLoop.handles[i].deleted = 1;
+virEventUnlock();
return 0;
}
}
+virEventInterruptLocked();
+virEventUnlock();
return -1;
}
@@ -157,17 +190,21 @@ int virEventAddTimeoutImpl(int frequency
void *opaque,
virFreeCallback ff) {
struct timeval now;
+int ret;
EVENT_DEBUG("Adding timer %d with %d ms freq", nextTimer, frequency);
if (gettimeofday(&now, NULL) < 0) {
return -1;
}
+virEventLock();
if (eventLoop.timeoutsCount == eventLoop.timeoutsAlloc) {
EVENT_DEBUG("Used %d timeout slots, adding %d more",
eventLoop.timeoutsAlloc, EVENT_ALLOC_EXTENT);
if (VIR_REALLOC_N(eventLoop.timeouts,
- (eventLoop.timeoutsAlloc + EVENT_ALLOC_EXTENT)) < 0)
+ (eventLoop.timeoutsAlloc + EVENT_ALLOC_EXTENT)) < 0)
{
+virEventUnlock();
return -1;
+}
eventLoop.timeoutsAlloc += E
Re: [libvirt] PATCH: 23/28: Replace client linked list with array
This replaces the linked list of 'struct qemud_client' instances with an
array instead, allowing easy fine grained per-client locking
qemud.c | 43 ---
qemud.h |4 +---
2 files changed, 25 insertions(+), 22 deletions(-)
Daniel
diff --git a/qemud/qemud.c b/qemud/qemud.c
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -1151,6 +1151,12 @@ static int qemudDispatchServer(struct qe
return -1;
}
+if (VIR_REALLOC_N(server->clients, server->nclients+1) < 0) {
+qemudLog(QEMUD_ERR, "%s", _("Out of memory allocating clients"));
+close(fd);
+return -1;
+}
+
/* Disable Nagle. Unix sockets will ignore this. */
setsockopt (fd, IPPROTO_TCP, TCP_NODELAY, (void *)&no_slow_start,
sizeof no_slow_start);
@@ -1231,9 +1237,7 @@ static int qemudDispatchServer(struct qe
}
}
-client->next = server->clients;
-server->clients = client;
-server->nclients++;
+server->clients[server->nclients++] = client;
return 0;
@@ -1248,19 +1252,19 @@ static int qemudDispatchServer(struct qe
static void qemudDispatchClientFailure(struct qemud_server *server, struct
qemud_client *client) {
-struct qemud_client *tmp = server->clients;
-struct qemud_client *prev = NULL;
-while (tmp) {
-if (tmp == client) {
-if (prev == NULL)
-server->clients = client->next;
-else
-prev->next = client->next;
-server->nclients--;
+int i, n = -1;
+for (i = 0 ; i < server->nclients ; i++) {
+if (server->clients[i] == client) {
+n = i;
break;
}
-prev = tmp;
-tmp = tmp->next;
+}
+if (n != -1) {
+if (n < (server->nclients-1))
+memmove(server->clients + n,
+server->clients + n + 1,
+server->nclients - (n + 1));
+server->nclients--;
}
virEventRemoveHandleImpl(client->watch);
@@ -1629,13 +1633,14 @@ static void
static void
qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) {
struct qemud_server *server = (struct qemud_server *)opaque;
-struct qemud_client *client = server->clients;
+struct qemud_client *client = NULL;
+int i;
-while (client) {
-if (client->watch == watch)
+for (i = 0 ; i < server->nclients ; i++) {
+if (server->clients[i]->watch == watch) {
+client = server->clients[i];
break;
-
-client = client->next;
+}
}
if (!client)
diff --git a/qemud/qemud.h b/qemud/qemud.h
--- a/qemud/qemud.h
+++ b/qemud/qemud.h
@@ -133,8 +133,6 @@ struct qemud_client {
/* back-pointer to our server */
struct qemud_server *server;
-
-struct qemud_client *next;
};
#define QEMUD_CLIENT_MAGIC 0x7788aaee
@@ -155,7 +153,7 @@ struct qemud_server {
int nsockets;
struct qemud_socket *sockets;
int nclients;
-struct qemud_client *clients;
+struct qemud_client **clients;
int sigread;
char logDir[PATH_MAX];
unsigned int shutdown : 1;
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 21/28: Fill in locking stubs
This patch fills in the previous stub methods for locking/unlocking
internal objects.
With the following methods return a locked object
virDomainFindByID
virDomainFindByName
virDomainFindByUUID
virDomainAssignDef
All the other methods accepting a virDomainObjPtr instance, require that
the object first be locked.
For virDomainDefPtr objects, if they are standalone, no locking is
required (hence why the Xen driver isn't touched in any of these patches).
If they are associated with a virDomainObjPtr though, this parent object
must be locked.
The same applies for virNetworkObjPtr, virStoragePoolObjPtr and the
virNodeDeviceObjPtr objects & their methods.
domain_conf.c | 40
domain_conf.h |2 ++
network_conf.c | 46 ++
network_conf.h |2 ++
node_device_conf.c | 28 +++-
node_device_conf.h |2 ++
storage_conf.c | 35 ---
storage_conf.h |2 ++
8 files changed, 141 insertions(+), 16 deletions(-)
Daniel
diff --git a/src/domain_conf.c b/src/domain_conf.c
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -151,10 +151,13 @@ virDomainObjPtr virDomainFindByID(const
{
unsigned int i;
-for (i = 0 ; i < doms->count ; i++)
+for (i = 0 ; i < doms->count ; i++) {
+virDomainObjLock(doms->objs[i]);
if (virDomainIsActive(doms->objs[i]) &&
doms->objs[i]->def->id == id)
return doms->objs[i];
+virDomainObjUnlock(doms->objs[i]);
+}
return NULL;
}
@@ -165,9 +168,12 @@ virDomainObjPtr virDomainFindByUUID(cons
{
unsigned int i;
-for (i = 0 ; i < doms->count ; i++)
+for (i = 0 ; i < doms->count ; i++) {
+virDomainObjLock(doms->objs[i]);
if (!memcmp(doms->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
return doms->objs[i];
+virDomainObjUnlock(doms->objs[i]);
+}
return NULL;
}
@@ -177,9 +183,12 @@ virDomainObjPtr virDomainFindByName(cons
{
unsigned int i;
-for (i = 0 ; i < doms->count ; i++)
+for (i = 0 ; i < doms->count ; i++) {
+virDomainObjLock(doms->objs[i]);
if (STREQ(doms->objs[i]->def->name, name))
return doms->objs[i];
+virDomainObjUnlock(doms->objs[i]);
+}
return NULL;
}
@@ -455,6 +464,8 @@ virDomainObjPtr virDomainAssignDef(virCo
return NULL;
}
+pthread_mutex_init(&domain->lock, NULL);
+virDomainObjLock(domain);
domain->state = VIR_DOMAIN_SHUTOFF;
domain->def = def;
@@ -475,8 +486,12 @@ void virDomainRemoveInactive(virDomainOb
{
unsigned int i;
+virDomainObjUnlock(dom);
+
for (i = 0 ; i < doms->count ; i++) {
+virDomainObjLock(doms->objs[i]);
if (doms->objs[i] == dom) {
+virDomainObjUnlock(doms->objs[i]);
virDomainObjFree(doms->objs[i]);
if (i < (doms->count - 1))
@@ -490,6 +505,7 @@ void virDomainRemoveInactive(virDomainOb
break;
}
+virDomainObjUnlock(doms->objs[i]);
}
}
@@ -3337,8 +3353,10 @@ int virDomainLoadAllConfigs(virConnectPt
entry->d_name,
notify,
opaque);
-if (dom)
+if (dom) {
+virDomainObjUnlock(dom);
dom->persistent = 1;
+}
}
closedir(dir);
@@ -3459,6 +3477,19 @@ const char *virDomainDefDefaultEmulator(
}
+#ifdef HAVE_PTHREAD_H
+
+void virDomainObjLock(virDomainObjPtr obj)
+{
+pthread_mutex_lock(&obj->lock);
+}
+
+void virDomainObjUnlock(virDomainObjPtr obj)
+{
+pthread_mutex_unlock(&obj->lock);
+}
+
+#else
void virDomainObjLock(virDomainObjPtr obj ATTRIBUTE_UNUSED)
{
}
@@ -3466,5 +3497,6 @@ void virDomainObjUnlock(virDomainObjPtr
void virDomainObjUnlock(virDomainObjPtr obj ATTRIBUTE_UNUSED)
{
}
+#endif
#endif /* ! PROXY */
diff --git a/src/domain_conf.h b/src/domain_conf.h
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -454,6 +454,8 @@ typedef struct _virDomainObj virDomainOb
typedef struct _virDomainObj virDomainObj;
typedef virDomainObj *virDomainObjPtr;
struct _virDomainObj {
+PTHREAD_MUTEX_T(lock);
+
int stdin_fd;
int stdout_fd;
int stdout_watch;
diff --git a/src/network_conf.c b/src/network_conf.c
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -58,9 +58,12 @@ virNetworkObjPtr virNetworkFindByUUID(co
{
unsigned int i;
-for (i = 0 ; i < nets->count ; i++)
+for (i = 0 ; i < nets->count ; i++) {
+virNetworkObjLock(nets->objs[i]);
if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
return nets->objs[i];
+virNetworkObjUnlock(nets->objs[i]);
+}
return NULL;
}
@@ -70,9 +73,12 @@ virNetworkObjPtr virNetworkFindByName(co
{
unsigned int i;
-for (i = 0 ; i < ne
Re: [libvirt] PATCH: 20/28: Thread safety for node device driver
This patch makes the node device driver thread safe. Almost.
There is on outstanding problem in that the 'reload' method is implemented
by just calling shutdown/startup. This causes the mutex we're locking on
to be free'd and re-allocated. I'll fix this up later.
libvirt_sym.version.in |1
node_device.c | 53 ++-
node_device.h |4 +
node_device_conf.h |3
node_device_devkit.c | 41
node_device_hal.c | 166 +
6 files changed, 199 insertions(+), 69 deletions(-)
Daniel
diff --git a/src/libvirt_sym.version.in b/src/libvirt_sym.version.in
--- a/src/libvirt_sym.version.in
+++ b/src/libvirt_sym.version.in
@@ -509,6 +509,7 @@ [EMAIL PROTECTED]@ {
virNodeDeviceDefFormat;
virNodeDeviceObjLock;
virNodeDeviceObjUnlock;
+ virNodeDeviceAssignDef;
/* qparams.h */
diff --git a/src/node_device.c b/src/node_device.c
--- a/src/node_device.c
+++ b/src/node_device.c
@@ -29,7 +29,7 @@
#include "virterror_internal.h"
#include "datatypes.h"
#include "memory.h"
-
+#include "logging.h"
#include "node_device_conf.h"
#include "node_device.h"
@@ -44,6 +44,17 @@ static int dev_has_cap(const virNodeDevi
return 0;
}
+
+void nodeDeviceLock(virDeviceMonitorStatePtr driver)
+{
+DEBUG("LOCK node %p", driver);
+pthread_mutex_lock(&driver->lock);
+}
+void nodeDeviceUnlock(virDeviceMonitorStatePtr driver)
+{
+DEBUG("UNLOCK node %p", driver);
+pthread_mutex_unlock(&driver->lock);
+}
static int nodeNumOfDevices(virConnectPtr conn,
const char *cap,
@@ -71,15 +82,24 @@ nodeListDevices(virConnectPtr conn,
int ndevs = 0;
unsigned int i;
-for (i = 0; i < driver->devs.count && ndevs < maxnames; i++)
+nodeDeviceLock(driver);
+for (i = 0; i < driver->devs.count && ndevs < maxnames; i++) {
+virNodeDeviceObjLock(driver->devs.objs[i]);
if (cap == NULL ||
-dev_has_cap(driver->devs.objs[i], cap))
-if ((names[ndevs++] = strdup(driver->devs.objs[i]->def->name)) ==
NULL)
+dev_has_cap(driver->devs.objs[i], cap)) {
+if ((names[ndevs++] = strdup(driver->devs.objs[i]->def->name)) ==
NULL) {
+virNodeDeviceObjUnlock(driver->devs.objs[i]);
goto failure;
+}
+}
+virNodeDeviceObjUnlock(driver->devs.objs[i]);
+}
+nodeDeviceUnlock(driver);
return ndevs;
failure:
+nodeDeviceUnlock(driver);
--ndevs;
while (--ndevs >= 0)
VIR_FREE(names[ndevs]);
@@ -94,7 +114,10 @@ static virNodeDevicePtr nodeDeviceLookup
virNodeDeviceObjPtr obj;
virNodeDevicePtr ret = NULL;
+nodeDeviceLock(driver);
obj = virNodeDeviceFindByName(&driver->devs, name);
+nodeDeviceUnlock(driver);
+
if (!obj) {
virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching name"));
@@ -104,6 +127,8 @@ static virNodeDevicePtr nodeDeviceLookup
ret = virGetNodeDevice(conn, name);
cleanup:
+if (obj)
+virNodeDeviceObjUnlock(obj);
return ret;
}
@@ -114,7 +139,10 @@ static char *nodeDeviceDumpXML(virNodeDe
virNodeDeviceObjPtr obj;
char *ret = NULL;
+nodeDeviceLock(driver);
obj = virNodeDeviceFindByName(&driver->devs, dev->name);
+nodeDeviceUnlock(driver);
+
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching name"));
@@ -124,6 +152,8 @@ static char *nodeDeviceDumpXML(virNodeDe
ret = virNodeDeviceDefFormat(dev->conn, obj->def);
cleanup:
+if (obj)
+virNodeDeviceObjUnlock(obj);
return ret;
}
@@ -134,7 +164,10 @@ static char *nodeDeviceGetParent(virNode
virNodeDeviceObjPtr obj;
char *ret = NULL;
+nodeDeviceLock(driver);
obj = virNodeDeviceFindByName(&driver->devs, dev->name);
+nodeDeviceUnlock(driver);
+
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching name"));
@@ -144,6 +177,8 @@ static char *nodeDeviceGetParent(virNode
ret = strdup(obj->def->parent);
cleanup:
+if (obj)
+virNodeDeviceObjUnlock(obj);
return ret;
}
@@ -156,7 +191,10 @@ static int nodeDeviceNumOfCaps(virNodeDe
int ncaps = 0;
int ret = -1;
+nodeDeviceLock(driver);
obj = virNodeDeviceFindByName(&driver->devs, dev->name);
+nodeDeviceUnlock(driver);
+
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching name"));
@@ -168,6 +206,8 @@ static int nodeDeviceNumOfCaps(virNodeDe
ret = ncaps;
cleanup:
+if (obj)
+virNodeDeviceObjUnlock(obj);
return r
Re: [libvirt] PATCH: 19/28: Reduce return paths for node device driver
This patch reduces the number of return paths in the node device driver
methods
In doing this I noticed an annoying problem in the public API contract
for virNodeDeviceGetParent. It returns a 'const char *', but the remote
driver is actually returning an malloc'd string requiring the caller to
free it, while the node_device driver is returning a const string the
caller must not free. Ideally we would not have the 'const' annotation
but changing that now is ABI change, so instead I work around it, by
making all drivers return a malloc'd string, but then caching this in
the virNodeDevicePtr object. This mallc'd string is then free'd when
the object is released. This allows us to work around the public API
contract problem
datatypes.c |1
datatypes.h |1
libvirt.c | 14
node_device.c | 65 ++
4 files changed, 54 insertions(+), 27 deletions(-)
Daniel
diff --git a/src/datatypes.c b/src/datatypes.c
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -861,6 +861,7 @@ virReleaseNodeDevice(virNodeDevicePtr de
dev->magic = -1;
VIR_FREE(dev->name);
+VIR_FREE(dev->parent);
VIR_FREE(dev);
DEBUG("unref connection %p %d", conn, conn->refs);
diff --git a/src/datatypes.h b/src/datatypes.h
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -199,6 +199,7 @@ struct _virNodeDevice {
int refs; /* reference count */
virConnectPtr conn; /* pointer back to the connection */
char *name; /* device name (unique on node) */
+char *parent; /* parent device name */
};
diff --git a/src/libvirt.c b/src/libvirt.c
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5689,11 +5689,15 @@ const char *virNodeDeviceGetParent(virNo
return NULL;
}
-if (dev->conn->deviceMonitor && dev->conn->deviceMonitor->deviceGetParent)
-return dev->conn->deviceMonitor->deviceGetParent (dev);
-
-virLibConnError (dev->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
-return NULL;
+if (!dev->parent) {
+if (dev->conn->deviceMonitor &&
dev->conn->deviceMonitor->deviceGetParent) {
+dev->parent = dev->conn->deviceMonitor->deviceGetParent (dev);
+} else {
+virLibConnError (dev->conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+return NULL;
+}
+}
+return dev->parent;
}
/**
diff --git a/src/node_device.c b/src/node_device.c
--- a/src/node_device.c
+++ b/src/node_device.c
@@ -91,66 +91,84 @@ static virNodeDevicePtr nodeDeviceLookup
const char *name)
{
virDeviceMonitorStatePtr driver = conn->devMonPrivateData;
-virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs, name);
+virNodeDeviceObjPtr obj;
+virNodeDevicePtr ret = NULL;
+obj = virNodeDeviceFindByName(&driver->devs, name);
if (!obj) {
virNodeDeviceReportError(conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching name"));
-return NULL;
+goto cleanup;
}
-return virGetNodeDevice(conn, name);
+ret = virGetNodeDevice(conn, name);
+cleanup:
+return ret;
}
static char *nodeDeviceDumpXML(virNodeDevicePtr dev,
unsigned int flags ATTRIBUTE_UNUSED)
{
virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData;
-virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs,
dev->name);
+virNodeDeviceObjPtr obj;
+char *ret = NULL;
+obj = virNodeDeviceFindByName(&driver->devs, dev->name);
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching name"));
-return NULL;
+goto cleanup;
}
-return virNodeDeviceDefFormat(dev->conn, obj->def);
+ret = virNodeDeviceDefFormat(dev->conn, obj->def);
+
+cleanup:
+return ret;
}
static char *nodeDeviceGetParent(virNodeDevicePtr dev)
{
virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData;
-virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs,
dev->name);
+virNodeDeviceObjPtr obj;
+char *ret = NULL;
+obj = virNodeDeviceFindByName(&driver->devs, dev->name);
if (!obj) {
virNodeDeviceReportError(dev->conn, VIR_ERR_INVALID_NODE_DEVICE,
"%s", _("no node device with matching name"));
-return NULL;
+goto cleanup;
}
-return obj->def->parent;
+ret = strdup(obj->def->parent);
+
+cleanup:
+return ret;
}
static int nodeDeviceNumOfCaps(virNodeDevicePtr dev)
{
virDeviceMonitorStatePtr driver = dev->conn->devMonPrivateData;
-virNodeDeviceObjPtr obj = virNodeDeviceFindByName(&driver->devs,
dev->name);
+virNodeDeviceObjPtr obj;
virNodeDevCapsDefPtr caps;
int nca
Re: [libvirt] PATCH: 18/28: Thread safety for OpenVZ driver
This patch makes the OpenVZ driver thread safe
openvz_conf.h |2 +
openvz_driver.c | 103 ++--
2 files changed, 102 insertions(+), 3 deletions(-)
Daniel
diff --git a/src/openvz_conf.h b/src/openvz_conf.h
--- a/src/openvz_conf.h
+++ b/src/openvz_conf.h
@@ -53,6 +53,8 @@ enum { OPENVZ_WARN, OPENVZ_ERR };
#define VZCTL_BRIDGE_MIN_VERSION ((3 * 1000 * 1000) + (0 * 1000) + 22 + 1)
struct openvz_driver {
+PTHREAD_MUTEX_T(lock);
+
virCapsPtr caps;
virDomainObjList domains;
int version;
diff --git a/src/openvz_driver.c b/src/openvz_driver.c
--- a/src/openvz_driver.c
+++ b/src/openvz_driver.c
@@ -66,6 +66,16 @@ static int openvzGetMaxVCPUs(virConnectP
static int openvzGetMaxVCPUs(virConnectPtr conn, const char *type);
static int openvzDomainGetMaxVcpus(virDomainPtr dom);
static int openvzDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus);
+
+static void openvzDriverLock(struct openvz_driver *driver)
+{
+pthread_mutex_lock(&driver->lock);
+}
+
+static void openvzDriverUnlock(struct openvz_driver *driver)
+{
+pthread_mutex_unlock(&driver->lock);
+}
struct openvz_driver ovz_driver;
@@ -159,7 +169,10 @@ static virDomainPtr openvzDomainLookupBy
virDomainObjPtr vm;
virDomainPtr dom = NULL;
+openvzDriverLock(driver);
vm = virDomainFindByID(&driver->domains, id);
+openvzDriverUnlock(driver);
+
if (!vm) {
openvzError(conn, VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -170,12 +183,16 @@ static virDomainPtr openvzDomainLookupBy
dom->id = vm->def->id;
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return dom;
}
static int openvzGetVersion(virConnectPtr conn, unsigned long *version) {
struct openvz_driver *driver = conn->privateData;
+openvzDriverLock(driver);
*version = driver->version;
+openvzDriverUnlock(driver);
return 0;
}
@@ -185,7 +202,10 @@ static char *openvzGetOSType(virDomainPt
virDomainObjPtr vm;
char *ret = NULL;
+openvzDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+openvzDriverUnlock(driver);
+
if (!vm) {
openvzError(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -195,6 +215,8 @@ static char *openvzGetOSType(virDomainPt
openvzError(dom->conn, VIR_ERR_NO_MEMORY, NULL);
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return ret;
}
@@ -205,7 +227,10 @@ static virDomainPtr openvzDomainLookupBy
virDomainObjPtr vm;
virDomainPtr dom = NULL;
+openvzDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, uuid);
+openvzDriverUnlock(driver);
+
if (!vm) {
openvzError(conn, VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -216,6 +241,8 @@ static virDomainPtr openvzDomainLookupBy
dom->id = vm->def->id;
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return dom;
}
@@ -225,7 +252,10 @@ static virDomainPtr openvzDomainLookupBy
virDomainObjPtr vm;
virDomainPtr dom = NULL;
+openvzDriverLock(driver);
vm = virDomainFindByName(&driver->domains, name);
+openvzDriverUnlock(driver);
+
if (!vm) {
openvzError(conn, VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -236,6 +266,8 @@ static virDomainPtr openvzDomainLookupBy
dom->id = vm->def->id;
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return dom;
}
@@ -245,7 +277,10 @@ static int openvzDomainGetInfo(virDomain
virDomainObjPtr vm;
int ret = -1;
+openvzDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+openvzDriverUnlock(driver);
+
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
"%s", _("no domain with matching uuid"));
@@ -270,6 +305,8 @@ static int openvzDomainGetInfo(virDomain
ret = 0;
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return ret;
}
@@ -279,7 +316,10 @@ static char *openvzDomainDumpXML(virDoma
virDomainObjPtr vm;
char *ret = NULL;
+openvzDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+openvzDriverUnlock(driver);
+
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
"%s", _("no domain with matching uuid"));
@@ -289,6 +329,8 @@ static char *openvzDomainDumpXML(virDoma
ret = virDomainDefFormat(dom->conn, vm->def, flags);
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return ret;
}
@@ -312,7 +354,10 @@ static int openvzDomainShutdown(virDomai
const char *prog[] = {VZCTL, "--quiet", "stop", PROGRAM_SENTINAL, NULL};
int ret = -1;
+openvzDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+openvzDriverUnlock(driver);
+
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
"%s", _("no domain with matching uui
Re: [libvirt] PATCH: 17/28: Reduce return paths in OpenVZ driver
This patch reduces the number of exit paths in the OpenVZ driver methods
openvz_driver.c | 392
1 file changed, 224 insertions(+), 168 deletions(-)
Daniel
diff --git a/src/openvz_driver.c b/src/openvz_driver.c
--- a/src/openvz_driver.c
+++ b/src/openvz_driver.c
@@ -155,96 +155,101 @@ static int openvzDomainDefineCmd(virConn
static virDomainPtr openvzDomainLookupByID(virConnectPtr conn,
int id) {
-struct openvz_driver *driver = (struct openvz_driver *)conn->privateData;
+struct openvz_driver *driver = conn->privateData;
virDomainObjPtr vm;
-virDomainPtr dom;
+virDomainPtr dom = NULL;
vm = virDomainFindByID(&driver->domains, id);
-
if (!vm) {
openvzError(conn, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
-if (!dom)
-return NULL;
+if (dom)
+dom->id = vm->def->id;
-dom->id = vm->def->id;
+cleanup:
return dom;
}
static int openvzGetVersion(virConnectPtr conn, unsigned long *version) {
-struct openvz_driver *driver = (struct openvz_driver *)conn->privateData;
+struct openvz_driver *driver = conn->privateData;
*version = driver->version;
return 0;
}
static char *openvzGetOSType(virDomainPtr dom)
{
-struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
-virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid);
-char *ret;
+struct openvz_driver *driver = dom->conn->privateData;
+virDomainObjPtr vm;
+char *ret = NULL;
+vm = virDomainFindByUUID(&driver->domains, dom->uuid);
if (!vm) {
openvzError(dom->conn, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
if (!(ret = strdup(vm->def->os.type)))
openvzError(dom->conn, VIR_ERR_NO_MEMORY, NULL);
+cleanup:
return ret;
}
static virDomainPtr openvzDomainLookupByUUID(virConnectPtr conn,
const unsigned char *uuid) {
-struct openvz_driver *driver = (struct openvz_driver *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, uuid);
-virDomainPtr dom;
+struct openvz_driver *driver = conn->privateData;
+virDomainObjPtr vm;
+virDomainPtr dom = NULL;
+vm = virDomainFindByUUID(&driver->domains, uuid);
if (!vm) {
openvzError(conn, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
-if (!dom)
-return NULL;
+if (dom)
+dom->id = vm->def->id;
-dom->id = vm->def->id;
+cleanup:
return dom;
}
static virDomainPtr openvzDomainLookupByName(virConnectPtr conn,
- const char *name) {
-struct openvz_driver *driver = (struct openvz_driver *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByName(&driver->domains, name);
-virDomainPtr dom;
+ const char *name) {
+struct openvz_driver *driver = conn->privateData;
+virDomainObjPtr vm;
+virDomainPtr dom = NULL;
+vm = virDomainFindByName(&driver->domains, name);
if (!vm) {
openvzError(conn, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
-if (!dom)
-return NULL;
+if (dom)
+dom->id = vm->def->id;
-dom->id = vm->def->id;
+cleanup:
return dom;
}
static int openvzDomainGetInfo(virDomainPtr dom,
virDomainInfoPtr info) {
-struct openvz_driver *driver = (struct openvz_driver
*)dom->conn->privateData;
-virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, dom->uuid);
+struct openvz_driver *driver = dom->conn->privateData;
+virDomainObjPtr vm;
+int ret = -1;
+vm = virDomainFindByUUID(&driver->domains, dom->uuid);
if (!vm) {
openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN,
"%s", _("no domain with matching uuid"));
-return -1;
+goto cleanup;
}
info->state = vm->state;
@@ -255,80 +260,110 @@ static int openvzDomainGetInfo(virDomain
if (openvzGetProcessInfo(&(info->cpuTime), dom->id) < 0) {
openvzError(dom->conn, VIR_ERR_OPERATION_FAILED,
_("cannot read cputime for domain %d"), dom->id);
-return -1;
+goto cleanup;
}
}
info->maxMem = vm->def->maxmem;
info->memory = vm->def->memory;
info->nrVirtCpu = vm->def->vcpus;
-return 0;
+ret = 0;
+
+cleanup:
+return ret;
}
static char *openvzDomainDumpXML(virDomainPtr dom, int flags) {
-struct openvz_driver *driver = (struct openvz_driver
*
Re: [libvirt] PATCH: 16/28: Thread safety for storage driver
This patch makes the storage driver thread safe
storage_conf.h |2
storage_driver.c | 240 +--
2 files changed, 202 insertions(+), 40 deletions(-)
Daniel
diff --git a/src/storage_conf.h b/src/storage_conf.h
--- a/src/storage_conf.h
+++ b/src/storage_conf.h
@@ -248,6 +248,8 @@ typedef virStorageDriverState *virStorag
typedef virStorageDriverState *virStorageDriverStatePtr;
struct _virStorageDriverState {
+PTHREAD_MUTEX_T(lock);
+
virStoragePoolObjList pools;
char *configDir;
diff --git a/src/storage_driver.c b/src/storage_driver.c
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -47,6 +47,14 @@ static virStorageDriverStatePtr driverSt
static int storageDriverShutdown(void);
+static void storageDriverLock(virStorageDriverStatePtr driver)
+{
+pthread_mutex_lock(&driver->lock);
+}
+static void storageDriverUnlock(virStorageDriverStatePtr driver)
+{
+pthread_mutex_unlock(&driver->lock);
+}
static void
storageDriverAutostart(virStorageDriverStatePtr driver) {
@@ -100,6 +108,9 @@ storageDriverStartup(void) {
if (VIR_ALLOC(driverState) < 0)
return -1;
+pthread_mutex_init(&driverState->lock, NULL);
+storageDriverLock(driverState);
+
if (!uid) {
if ((base = strdup (SYSCONF_DIR "/libvirt")) == NULL)
goto out_of_memory;
@@ -132,8 +143,7 @@ storageDriverStartup(void) {
"%s/storage/autostart", base) == -1)
goto out_of_memory;
-free(base);
-base = NULL;
+VIR_FREE(base);
/*
if (virStorageLoadDriverConfig(driver, driverConf) < 0) {
@@ -145,19 +155,19 @@ storageDriverStartup(void) {
if (virStoragePoolLoadAllConfigs(NULL,
&driverState->pools,
driverState->configDir,
- driverState->autostartDir) < 0) {
-storageDriverShutdown();
-return -1;
-}
+ driverState->autostartDir) < 0)
+goto error;
storageDriverAutostart(driverState);
+storageDriverUnlock(driverState);
return 0;
- out_of_memory:
+out_of_memory:
storageLog("virStorageStartup: out of memory");
-free(base);
-free(driverState);
-driverState = NULL;
+error:
+VIR_FREE(base);
+storageDriverUnlock(driverState);
+storageDriverShutdown();
return -1;
}
@@ -172,11 +182,13 @@ storageDriverReload(void) {
if (!driverState)
return -1;
+storageDriverLock(driverState);
virStoragePoolLoadAllConfigs(NULL,
&driverState->pools,
driverState->configDir,
driverState->autostartDir);
storageDriverAutostart(driverState);
+storageDriverUnlock(driverState);
return 0;
}
@@ -191,19 +203,22 @@ static int
static int
storageDriverActive(void) {
unsigned int i;
+int active = 0;
if (!driverState)
return 0;
-/* If we've any active networks or guests, then we
- * mark this driver as active
- */
-for (i = 0 ; i < driverState->pools.count ; i++)
+storageDriverLock(driverState);
+
+for (i = 0 ; i < driverState->pools.count ; i++) {
+virStoragePoolObjLock(driverState->pools.objs[i]);
if (virStoragePoolObjIsActive(driverState->pools.objs[i]))
-return 1;
+active = 1;
+virStoragePoolObjUnlock(driverState->pools.objs[i]);
+}
-/* Otherwise we're happy to deal with a shutdown */
-return 0;
+storageDriverUnlock(driverState);
+return active;
}
/**
@@ -218,6 +233,7 @@ storageDriverShutdown(void) {
if (!driverState)
return -1;
+storageDriverLock(driverState);
/* shutdown active pools */
for (i = 0 ; i < driverState->pools.count ; i++) {
virStoragePoolObjPtr pool = driverState->pools.objs[i];
@@ -244,6 +260,7 @@ storageDriverShutdown(void) {
VIR_FREE(driverState->configDir);
VIR_FREE(driverState->autostartDir);
+storageDriverUnlock(driverState);
VIR_FREE(driverState);
return 0;
@@ -258,7 +275,10 @@ storagePoolLookupByUUID(virConnectPtr co
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
+storageDriverLock(driver);
pool = virStoragePoolObjFindByUUID(&driver->pools, uuid);
+storageDriverUnlock(driver);
+
if (!pool) {
virStorageReportError(conn, VIR_ERR_NO_STORAGE_POOL,
"%s", _("no pool with matching uuid"));
@@ -268,6 +288,8 @@ storagePoolLookupByUUID(virConnectPtr co
ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
cleanup:
+if (pool)
+virStoragePoolObjUnlock(pool);
return ret;
}
@@ -278,7 +300,10 @@ storagePoolLookupByName(virConnectPtr co
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
+storageDriverLo
Re: [libvirt] PATCH: 15/28: Reduce return points for storage driver
This patch reduces the number of return points in the storage driver
methods
storage_driver.c | 570 ++-
1 file changed, 313 insertions(+), 257 deletions(-)
Daniel
diff --git a/src/storage_driver.c b/src/storage_driver.c
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -254,36 +254,40 @@ static virStoragePoolPtr
static virStoragePoolPtr
storagePoolLookupByUUID(virConnectPtr conn,
const unsigned char *uuid) {
-virStorageDriverStatePtr driver =
-(virStorageDriverStatePtr)conn->storagePrivateData;
-virStoragePoolObjPtr pool = virStoragePoolObjFindByUUID(&driver->pools,
uuid);
-virStoragePoolPtr ret;
+virStorageDriverStatePtr driver = conn->storagePrivateData;
+virStoragePoolObjPtr pool;
+virStoragePoolPtr ret = NULL;
+pool = virStoragePoolObjFindByUUID(&driver->pools, uuid);
if (!pool) {
virStorageReportError(conn, VIR_ERR_NO_STORAGE_POOL,
"%s", _("no pool with matching uuid"));
-return NULL;
+goto cleanup;
}
ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
+
+cleanup:
return ret;
}
static virStoragePoolPtr
storagePoolLookupByName(virConnectPtr conn,
const char *name) {
-virStorageDriverStatePtr driver =
-(virStorageDriverStatePtr)conn->storagePrivateData;
-virStoragePoolObjPtr pool = virStoragePoolObjFindByName(&driver->pools,
name);
-virStoragePoolPtr ret;
+virStorageDriverStatePtr driver = conn->storagePrivateData;
+virStoragePoolObjPtr pool;
+virStoragePoolPtr ret = NULL;
+pool = virStoragePoolObjFindByName(&driver->pools, name);
if (!pool) {
virStorageReportError(conn, VIR_ERR_NO_STORAGE_POOL,
"%s", _("no pool with matching name"));
-return NULL;
+goto cleanup;
}
ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid);
+
+cleanup:
return ret;
}
@@ -311,8 +315,7 @@ storageClose(virConnectPtr conn) {
static int
storageNumPools(virConnectPtr conn) {
-virStorageDriverStatePtr driver
-= (virStorageDriverStatePtr)conn->storagePrivateData;
+virStorageDriverStatePtr driver = conn->storagePrivateData;
unsigned int i, nactive = 0;
for (i = 0 ; i < driver->pools.count ; i++)
@@ -326,8 +329,7 @@ storageListPools(virConnectPtr conn,
storageListPools(virConnectPtr conn,
char **const names,
int nnames) {
-virStorageDriverStatePtr driver =
-(virStorageDriverStatePtr)conn->storagePrivateData;
+virStorageDriverStatePtr driver = conn->storagePrivateData;
int got = 0, i;
for (i = 0 ; i < driver->pools.count && got < nnames ; i++) {
@@ -353,8 +355,7 @@ storageListPools(virConnectPtr conn,
static int
storageNumDefinedPools(virConnectPtr conn) {
-virStorageDriverStatePtr driver
-= (virStorageDriverStatePtr)conn->storagePrivateData;
+virStorageDriverStatePtr driver = conn->storagePrivateData;
unsigned int i, nactive = 0;
for (i = 0 ; i < driver->pools.count ; i++)
@@ -368,8 +369,7 @@ storageListDefinedPools(virConnectPtr co
storageListDefinedPools(virConnectPtr conn,
char **const names,
int nnames) {
-virStorageDriverStatePtr driver
-= (virStorageDriverStatePtr)conn->storagePrivateData;
+virStorageDriverStatePtr driver = conn->storagePrivateData;
int got = 0, i;
for (i = 0 ; i < driver->pools.count && got < nnames ; i++) {
@@ -401,19 +401,21 @@ storageFindPoolSources(virConnectPtr con
{
int backend_type;
virStorageBackendPtr backend;
+char *ret = NULL;
backend_type = virStoragePoolTypeFromString(type);
if (backend_type < 0)
-return NULL;
+goto cleanup;
backend = virStorageBackendForType(backend_type);
if (backend == NULL)
-return NULL;
+goto cleanup;
if (backend->findPoolSources)
-return backend->findPoolSources(conn, srcSpec, flags);
+ret = backend->findPoolSources(conn, srcSpec, flags);
-return NULL;
+cleanup:
+return ret;
}
@@ -421,46 +423,47 @@ storagePoolCreate(virConnectPtr conn,
storagePoolCreate(virConnectPtr conn,
const char *xml,
unsigned int flags ATTRIBUTE_UNUSED) {
-virStorageDriverStatePtr driver =
-(virStorageDriverStatePtr )conn->storagePrivateData;
+virStorageDriverStatePtr driver = conn->storagePrivateData;
virStoragePoolDefPtr def;
virStoragePoolObjPtr pool;
-virStoragePoolPtr ret;
+virStoragePoolPtr ret = NULL;
virStorageBackendPtr backend;
if (!(def = virStoragePoolDefParse(conn, xml, NULL)))
-return NULL;
+goto cleanup;
-if (virStoragePoolObjFindByUUID(&driver->pools, def->uuid) ||
-virStoragePoolObjFindByN
Re: [libvirt] PATCH: 14/28: Thread safety for network driver
This patch makes the network driver thread safe, following the pattern of
the earlier patches
network_driver.c | 136 ++-
1 file changed, 125 insertions(+), 11 deletions(-)
Daniel
diff --git a/src/network_driver.c b/src/network_driver.c
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -59,6 +59,8 @@
/* Main driver state */
struct network_driver {
+PTHREAD_MUTEX_T(lock);
+
virNetworkObjList networks;
iptablesContext *iptables;
@@ -67,6 +69,16 @@ struct network_driver {
char *networkAutostartDir;
char *logDir;
};
+
+
+static void networkDriverLock(struct network_driver *driver)
+{
+pthread_mutex_lock(&driver->lock);
+}
+static void networkDriverUnlock(struct network_driver *driver)
+{
+pthread_mutex_unlock(&driver->lock);
+}
static int networkShutdown(void);
@@ -95,6 +107,7 @@ networkAutostartConfigs(struct network_d
unsigned int i;
for (i = 0 ; i < driver->networks.count ; i++) {
+virNetworkObjLock(driver->networks.objs[i]);
if (driver->networks.objs[i]->autostart &&
!virNetworkIsActive(driver->networks.objs[i]) &&
networkStartNetworkDaemon(NULL, driver, driver->networks.objs[i])
< 0) {
@@ -103,6 +116,7 @@ networkAutostartConfigs(struct network_d
driver->networks.objs[i]->def->name,
err ? err->message : NULL);
}
+virNetworkObjUnlock(driver->networks.objs[i]);
}
}
@@ -119,6 +133,9 @@ networkStartup(void) {
if (VIR_ALLOC(driverState) < 0)
goto error;
+
+pthread_mutex_init(&driverState->lock, NULL);
+networkDriverLock(driverState);
if (!uid) {
if (asprintf(&driverState->logDir,
@@ -165,6 +182,8 @@ networkStartup(void) {
networkAutostartConfigs(driverState);
+networkDriverUnlock(driverState);
+
return 0;
out_of_memory:
@@ -172,6 +191,9 @@ out_of_memory:
"%s", _("networkStartup: out of memory\n"));
error:
+if (driverState)
+networkDriverUnlock(driverState);
+
VIR_FREE(base);
networkShutdown();
return -1;
@@ -188,6 +210,7 @@ networkReload(void) {
if (!driverState)
return 0;
+networkDriverLock(driverState);
virNetworkLoadAllConfigs(NULL,
&driverState->networks,
driverState->networkConfigDir,
@@ -200,7 +223,7 @@ networkReload(void) {
}
networkAutostartConfigs(driverState);
-
+networkDriverUnlock(driverState);
return 0;
}
@@ -220,12 +243,15 @@ networkActive(void) {
if (!driverState)
return 0;
+networkDriverLock(driverState);
for (i = 0 ; i < driverState->networks.count ; i++) {
virNetworkObjPtr net = driverState->networks.objs[i];
+virNetworkObjLock(net);
if (virNetworkIsActive(net))
active = 1;
+virNetworkObjUnlock(net);
}
-
+networkDriverUnlock(driverState);
return active;
}
@@ -241,12 +267,16 @@ networkShutdown(void) {
if (!driverState)
return -1;
+networkDriverLock(driverState);
+
/* shutdown active networks */
for (i = 0 ; i < driverState->networks.count ; i++) {
virNetworkObjPtr net = driverState->networks.objs[i];
+virNetworkObjLock(net);
if (virNetworkIsActive(net))
networkShutdownNetworkDaemon(NULL, driverState,
driverState->networks.objs[i]);
+virNetworkObjUnlock(net);
}
/* free inactive networks */
@@ -260,6 +290,8 @@ networkShutdown(void) {
brShutdown(driverState->brctl);
if (driverState->iptables)
iptablesContextFree(driverState->iptables);
+
+networkDriverUnlock(driverState);
VIR_FREE(driverState);
@@ -801,10 +833,6 @@ static int networkShutdownNetworkDaemon(
network->newDef = NULL;
}
-if (!network->configFile)
-virNetworkRemoveInactive(&driver->networks,
- network);
-
return 0;
}
@@ -815,7 +843,9 @@ static virNetworkPtr networkLookupByUUID
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
+networkDriverLock(driver);
network = virNetworkFindByUUID(&driver->networks, uuid);
+networkDriverUnlock(driver);
if (!network) {
networkReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK,
"%s", _("no network with matching uuid"));
@@ -825,6 +855,8 @@ static virNetworkPtr networkLookupByUUID
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
+if (network)
+virNetworkObjUnlock(network);
return ret;
}
@@ -834,7 +866,9 @@ static virNetworkPtr networkLookupByName
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
+networkDriverLock(driver);
network = virNetworkFindByName(&driver->networks, name);
+networkDriverUnlock
Re: [libvirt] PATCH: 13/28: Reduce return points in network driver
This patch reduces the number of return points in the network driver
methods
network_driver.c | 275 +++
1 file changed, 159 insertions(+), 116 deletions(-)
Daniel
diff --git a/src/network_driver.c b/src/network_driver.c
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -118,7 +118,7 @@ networkStartup(void) {
char *base = NULL;
if (VIR_ALLOC(driverState) < 0)
-return -1;
+goto error;
if (!uid) {
if (asprintf(&driverState->logDir,
@@ -160,19 +160,20 @@ networkStartup(void) {
if (virNetworkLoadAllConfigs(NULL,
&driverState->networks,
driverState->networkConfigDir,
- driverState->networkAutostartDir) < 0) {
-networkShutdown();
-return -1;
-}
+ driverState->networkAutostartDir) < 0)
+goto error;
+
networkAutostartConfigs(driverState);
return 0;
- out_of_memory:
+out_of_memory:
networkLog (NETWORK_ERR,
"%s", _("networkStartup: out of memory\n"));
+
+error:
VIR_FREE(base);
-VIR_FREE(driverState);
+networkShutdown();
return -1;
}
@@ -214,16 +215,18 @@ static int
static int
networkActive(void) {
unsigned int i;
+int active = 0;
if (!driverState)
return 0;
-for (i = 0 ; i < driverState->networks.count ; i++)
-if (virNetworkIsActive(driverState->networks.objs[i]))
-return 1;
+for (i = 0 ; i < driverState->networks.count ; i++) {
+virNetworkObjPtr net = driverState->networks.objs[i];
+if (virNetworkIsActive(net))
+active = 1;
+}
-/* Otherwise we're happy to deal with a shutdown */
-return 0;
+return active;
}
/**
@@ -239,10 +242,12 @@ networkShutdown(void) {
return -1;
/* shutdown active networks */
-for (i = 0 ; i < driverState->networks.count ; i++)
-if (virNetworkIsActive(driverState->networks.objs[i]))
+for (i = 0 ; i < driverState->networks.count ; i++) {
+virNetworkObjPtr net = driverState->networks.objs[i];
+if (virNetworkIsActive(net))
networkShutdownNetworkDaemon(NULL, driverState,
driverState->networks.objs[i]);
+}
/* free inactive networks */
virNetworkObjListFree(&driverState->networks);
@@ -804,35 +809,42 @@ static int networkShutdownNetworkDaemon(
}
-static virNetworkPtr networkLookupByUUID(virConnectPtr conn ATTRIBUTE_UNUSED,
- const unsigned char *uuid) {
-struct network_driver *driver = (struct network_driver
*)conn->networkPrivateData;
-virNetworkObjPtr network = virNetworkFindByUUID(&driver->networks, uuid);
-virNetworkPtr net;
+static virNetworkPtr networkLookupByUUID(virConnectPtr conn,
+ const unsigned char *uuid) {
+struct network_driver *driver = conn->networkPrivateData;
+virNetworkObjPtr network;
+virNetworkPtr ret = NULL;
+network = virNetworkFindByUUID(&driver->networks, uuid);
if (!network) {
networkReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK,
"%s", _("no network with matching uuid"));
-return NULL;
+goto cleanup;
}
-net = virGetNetwork(conn, network->def->name, network->def->uuid);
-return net;
+ret = virGetNetwork(conn, network->def->name, network->def->uuid);
+
+cleanup:
+return ret;
}
-static virNetworkPtr networkLookupByName(virConnectPtr conn ATTRIBUTE_UNUSED,
- const char *name) {
-struct network_driver *driver = (struct network_driver
*)conn->networkPrivateData;
-virNetworkObjPtr network = virNetworkFindByName(&driver->networks, name);
-virNetworkPtr net;
+static virNetworkPtr networkLookupByName(virConnectPtr conn,
+ const char *name) {
+struct network_driver *driver = conn->networkPrivateData;
+virNetworkObjPtr network;
+virNetworkPtr ret = NULL;
+
+network = virNetworkFindByName(&driver->networks, name);
if (!network) {
networkReportError(conn, NULL, NULL, VIR_ERR_NO_NETWORK,
"%s", _("no network with matching name"));
-return NULL;
+goto cleanup;
}
-net = virGetNetwork(conn, network->def->name, network->def->uuid);
-return net;
+ret = virGetNetwork(conn, network->def->name, network->def->uuid);
+
+cleanup:
+return ret;
}
static virDrvOpenStatus networkOpenNetwork(virConnectPtr conn,
@@ -852,7 +864,7 @@ static int networkCloseNetwork(virConnec
static int networkNumNetworks(virConnectPtr conn) {
int nactive = 0, i;
-struct network_driver *driver = (struct network_driver
*)conn->networkPrivateData;
+struct network_driver *d
Re: [libvirt] PATCH: 12/28: Thread safety for UML driver
This patch makes the UML driver thread safe
uml_conf.h |2
uml_driver.c | 209 +++
2 files changed, 172 insertions(+), 39 deletions(-)
Daniel
diff --git a/src/uml_conf.h b/src/uml_conf.h
--- a/src/uml_conf.h
+++ b/src/uml_conf.h
@@ -39,6 +39,8 @@
/* Main driver state */
struct uml_driver {
+PTHREAD_MUTEX_T(lock);
+
unsigned int umlVersion;
int nextvmid;
diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -73,6 +73,15 @@ static int umlShutdown(void);
#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)
#define umlLog(level, msg...) fprintf(stderr, msg)
+
+static void umlDriverLock(struct uml_driver *driver)
+{
+pthread_mutex_lock(&driver->lock);
+}
+static void umlDriverUnlock(struct uml_driver *driver)
+{
+pthread_mutex_unlock(&driver->lock);
+}
static int umlOpenMonitor(virConnectPtr conn,
@@ -288,13 +297,16 @@ umlStartup(void) {
if (VIR_ALLOC(uml_driver) < 0)
return -1;
+pthread_mutex_init(¨_driver->lock, NULL);
+umlDriverLock(uml_driver);
+
/* Don't have a dom0 so start from 1 */
uml_driver->nextvmid = 1;
if (!(pw = getpwuid(uid))) {
umlLog(UML_ERR, _("Failed to find user record for uid '%d': %s\n"),
uid, strerror(errno));
-goto out_nouid;
+goto error;
}
if (!uid) {
@@ -338,49 +350,45 @@ umlStartup(void) {
if ((uml_driver->inotifyFD = inotify_init()) < 0) {
umlLog(UML_ERR, "%s", _("cannot initialize inotify"));
-goto out_nouid;
+goto error;
}
if (virFileMakePath(uml_driver->monitorDir) < 0) {
umlLog(UML_ERR, _("Failed to create monitor directory %s: %s"),
uml_driver->monitorDir, strerror(errno));
-umlShutdown();
-return -1;
+goto error;
}
if ((uml_driver->inotifyWatch =
inotify_add_watch(uml_driver->inotifyFD,
uml_driver->monitorDir,
- IN_CREATE | IN_MODIFY | IN_DELETE)) < 0) {
-umlShutdown();
-return -1;
-}
+ IN_CREATE | IN_MODIFY | IN_DELETE)) < 0)
+goto error;
if (virEventAddHandle(uml_driver->inotifyFD, POLLIN,
- umlInotifyEvent, uml_driver, NULL) < 0) {
-umlShutdown();
-return -1;
-}
+ umlInotifyEvent, uml_driver, NULL) < 0)
+goto error;
if (virDomainLoadAllConfigs(NULL,
uml_driver->caps,
¨_driver->domains,
uml_driver->configDir,
uml_driver->autostartDir,
-NULL, NULL) < 0) {
-umlShutdown();
-return -1;
-}
+NULL, NULL) < 0)
+goto error;
+
umlAutostartConfigs(uml_driver);
+umlDriverUnlock(uml_driver);
return 0;
- out_of_memory:
+out_of_memory:
umlLog (UML_ERR,
"%s", _("umlStartup: out of memory\n"));
- out_nouid:
+error:
VIR_FREE(base);
-VIR_FREE(uml_driver);
+umlDriverUnlock(uml_driver);
+umlShutdown();
return -1;
}
@@ -395,6 +403,7 @@ umlReload(void) {
if (!uml_driver)
return 0;
+umlDriverLock(uml_driver);
virDomainLoadAllConfigs(NULL,
uml_driver->caps,
¨_driver->domains,
@@ -403,6 +412,7 @@ umlReload(void) {
NULL, NULL);
umlAutostartConfigs(uml_driver);
+umlDriverUnlock(uml_driver);
return 0;
}
@@ -418,16 +428,21 @@ static int
static int
umlActive(void) {
unsigned int i;
+int active = 0;
if (!uml_driver)
return 0;
-for (i = 0 ; i < uml_driver->domains.count ; i++)
+umlDriverLock(uml_driver);
+for (i = 0 ; i < uml_driver->domains.count ; i++) {
+virDomainObjLock(uml_driver->domains.objs[i]);
if (virDomainIsActive(uml_driver->domains.objs[i]))
-return 1;
+active = 1;
+virDomainObjUnlock(uml_driver->domains.objs[i]);
+}
+umlDriverUnlock(uml_driver);
-/* Otherwise we're happy to deal with a shutdown */
-return 0;
+return active;
}
/**
@@ -442,6 +457,7 @@ umlShutdown(void) {
if (!uml_driver)
return -1;
+umlDriverLock(uml_driver);
virEventRemoveHandle(uml_driver->inotifyWatch);
close(uml_driver->inotifyFD);
virCapabilitiesFree(uml_driver->caps);
@@ -451,9 +467,6 @@ umlShutdown(void) {
virDomainObjPtr dom = uml_driver->domains.objs[i];
if (virDomainIsActive(dom))
umlShutdownVMDaemon(NULL, uml_driver, dom);
-if (!dom->persistent)
-virDomainRemoveInactive(¨_driver->domains,
-dom);
}
virDomainObjListFree(
Re: [libvirt] PATCH: 11/28: Reduce return points for UML driver
This patch reduces the number of return points in the UML driver methods
Makefile.maint |1
src/uml_driver.c | 410 ---
2 files changed, 243 insertions(+), 168 deletions(-)
Daniel
diff --git a/Makefile.maint b/Makefile.maint
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -355,6 +355,7 @@ msg_gen_function += virDomainReportError
msg_gen_function += virDomainReportError
msg_gen_function += virReportErrorHelper
msg_gen_function += lxcError
+msg_gen_function += umlError
# Uncomment the following and run "make syntax-check" to see diagnostics
# that are not yet marked for translation, but that need to be rewritten
diff --git a/src/uml_driver.c b/src/uml_driver.c
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -883,7 +883,7 @@ static virDrvOpenStatus umlOpen(virConne
}
static int umlClose(virConnectPtr conn) {
-/*struct uml_driver *driver = (struct uml_driver *)conn->privateData;*/
+/*struct uml_driver *driver = conn->privateData;*/
conn->privateData = NULL;
@@ -904,11 +904,9 @@ static char *umlGetCapabilities(virConne
struct uml_driver *driver = (struct uml_driver *)conn->privateData;
char *xml;
-if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL) {
+if ((xml = virCapabilitiesFormatXML(driver->caps)) == NULL)
umlReportError(conn, NULL, NULL, VIR_ERR_NO_MEMORY,
"%s", _("failed to allocate space for capabilities support"));
-return NULL;
-}
return xml;
}
@@ -922,11 +920,12 @@ umlNodeGetCellsFreeMemory(virConnectPtr
int maxCells)
{
int n, lastCell, numCells;
+int ret = -1;
if (numa_available() < 0) {
umlReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
"%s", _("NUMA not supported on this host"));
-return -1;
+goto cleanup;
}
lastCell = startCell + maxCells - 1;
if (lastCell > numa_max_node())
@@ -937,22 +936,27 @@ umlNodeGetCellsFreeMemory(virConnectPtr
if (numa_node_size64(n, &mem) < 0) {
umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Failed to query NUMA free memory"));
-return -1;
+goto cleanup;
}
freeMems[numCells++] = mem;
}
-return numCells;
+ret = numCells;
+
+cleanup:
+return ret;
}
static unsigned long long
umlNodeGetFreeMemory (virConnectPtr conn)
{
unsigned long long freeMem = 0;
+unsigned long long ret = -1;
int n;
+
if (numa_available() < 0) {
umlReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
"%s", _("NUMA not supported on this host"));
-return -1;
+goto cleanup;
}
for (n = 0 ; n <= numa_max_node() ; n++) {
@@ -960,12 +964,14 @@ umlNodeGetFreeMemory (virConnectPtr conn
if (numa_node_size64(n, &mem) < 0) {
umlReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Failed to query NUMA free memory"));
-return -1;
+goto cleanup;
}
freeMem += mem;
}
+ret = freeMem;
-return freeMem;
+cleanup:
+return ret;
}
#endif
@@ -1009,46 +1015,57 @@ static virDomainPtr umlDomainLookupByID(
static virDomainPtr umlDomainLookupByID(virConnectPtr conn,
int id) {
struct uml_driver *driver = (struct uml_driver *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByID(&driver->domains, id);
-virDomainPtr dom;
+virDomainObjPtr vm;
+virDomainPtr dom = NULL;
+vm = virDomainFindByID(&driver->domains, id);
if (!vm) {
umlReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
if (dom) dom->id = vm->def->id;
+
+cleanup:
return dom;
}
+
static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn,
const unsigned char *uuid) {
struct uml_driver *driver = (struct uml_driver *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, uuid);
-virDomainPtr dom;
+virDomainObjPtr vm;
+virDomainPtr dom = NULL;
+vm = virDomainFindByUUID(&driver->domains, uuid);
if (!vm) {
umlReportError(conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
if (dom) dom->id = vm->def->id;
+
+cleanup:
return dom;
}
+
static virDomainPtr umlDomainLookupByName(virConnectPtr conn,
const char *name) {
struct uml_driver *driver = (struct uml_driver *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByName(&driver->domains, name);
-virDomainPtr dom;
+virDo
Re: [libvirt] PATCH: 10/28: Thread safety for LXC driver
This patch makes the LXC driver thread safe following the same pattern
as the Test/QEMU drivers
lxc_conf.h |2
lxc_driver.c | 187 +++
2 files changed, 151 insertions(+), 38 deletions(-)
Daniel
diff --git a/src/lxc_conf.h b/src/lxc_conf.h
--- a/src/lxc_conf.h
+++ b/src/lxc_conf.h
@@ -36,6 +36,8 @@
typedef struct __lxc_driver lxc_driver_t;
struct __lxc_driver {
+PTHREAD_MUTEX_T(lock);
+
virCapsPtr caps;
virDomainObjList domains;
diff --git a/src/lxc_driver.c b/src/lxc_driver.c
--- a/src/lxc_driver.c
+++ b/src/lxc_driver.c
@@ -54,6 +54,16 @@ static lxc_driver_t *lxc_driver = NULL;
static lxc_driver_t *lxc_driver = NULL;
/* Functions */
+
+static void lxcDriverLock(lxc_driver_t *driver)
+{
+pthread_mutex_lock(&driver->lock);
+}
+static void lxcDriverUnlock(lxc_driver_t *driver)
+{
+pthread_mutex_unlock(&driver->lock);
+}
+
static int lxcProbe(void)
{
@@ -107,7 +117,10 @@ static virDomainPtr lxcDomainLookupByID(
virDomainObjPtr vm;
virDomainPtr dom = NULL;
+lxcDriverLock(driver);
vm = virDomainFindByID(&driver->domains, id);
+lxcDriverUnlock(driver);
+
if (!vm) {
lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -118,6 +131,8 @@ static virDomainPtr lxcDomainLookupByID(
dom->id = vm->def->id;
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return dom;
}
@@ -128,7 +143,10 @@ static virDomainPtr lxcDomainLookupByUUI
virDomainObjPtr vm;
virDomainPtr dom = NULL;
+lxcDriverLock(driver);
vm = virDomainFindByUUID(&driver->domains, uuid);
+lxcDriverUnlock(driver);
+
if (!vm) {
lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -139,6 +157,8 @@ static virDomainPtr lxcDomainLookupByUUI
dom->id = vm->def->id;
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return dom;
}
@@ -149,7 +169,9 @@ static virDomainPtr lxcDomainLookupByNam
virDomainObjPtr vm;
virDomainPtr dom = NULL;
+lxcDriverLock(driver);
vm = virDomainFindByName(&driver->domains, name);
+lxcDriverUnlock(driver);
if (!vm) {
lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
goto cleanup;
@@ -160,6 +182,8 @@ static virDomainPtr lxcDomainLookupByNam
dom->id = vm->def->id;
cleanup:
+if (vm)
+virDomainObjUnlock(vm);
return dom;
}
@@ -167,9 +191,14 @@ static int lxcListDomains(virConnectPtr
lxc_driver_t *driver = conn->privateData;
int got = 0, i;
-for (i = 0 ; i < driver->domains.count && got < nids ; i++)
+lxcDriverLock(driver);
+for (i = 0 ; i < driver->domains.count && got < nids ; i++) {
+virDomainObjLock(driver->domains.objs[i]);
if (virDomainIsActive(driver->domains.objs[i]))
ids[got++] = driver->domains.objs[i]->def->id;
+virDomainObjUnlock(driver->domains.objs[i]);
+}
+lxcDriverUnlock(driver);
return got;
}
@@ -178,9 +207,14 @@ static int lxcNumDomains(virConnectPtr c
lxc_driver_t *driver = conn->privateData;
int n = 0, i;
-for (i = 0 ; i < driver->domains.count ; i++)
+lxcDriverLock(driver);
+for (i = 0 ; i < driver->domains.count ; i++) {
+virDomainObjLock(driver->domains.objs[i]);
if (virDomainIsActive(driver->domains.objs[i]))
n++;
+virDomainObjUnlock(driver->domains.objs[i]);
+}
+lxcDriverUnlock(driver);
return n;
}
@@ -190,21 +224,27 @@ static int lxcListDefinedDomains(virConn
lxc_driver_t *driver = conn->privateData;
int got = 0, i;
+lxcDriverLock(driver);
for (i = 0 ; i < driver->domains.count && got < nnames ; i++) {
+virDomainObjLock(driver->domains.objs[i]);
if (!virDomainIsActive(driver->domains.objs[i])) {
if (!(names[got++] = strdup(driver->domains.objs[i]->def->name))) {
lxcError(conn, NULL, VIR_ERR_NO_MEMORY,
"%s", _("failed to allocate space for VM name
string"));
+virDomainObjUnlock(driver->domains.objs[i]);
goto cleanup;
}
}
+virDomainObjUnlock(driver->domains.objs[i]);
}
+lxcDriverUnlock(driver);
return got;
cleanup:
for (i = 0 ; i < got ; i++)
VIR_FREE(names[i]);
+lxcDriverUnlock(driver);
return -1;
}
@@ -213,9 +253,14 @@ static int lxcNumDefinedDomains(virConne
lxc_driver_t *driver = conn->privateData;
int n = 0, i;
-for (i = 0 ; i < driver->domains.count ; i++)
+lxcDriverLock(driver);
+for (i = 0 ; i < driver->domains.count ; i++) {
+virDomainObjLock(driver->domains.objs[i]);
if (!virDomainIsActive(driver->domains.objs[i]))
n++;
+virDomainObjUnlock(driver->domains.objs[i]);
+}
+lxcDriverUnlock(driver);
return n;
}
@@ -226,9 +271,10 @@ static virDomainPtr l
Re: [libvirt] PATCH: 9/28: Reduce exit paths for LXC driver
This patch reduces the number of exit paths in the LXC driver, much like
the earlier patches for other drivers
lxc_driver.c | 278 ---
1 file changed, 154 insertions(+), 124 deletions(-)
Daniel
diff --git a/src/lxc_driver.c b/src/lxc_driver.c
--- a/src/lxc_driver.c
+++ b/src/lxc_driver.c
@@ -103,65 +103,68 @@ static virDomainPtr lxcDomainLookupByID(
static virDomainPtr lxcDomainLookupByID(virConnectPtr conn,
int id)
{
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByID(&driver->domains, id);
-virDomainPtr dom;
+lxc_driver_t *driver = conn->privateData;
+virDomainObjPtr vm;
+virDomainPtr dom = NULL;
+vm = virDomainFindByID(&driver->domains, id);
if (!vm) {
lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
-if (dom) {
+if (dom)
dom->id = vm->def->id;
-}
+cleanup:
return dom;
}
static virDomainPtr lxcDomainLookupByUUID(virConnectPtr conn,
const unsigned char *uuid)
{
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByUUID(&driver->domains, uuid);
-virDomainPtr dom;
+lxc_driver_t *driver = conn->privateData;
+virDomainObjPtr vm;
+virDomainPtr dom = NULL;
+vm = virDomainFindByUUID(&driver->domains, uuid);
if (!vm) {
lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
-if (dom) {
+if (dom)
dom->id = vm->def->id;
-}
+cleanup:
return dom;
}
static virDomainPtr lxcDomainLookupByName(virConnectPtr conn,
const char *name)
{
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
-virDomainObjPtr vm = virDomainFindByName(&driver->domains, name);
-virDomainPtr dom;
+lxc_driver_t *driver = conn->privateData;
+virDomainObjPtr vm;
+virDomainPtr dom = NULL;
+vm = virDomainFindByName(&driver->domains, name);
if (!vm) {
lxcError(conn, NULL, VIR_ERR_NO_DOMAIN, NULL);
-return NULL;
+goto cleanup;
}
dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
-if (dom) {
+if (dom)
dom->id = vm->def->id;
-}
+cleanup:
return dom;
}
static int lxcListDomains(virConnectPtr conn, int *ids, int nids) {
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
+lxc_driver_t *driver = conn->privateData;
int got = 0, i;
for (i = 0 ; i < driver->domains.count && got < nids ; i++)
@@ -170,8 +173,9 @@ static int lxcListDomains(virConnectPtr
return got;
}
+
static int lxcNumDomains(virConnectPtr conn) {
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
+lxc_driver_t *driver = conn->privateData;
int n = 0, i;
for (i = 0 ; i < driver->domains.count ; i++)
@@ -183,7 +187,7 @@ static int lxcNumDomains(virConnectPtr c
static int lxcListDefinedDomains(virConnectPtr conn,
char **const names, int nnames) {
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
+lxc_driver_t *driver = conn->privateData;
int got = 0, i;
for (i = 0 ; i < driver->domains.count && got < nnames ; i++) {
@@ -206,7 +210,7 @@ static int lxcListDefinedDomains(virConn
static int lxcNumDefinedDomains(virConnectPtr conn) {
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
+lxc_driver_t *driver = conn->privateData;
int n = 0, i;
for (i = 0 ; i < driver->domains.count ; i++)
@@ -220,86 +224,91 @@ static int lxcNumDefinedDomains(virConne
static virDomainPtr lxcDomainDefine(virConnectPtr conn, const char *xml)
{
-lxc_driver_t *driver = (lxc_driver_t *)conn->privateData;
-virDomainDefPtr def;
+lxc_driver_t *driver = conn->privateData;
+virDomainDefPtr def = NULL;
virDomainObjPtr vm;
-virDomainPtr dom;
+virDomainPtr dom = NULL;
if (!(def = virDomainDefParseString(conn, driver->caps, xml)))
-return NULL;
+goto cleanup;
if ((def->nets != NULL) && !(driver->have_netns)) {
lxcError(conn, NULL, VIR_ERR_NO_SUPPORT,
"%s", _("System lacks NETNS support"));
-virDomainDefFree(def);
-return NULL;
+goto cleanup;
}
-if (!(vm = virDomainAssignDef(conn, &driver->domains, def))) {
-virDomainDefFree(def);
-return NULL;
-}
+if (!(vm = virDomainAssignDef(conn, &driver->domains, def)))
+goto cleanup;
+def = NULL;
vm->persistent = 1;
if (virDomainSaveConfig(conn,
driver->configDir,
Re: [libvirt] PATCH: 8/28: Thread safety for domain events code
This patch adds thread safety to the domain events processing code of
the QEMU driver. This entailed rather a large refactoring of the domain
events code and its quite complicated to explain.
- A convenient helper is added for creating event queues
virDomainEventQueuePtr virDomainEventQueueNew(void);
- Convenient helpers are added for creating virDomainEventPtr instances
from a variety of sources - each driver has different needs
virDomainEventPtr virDomainEventNew(int id, const char *name, const unsigned
char *uuid, int type, int detail);
virDomainEventPtr virDomainEventNewFromDom(virDomainPtr dom, int type, int
detail);
virDomainEventPtr virDomainEventNewFromObj(virDomainObjPtr obj, int type,
int detail);
virDomainEventPtr virDomainEventNewFromDef(virDomainDefPtr def, int type,
int detail);
- The virDomainEvent struct held a reference to a virDomainPtr object.
This is replaced with a direct triple (id, name, uuid), avoiding the
need to instantiate virDomainPtr objects deep inside the QEMU code.
- The virDomainEventQueuePush() method is changed to take a
virDomainEventPtr object, rather than a virDomainPtr object
These last two changes are important to allow safe re-entrancy of the
event dispatch process. The virDomainEventPtr instance can be allocated
within a critical section lockde on the virDomainObjPtr instance, while
the event is queued outside the critical section, while only the driver
lock is held. Without this we'd have to hold the per-driver lock over
a much larger block of code which hurts concurrancy.
The QEMU driver cannot directly dispatch events, instead we have to
follow the remote driver and maintain a queue of pending events, and
use a timer to flush them. Again this is neccessary to improve
concurrency & provide safe re-entrancy.
Since we have two driver maintaining queues of evnts I add more
helper functions to allow code sharing
- Send a single vent to a list of callbacks:
void virDomainEventDispatch(virDomainEventPtr event,
virDomainEventCallbackListPtr cbs,
virDomainEventDispatchFunc dispatch,
void *opaque);
- Send a set of queued events to a list of callbacks
void virDomainEventQueueDispatch(virDomainEventQueuePtr queue,
virDomainEventCallbackListPtr cbs,
virDomainEventDispatchFunc dispatch,
void *opaque);
The virDomainEventDispatchFunc is what is invoked to finally dispatch
a single event, to a single registered callback. The default impl just
invokes the callback directly. The QEMU driver, however, uses a wrapper
which first releases its driver lock, invokes the callback, and then
re-aquires the lock.
As a further complication it is not safe for virDomainEventUnregister
to actually remove the callback from the list directly. Instead we
add a helper that simply marks it as removed, and then actually
purge it from a safe point in the code, when its guarenteed that the
driver isn't in the middle of dispatching.
As well as making the QEMU driver thread safe, this also takes the
opportunity to refactor the Xen / remote drivers to use more helpers
domain_event.c | 191 +++
domain_event.h | 51 ++-
libvirt_sym.version.in | 16 ++
qemu_conf.h|3
qemu_driver.c | 339 +++--
remote_internal.c | 93 +
xen_inotify.c | 135 ++-
xen_unified.c | 30 +---
xen_unified.h |4
xs_internal.c | 56 +++-
10 files changed, 594 insertions(+), 324 deletions(-)
Daniel
diff --git a/src/domain_event.c b/src/domain_event.c
--- a/src/domain_event.c
+++ b/src/domain_event.c
@@ -125,6 +125,49 @@ virDomainEventCallbackListRemoveConn(vir
return 0;
}
+int virDomainEventCallbackListMarkDelete(virConnectPtr conn,
+ virDomainEventCallbackListPtr cbList,
+ virConnectDomainEventCallback
callback)
+{
+int i;
+for (i = 0 ; i < cbList->count ; i++) {
+if (cbList->callbacks[i]->conn == conn &&
+cbList->callbacks[i]->cb == callback) {
+cbList->callbacks[i]->deleted = 1;
+return 0;
+}
+}
+return -1;
+}
+
+int virDomainEventCallbackListPurgeMarked(virDomainEventCallbackListPtr cbList)
+{
+int old_count = cbList->count;
+int i;
+for (i = 0 ; i < cbList->count ; i++) {
+if (cbList->callbacks[i]->deleted) {
+virFreeCallback freecb = cbList->callbacks[i]->freecb;
+if (freecb)
+(*freecb)(cbList->callbacks[i]->opaque);
+virUnrefConnect(cbList->callbacks[i]->conn);
+VIR_FREE(cbList->callbacks[i]);
+
+if (i < (cbList->count - 1))
+
Re: [libvirt] PATCH: 7/28: Thread safety for QEMU driver
This patch makes the QEMU driver threadsafe with the exception of the
domain events code.
qemu_conf.h |2
qemu_driver.c | 458 ++
2 files changed, 337 insertions(+), 123 deletions(-)
Daniel
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -51,6 +51,8 @@ enum qemud_cmd_flags {
/* Main driver state */
struct qemud_driver {
+PTHREAD_MUTEX_T(lock);
+
unsigned int qemuVersion;
int nextvmid;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -76,6 +76,15 @@ static int qemudShutdown(void);
#define qemudLog(level, msg...) fprintf(stderr, msg)
+static void qemuDriverLock(struct qemud_driver *driver)
+{
+pthread_mutex_lock(&driver->lock);
+}
+static void qemuDriverUnlock(struct qemud_driver *driver)
+{
+pthread_mutex_unlock(&driver->lock);
+}
+
static int qemudSetCloseExec(int fd) {
int flags;
if ((flags = fcntl(fd, F_GETFD)) < 0)
@@ -141,6 +150,7 @@ qemudAutostartConfigs(struct qemud_drive
for (i = 0 ; i < driver->domains.count ; i++) {
virDomainObjPtr vm = driver->domains.objs[i];
+virDomainObjLock(vm);
if (vm->autostart &&
!virDomainIsActive(vm)) {
int ret = qemudStartVMDaemon(NULL, driver, vm, NULL);
@@ -154,6 +164,7 @@ qemudAutostartConfigs(struct qemud_drive
VIR_DOMAIN_EVENT_STARTED_BOOTED);
}
}
+virDomainObjUnlock(vm);
}
}
@@ -172,12 +183,15 @@ qemudStartup(void) {
if (VIR_ALLOC(qemu_driver) < 0)
return -1;
+pthread_mutex_init(&qemu_driver->lock, NULL);
+qemuDriverLock(qemu_driver);
+
/* Don't have a dom0 so start from 1 */
qemu_driver->nextvmid = 1;
/* Init callback list */
if(VIR_ALLOC(qemu_driver->domainEventCallbacks) < 0)
-return -1;
+goto out_of_memory;
if (!uid) {
if (asprintf(&qemu_driver->logDir,
@@ -190,7 +204,7 @@ qemudStartup(void) {
if (!(pw = getpwuid(uid))) {
qemudLog(QEMUD_ERR, _("Failed to find user record for uid '%d':
%s\n"),
uid, strerror(errno));
-goto out_nouid;
+goto error;
}
if (asprintf(&qemu_driver->logDir,
@@ -229,20 +243,22 @@ qemudStartup(void) {
&qemu_driver->domains,
qemu_driver->configDir,
qemu_driver->autostartDir,
-NULL, NULL) < 0) {
-qemudShutdown();
-return -1;
-}
+NULL, NULL) < 0)
+goto error;
qemudAutostartConfigs(qemu_driver);
-return 0;
-
- out_of_memory:
+qemuDriverUnlock(qemu_driver);
+
+return 0;
+
+out_of_memory:
qemudLog (QEMUD_ERR,
"%s", _("qemudStartup: out of memory\n"));
- out_nouid:
+error:
+if (qemu_driver)
+qemuDriverUnlock(qemu_driver);
VIR_FREE(base);
-VIR_FREE(qemu_driver);
+qemudShutdown();
return -1;
}
@@ -267,6 +283,7 @@ qemudReload(void) {
if (!qemu_driver)
return 0;
+qemuDriverLock(qemu_driver);
virDomainLoadAllConfigs(NULL,
qemu_driver->caps,
&qemu_driver->domains,
@@ -275,6 +292,7 @@ qemudReload(void) {
qemudNotifyLoadDomain, qemu_driver);
qemudAutostartConfigs(qemu_driver);
+qemuDriverUnlock(qemu_driver);
return 0;
}
@@ -290,16 +308,18 @@ static int
static int
qemudActive(void) {
unsigned int i;
+int active = 0;
if (!qemu_driver)
return 0;
+qemuDriverLock(qemu_driver);
for (i = 0 ; i < qemu_driver->domains.count ; i++)
if (virDomainIsActive(qemu_driver->domains.objs[i]))
-return 1;
-
-/* Otherwise we're happy to deal with a shutdown */
-return 0;
+active = 1;
+
+qemuDriverUnlock(qemu_driver);
+return active;
}
/**
@@ -314,16 +334,16 @@ qemudShutdown(void) {
if (!qemu_driver)
return -1;
+qemuDriverLock(qemu_driver);
virCapabilitiesFree(qemu_driver->caps);
/* shutdown active VMs */
for (i = 0 ; i < qemu_driver->domains.count ; i++) {
virDomainObjPtr dom = qemu_driver->domains.objs[i];
+virDomainObjLock(dom);
if (virDomainIsActive(dom))
qemudShutdownVMDaemon(NULL, qemu_driver, dom);
-if (!dom->persistent)
-virDomainRemoveInactive(&qemu_driver->domains,
-dom);
+virDomainObjUnlock(dom);
}
virDomainObjListFree(&qemu_driver->domains);
@@ -340,6 +360,7 @@ qemudShutdown(void) {
if (qemu_driver->brctl)
brShutdown(qemu_driver->brctl);
+qemuDriverUnlock(qemu_driver);
VIR_FREE(qemu_driver);
return 0
Re: [libvirt] PATCH: 5/28: Stub out node device APIs in test driver
When adding the node device APIs we never added a stub for the test driver,
so the test driver was connecting to the daemon for node device APIS,
which rather defeats the purpose of the test driver. This patch doesn't
implement the node device APIs, just stubs out the open/close methods
so we don't call into the daemon.
test.c | 27 +++
1 file changed, 27 insertions(+)
Daniel
diff --git a/src/test.c b/src/test.c
--- a/src/test.c
+++ b/src/test.c
@@ -3220,6 +3220,22 @@ cleanup:
}
+static virDrvOpenStatus testDevMonOpen(virConnectPtr conn,
+ virConnectAuthPtr auth ATTRIBUTE_UNUSED,
+ int flags ATTRIBUTE_UNUSED) {
+if (STRNEQ(conn->driver->name, "Test"))
+return VIR_DRV_OPEN_DECLINED;
+
+conn->devMonPrivateData = conn->privateData;
+return VIR_DRV_OPEN_SUCCESS;
+}
+
+static int testDevMonClose(virConnectPtr conn) {
+conn->devMonPrivateData = NULL;
+return 0;
+}
+
+
static virDriver testDriver = {
VIR_DRV_TEST,
"Test",
@@ -3343,6 +3359,14 @@ static virStorageDriver testStorageDrive
.volGetPath = testStorageVolumeGetPath,
};
+static virDeviceMonitor testDevMonitor = {
+.name = "Test",
+.open = testDevMonOpen,
+.close = testDevMonClose,
+};
+
+
+
/**
* testRegister:
*
@@ -3357,5 +3381,8 @@ testRegister(void)
return -1;
if (virRegisterStorageDriver(&testStorageDriver) < 0)
return -1;
+if (virRegisterDeviceMonitor(&testDevMonitor) < 0)
+return -1;
+
return 0;
}
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 4/28: Thread safety for test driver
This patch makes the test driver thread safe, adding a global driver lock,
and the neccessary locking calls on domain/network/storagepool objects.
You'll notice there are many calls to
virDomainObjUnlock
but very few corresponding calls to
virDomainObjLock
This is because the contract of virDomainFindByUUID declares that the
object it returns is already locked.
Methods which create / delete virDomainObj instances have to keep the
global driver lock held for their whole duration, but others can drop
it immediately after getting the virDomainObjPtr instance.
src/test.c | 460 +++-
tests/virsh-all |1
2 files changed, 420 insertions(+), 41 deletions(-)
Daniel
diff --git a/src/test.c b/src/test.c
--- a/src/test.c
+++ b/src/test.c
@@ -58,6 +58,8 @@ typedef struct _testCell *testCellPtr;
#define MAX_CELLS 128
struct _testConn {
+PTHREAD_MUTEX_T(lock);
+
char path[PATH_MAX];
int nextDomID;
virCapsPtr caps;
@@ -90,6 +92,16 @@ static const virNodeInfo defaultNodeInfo
#define testError(conn, code, fmt...) \
virReportErrorHelper(conn, VIR_FROM_TEST, code, __FILE__, \
__FUNCTION__, __LINE__, fmt)
+
+static void testDriverLock(testConnPtr driver)
+{
+pthread_mutex_lock(&driver->lock);
+}
+
+static void testDriverUnlock(testConnPtr driver)
+{
+pthread_mutex_unlock(&driver->lock);
+}
static virCapsPtr
testBuildCapabilities(virConnectPtr conn) {
@@ -200,6 +212,8 @@ static int testOpenDefault(virConnectPtr
return VIR_DRV_OPEN_ERROR;
}
conn->privateData = privconn;
+pthread_mutex_init(&privconn->lock, NULL);
+testDriverLock(privconn);
if (gettimeofday(&tv, NULL) < 0) {
testError(NULL, VIR_ERR_INTERNAL_ERROR, "%s", _("getting time of
day"));
@@ -232,6 +246,7 @@ static int testOpenDefault(virConnectPtr
domobj->def->id = privconn->nextDomID++;
domobj->state = VIR_DOMAIN_RUNNING;
domobj->persistent = 1;
+virDomainObjUnlock(domobj);
if (!(netdef = virNetworkDefParseString(conn, defaultNetworkXML)))
goto error;
@@ -241,6 +256,7 @@ static int testOpenDefault(virConnectPtr
}
netobj->active = 1;
netobj->persistent = 1;
+virNetworkObjUnlock(netobj);
if (!(pooldef = virStoragePoolDefParse(conn, defaultPoolXML, NULL)))
goto error;
@@ -250,10 +266,15 @@ static int testOpenDefault(virConnectPtr
virStoragePoolDefFree(pooldef);
goto error;
}
-if (testStoragePoolObjSetDefaults(poolobj) == -1)
+
+if (testStoragePoolObjSetDefaults(poolobj) == -1) {
+virStoragePoolObjUnlock(poolobj);
goto error;
+}
poolobj->active = 1;
+virStoragePoolObjUnlock(poolobj);
+testDriverUnlock(privconn);
return VIR_DRV_OPEN_SUCCESS;
error:
@@ -261,6 +282,7 @@ error:
virNetworkObjListFree(&privconn->networks);
virStoragePoolObjListFree(&privconn->pools);
virCapabilitiesFree(privconn->caps);
+testDriverUnlock(privconn);
VIR_FREE(privconn);
return VIR_DRV_OPEN_ERROR;
}
@@ -307,6 +329,8 @@ static int testOpenFromFile(virConnectPt
return VIR_DRV_OPEN_ERROR;
}
conn->privateData = privconn;
+pthread_mutex_init(&privconn->lock, NULL);
+testDriverLock(privconn);
if (!(privconn->caps = testBuildCapabilities(conn)))
goto error;
@@ -445,6 +469,7 @@ static int testOpenFromFile(virConnectPt
dom->state = VIR_DOMAIN_RUNNING;
dom->def->id = privconn->nextDomID++;
dom->persistent = 1;
+virDomainObjUnlock(dom);
}
if (domains != NULL)
VIR_FREE(domains);
@@ -478,8 +503,8 @@ static int testOpenFromFile(virConnectPt
virNetworkDefFree(def);
goto error;
}
-
net->persistent = 1;
+virNetworkObjUnlock(net);
}
if (networks != NULL)
VIR_FREE(networks);
@@ -529,15 +554,19 @@ static int testOpenFromFile(virConnectPt
goto error;
}
-if (testStoragePoolObjSetDefaults(pool) == -1)
+if (testStoragePoolObjSetDefaults(pool) == -1) {
+virStoragePoolObjUnlock(pool);
goto error;
+}
pool->active = 1;
+virStoragePoolObjUnlock(pool);
}
if (pools != NULL)
VIR_FREE(pools);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
+testDriverUnlock(privconn);
return (0);
@@ -552,6 +581,7 @@ static int testOpenFromFile(virConnectPt
virDomainObjListFree(&privconn->domains);
virNetworkObjListFree(&privconn->networks);
virStoragePoolObjListFree(&privconn->pools);
+testDriverUnlock(privconn);
VIR_FREE(privconn);
conn->privateData = NULL;
return VIR_DRV_OPEN_ERROR;
@@ -598,11 +628,12 @@ static int testClose(virConnectPtr conn)
static int testClose(virConnectPtr conn)
{
testConnPtr privconn = conn->p
[libvirt] KVM/qemu: problems with autostart of vms with non-bridged nets
Hi,
I'm using libvirt 0.5.0 to manage my kvm domains. Everything works fine except
autostarting vms which use a network device not directly bridged to hw. I
have the following net defined (and set to autostart):
local
7ec7a457-0b8e-4670-3b4a-2d0914288daa
When a vm using this net is autostarted I get the following error:
libvir: error : invalid connection pointer in virNetworkLookupByName
libvir: QEMU error : internal error Network 'local' not found
Failed to autostart VM 'testlocalnet': internal error Network 'local' not
found
I took a dive into the code and found two problems causing this:
1. initialization order:
qemuStateDriver is registered (and thus initialized) before
networkStateDriver. This means the network is not up yet when qemu does it's
autostart.
I could fix this with the attached patch.
2. missing virConnectPtr during autostart of the vm:
in src/qemu_driver.c:145, qemudAutostartConfigs() you can find this call:
int ret = qemudStartVMDaemon(NULL, driver, vm, NULL);
This means virConnectPtr conn is NULL within qemudStartVMDaemon().
Following the calls:
qemudStartVMDaemon()
-> qemudBuildCommandLine()
-> qemudNetworkIfaceConnect()
-> virNetworkLookupByName()
virNetworkLookupByName bails out if called with invalid virConnectPtr. This
means starting this vm fails.
Currently I'm not familiar enough with the libvirt structures to come up with
an idea how to fix this.
Could anybody with a deeper knowledge of the code take a look at this? Thank
you very much.
Kind regards,
Gerd
diff -r -u libvirt-0.5.0.orig/qemud/qemud.c libvirt-0.5.0/qemud/qemud.c
--- libvirt-0.5.0.orig/qemud/qemud.c2008-11-21 13:47:32.0 +0100
+++ libvirt-0.5.0/qemud/qemud.c 2008-11-30 21:27:06.0 +0100
@@ -761,22 +761,13 @@
* If they try to use a open a connection for a module that
* is not loaded they'll get a suitable error at that point
*/
-virDriverLoadModule("qemu");
-virDriverLoadModule("lxc");
-virDriverLoadModule("uml");
virDriverLoadModule("network");
virDriverLoadModule("storage");
virDriverLoadModule("nodedev");
+virDriverLoadModule("qemu");
+virDriverLoadModule("lxc");
+virDriverLoadModule("uml");
#else
-#ifdef WITH_QEMU
-qemuRegister();
-#endif
-#ifdef WITH_LXC
-lxcRegister();
-#endif
-#ifdef WITH_UML
-umlRegister();
-#endif
#ifdef WITH_NETWORK
networkRegister();
#endif
@@ -786,6 +777,15 @@
#if defined(HAVE_HAL) || defined(HAVE_DEVKIT)
nodedevRegister();
#endif
+#ifdef WITH_QEMU
+qemuRegister();
+#endif
+#ifdef WITH_LXC
+lxcRegister();
+#endif
+#ifdef WITH_UML
+umlRegister();
+#endif
#endif
virEventRegisterImpl(virEventAddHandleImpl,
--
Address (better: trap) for people I really don't want to get mail from:
[EMAIL PROTECTED]
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 2/28: Kill off macros in test driver
The test driver has alot of convenience macros for for fetching the private
internal object impls from the public API parameters. Unfortunately these
rather obscure/hide code flow & variable accesses in the test driver, making
it hard to determine whether a method is thread safe. So this patch removes
all the macros, bringing this driver inline with the style of the other
drivers
test.c | 899 -
1 file changed, 670 insertions(+), 229 deletions(-)
Daniel
diff --git a/src/test.c b/src/test.c
--- a/src/test.c
+++ b/src/test.c
@@ -86,80 +86,22 @@ static const virNodeInfo defaultNodeInfo
2,
};
-#define GET_DOMAIN(dom, ret)\
-testConnPtr privconn; \
-virDomainObjPtr privdom;\
-\
-privconn = (testConnPtr)dom->conn->privateData; \
-do {\
-if ((privdom = virDomainFindByName(&privconn->domains, \
-(dom)->name)) == NULL) {\
-testError((dom)->conn, VIR_ERR_INVALID_ARG, __FUNCTION__); \
-return (ret); \
-} \
-} while (0)
-#define GET_NETWORK(net, ret) \
-testConnPtr privconn; \
-virNetworkObjPtr privnet; \
-\
-privconn = (testConnPtr)net->conn->privateData; \
-do {\
-if ((privnet = virNetworkFindByName(&privconn->networks,\
-(net)->name)) == NULL) {\
-testError((net)->conn, VIR_ERR_INVALID_ARG, __FUNCTION__); \
-return (ret); \
-} \
-} while (0)
-#define GET_POOL(pool, ret) \
-testConnPtr privconn; \
-virStoragePoolObjPtr privpool; \
-\
-privconn = (testConnPtr)pool->conn->privateData;\
-do {\
-if ((privpool = virStoragePoolObjFindByName(&privconn->pools, \
-(pool)->name)) == NULL) {\
-testError((pool)->conn, VIR_ERR_INVALID_ARG, __FUNCTION__); \
-return (ret); \
-} \
-} while (0)
-#define GET_POOL_FROM_VOL(vol, ret) \
-GET_POOL(testStoragePoolLookupByName((virConnectPtr)\
- vol->conn, \
- vol->pool), ret)
-#define GET_VOL(vol, pool, ret) \
-virStorageVolDefPtr privvol;\
-\
-privvol = virStorageVolDefFindByName(pool, vol->name); \
-do {\
-if (!privvol) { \
-testError(vol->conn, VIR_ERR_INVALID_STORAGE_VOL, \
- _("no storage vol with matching name '%s'"), \
- vol->name); \
-return (ret); \
-} \
-} while (0) \
-
-#define GET_CONNECTION(conn)\
-testConnPtr privconn; \
-\
-privconn = (testConnPtr)conn->privateData;
-
-#define POOL_IS_ACTIVE(pool, ret) \
-if (!virStoragePoolObjIsActive(pool)) { \
-testError(obj->conn, VIR_ERR_INTERNAL_ERROR,\
- _("storage pool '%s' is not active"), pool->def->name); \
- return (ret)
Re: [libvirt] PATCH: 1/28: Stubs for object locking APIs
To facilitate later patches this introduces lock & unlock API calls for
each internal object which are just no-ops. Once all the drivers are
updated to call this at appropriate places, the stubs will be filled
out with actual locking impls. This ensures each intermediate patch
still results in a functional system
domain_conf.c |8
domain_conf.h |2 ++
libvirt_sym.version.in |8
network_conf.c |8
network_conf.h |3 +++
node_device_conf.c |8
node_device_conf.h |3 +++
storage_conf.c |9 +
storage_conf.h |3 +++
9 files changed, 52 insertions(+)
Daniel
diff --git a/src/domain_conf.c b/src/domain_conf.c
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -3459,4 +3459,12 @@ const char *virDomainDefDefaultEmulator(
}
+void virDomainObjLock(virDomainObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
+
+void virDomainObjUnlock(virDomainObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
+
#endif /* ! PROXY */
diff --git a/src/domain_conf.h b/src/domain_conf.h
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -591,6 +591,8 @@ const char *virDomainDefDefaultEmulator(
virDomainDefPtr def,
virCapsPtr caps);
+void virDomainObjLock(virDomainObjPtr obj);
+void virDomainObjUnlock(virDomainObjPtr obj);
VIR_ENUM_DECL(virDomainVirt)
VIR_ENUM_DECL(virDomainBoot)
diff --git a/src/libvirt_sym.version.in b/src/libvirt_sym.version.in
--- a/src/libvirt_sym.version.in
+++ b/src/libvirt_sym.version.in
@@ -373,6 +373,8 @@ [EMAIL PROTECTED]@ {
virDomainSoundModelTypeToString;
virDomainVirtTypeToString;
virDomainFSDefFree;
+ virDomainObjLock;
+ virDomainObjUnlock;
/* domain_event.h */
@@ -477,6 +479,8 @@ [EMAIL PROTECTED]@ {
virNetworkDefParseNode;
virNetworkRemoveInactive;
virNetworkSaveConfig;
+ virNetworkObjLock;
+ virNetworkObjUnlock;
/* nodeinfo.h */
@@ -491,6 +495,8 @@ [EMAIL PROTECTED]@ {
virNodeDeviceDefFree;
virNodeDevCapsDefFree;
virNodeDeviceDefFormat;
+ virNodeDeviceObjLock;
+ virNodeDeviceObjUnlock;
/* qparams.h */
@@ -544,6 +550,8 @@ [EMAIL PROTECTED]@ {
virStoragePoolFormatFileSystemNetTypeToString;
virStorageVolFormatFileSystemTypeToString;
virStoragePoolTypeFromString;
+ virStoragePoolObjLock;
+ virStoragePoolObjUnlock;
/* util.h */
diff --git a/src/network_conf.c b/src/network_conf.c
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -811,3 +811,11 @@ int virNetworkDeleteConfig(virConnectPtr
return 0;
}
+
+void virNetworkObjLock(virNetworkObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
+
+void virNetworkObjUnlock(virNetworkObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
diff --git a/src/network_conf.h b/src/network_conf.h
--- a/src/network_conf.h
+++ b/src/network_conf.h
@@ -155,5 +155,8 @@ int virNetworkDeleteConfig(virConnectPtr
int virNetworkDeleteConfig(virConnectPtr conn,
virNetworkObjPtr net);
+void virNetworkObjLock(virNetworkObjPtr obj);
+void virNetworkObjUnlock(virNetworkObjPtr obj);
+
#endif /* __NETWORK_CONF_H__ */
diff --git a/src/node_device_conf.c b/src/node_device_conf.c
--- a/src/node_device_conf.c
+++ b/src/node_device_conf.c
@@ -397,3 +397,11 @@ void virNodeDevCapsDefFree(virNodeDevCap
VIR_FREE(caps);
}
+
+void virNodeDeviceObjLock(virNodeDeviceObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
+
+void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
diff --git a/src/node_device_conf.h b/src/node_device_conf.h
--- a/src/node_device_conf.h
+++ b/src/node_device_conf.h
@@ -190,4 +190,7 @@ void virNodeDeviceObjListFree(virNodeDev
void virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps);
+void virNodeDeviceObjLock(virNodeDeviceObjPtr obj);
+void virNodeDeviceObjUnlock(virNodeDeviceObjPtr obj);
+
#endif /* __VIR_NODE_DEVICE_CONF_H__ */
diff --git a/src/storage_conf.c b/src/storage_conf.c
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -1497,3 +1497,12 @@ char *virStoragePoolSourceListFormat(vir
free(virBufferContentAndReset(&buf));
return NULL;
}
+
+
+void virStoragePoolObjLock(virStoragePoolObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
+
+void virStoragePoolObjUnlock(virStoragePoolObjPtr obj ATTRIBUTE_UNUSED)
+{
+}
diff --git a/src/storage_conf.h b/src/storage_conf.h
--- a/src/storage_conf.h
+++ b/src/storage_conf.h
@@ -326,6 +326,9 @@ char *virStoragePoolSourceListFormat(vir
char *virStoragePoolSourceListFormat(virConnectPtr conn,
virStoragePoolSourceListPtr def);
+void virStoragePoolObjLock(virStoragePoolObjPtr obj);
+void virStoragePoolObjUnlock(virStoragePoolObjPtr obj);
+
enum virStoragePoolFormatFileSystem {
VIR_STORAGE_POOL_FS_AUTO = 0,
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
[libvirt] PATCH: 0/28: Thread safety for libvirtd daemon and drivers
The following huge series of patches adds thread safety for the libvirtd daemon and drivers, and makes the daemon multi-threaded in processing RPC calls. This enables multiple clients to be processed in parallel, without blocking each other. It does not change the thread rules for the virConnectPtr object though, so each individual client is still serialized. There are two core places where we have to have synchronization in the threading model this patch series introduces - The libvirt daemon code - A single global server lock (aka struct qemud_server) - One lock per client connection (aka struct qemud_client) - The driver implementations - One lock per driver (aka QEMU, LXC, Test, UML, OpenVZ, Network, Storage, Node Devices) - One lock per primary object in a driver (virDomainObjPtr, virNetworkObjPtr, virStoragePoolObjPtr, virNodeDeviceObjptr instances) For most cases, the big global server / driver locks are only held while obtaining one of the finer grained locks. This gives a fairly good level of concurrency for operations touching different objects. Once this core infrastructure is merged, it will be possible to iterate on impl of drivers to reduce the time locks are held - eg avoid holding a lock while talking to the QEMU monitor interface. To try and make it easier to spot thread locking problems this series refactors alot of methods so that there is only a single return point where the unlock call can be placed, rather than multiple return point which increases the chances of missing an unlock call. This touches a huge amount of code, so I'd like to get this all merged ASAP as it'll be really hard to keep it synced with ongoing changes. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Concerning umlInotifyEvent
"Daniel P. Berrange" <[EMAIL PROTECTED]> wrote: >That's a symptom of a bug earlier on. Try this patch: Thanks, Daniel, that seems to have fixed it. Ron -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Memory leak in xen_inotify.c
On Sat, Nov 29, 2008 at 01:15:52PM -0800, Niraj Tolia wrote:
> Hi,
>
> It looks like xenInotifyOpen() in xen_inotify.c has an opendir() call.
> However, as there is no corresponding closedir() and opendir() uses
> malloc internally, valgrind reports this as a memory leak. Adding a
> closedir() to the end of this function should fix this.
Thanks for the bug info, I've committed the following patch:
diff -u -r1.1 xen_inotify.c
--- xen_inotify.c 25 Nov 2008 10:44:53 - 1.1
+++ xen_inotify.c 30 Nov 2008 18:36:00 -
@@ -395,7 +395,7 @@
return -1;
}
}
-
+closedir(dh);
}
if ((priv->inotifyFD = inotify_init()) < 0) {
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Concerning umlInotifyEvent
On Sun, Nov 30, 2008 at 03:17:04PM +, Ron Yorston wrote:
> At the top of umlInotifyEvent in uml_driver.c there is this test:
>
> if (watch != driver->inotifyWatch)
> return;
>
> This doesn't seem to be correct. I have to comment out the test to get
> libvirtd to work with UML.
That's a symptom of a bug earlier on. Try this patch:
Index: src/uml_driver.c
===
RCS file: /data/cvs/libvirt/src/uml_driver.c,v
retrieving revision 1.4
diff -u -p -r1.4 uml_driver.c
--- src/uml_driver.c28 Nov 2008 12:03:20 - 1.4
+++ src/uml_driver.c30 Nov 2008 18:34:43 -
@@ -348,16 +348,16 @@ umlStartup(void) {
return -1;
}
-if ((uml_driver->inotifyWatch =
- inotify_add_watch(uml_driver->inotifyFD,
- uml_driver->monitorDir,
- IN_CREATE | IN_MODIFY | IN_DELETE)) < 0) {
+if (inotify_add_watch(uml_driver->inotifyFD,
+ uml_driver->monitorDir,
+ IN_CREATE | IN_MODIFY | IN_DELETE) < 0) {
umlShutdown();
return -1;
}
-if (virEventAddHandle(uml_driver->inotifyFD, POLLIN,
- umlInotifyEvent, uml_driver, NULL) < 0) {
+if ((uml_driver->inotifyWatch =
+ virEventAddHandle(uml_driver->inotifyFD, POLLIN,
+ umlInotifyEvent, uml_driver, NULL)) < 0) {
umlShutdown();
return -1;
}
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Concerning umlInotifyEvent
At the top of umlInotifyEvent in uml_driver.c there is this test: if (watch != driver->inotifyWatch) return; This doesn't seem to be correct. I have to comment out the test to get libvirtd to work with UML. Ron -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [PATCH/RFC] kvm/qemu vs libvirtd restarts
On Wed, Nov 26, 2008 at 08:54:40PM +, Daniel P. Berrange wrote:
[..snip..]
> Hmm, interesting idea - we already have a variant for parsing based
> off an arbitrary XML node, rather than assuming the root,
>
> virDomainDefPtr virDomainDefParseNode(virConnectPtr conn,
> virCapsPtr caps,
> xmlDocPtr doc,
> xmlNodePtr root);
>
> So we could add a wrapper function which writes out a XML doc
>
>
>
>
> ... normal domain XML config ...
>
>
>
> And so have a function
>
>virDomainObjPtr virDomainObjParse(const char *filename)
>
> which'd parse the custom QEMU state, and then just invoke the
> virDomainDefParseNode for the nested tag.
Yes, nice - this simplifies things. I'll try that.
> Now, some things like VNC port, domain ID really are easily tracked
> in the regular domain XML - rather than adding VIR_DOMAIN_XML_STATE
> I've a slight alternative idea.
>
> The formatXML methods already accept a flag VIR_DOMAIN_XML_INACTIVE
> which when passed causes it to skip some attributes which only
> make sense for running VMs - VNC port, ID, VIF name
>
> The parseXML methods, just hardcode this and always skip these
> attributes. We should fix this so the parsing only skips these
> attrs if this same VIR_DOMAIN_XML_INACTIVE flag is passed. Then
> just make sure all existing code is changed to set this flag
> when parsing XML. The code for loading VM from disk can simply
> not set this flag. Technically the LXC driver shold be doing
> this already, precisely for the VIF name issue.
Like the attached patch?
-- Guido
>From 87db4a698ed9b49294c0f94137fc6beef13bd4e8 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Guido=20G=C3=BCnther?= <[EMAIL PROTECTED]>
Date: Tue, 25 Nov 2008 13:02:43 +0100
Subject: [PATCH] differentiate between active and inactive configs
by honoring the VIR_DOMAIN_XML_INACTIVE flag.
---
src/domain_conf.c| 35 +++
src/domain_conf.h|6 --
src/lxc_controller.c |3 ++-
src/lxc_driver.c |2 +-
src/test.c |6 --
tests/qemuxml2argvtest.c |3 ++-
6 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/src/domain_conf.c b/src/domain_conf.c
index 4adab69..292cecf 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -1318,7 +1318,7 @@ error:
/* Parse the XML definition for a graphics device */
static virDomainGraphicsDefPtr
virDomainGraphicsDefParseXML(virConnectPtr conn,
- xmlNodePtr node) {
+ xmlNodePtr node, int flags) {
virDomainGraphicsDefPtr def;
char *type = NULL;
@@ -1355,7 +1355,8 @@ virDomainGraphicsDefParseXML(virConnectPtr conn,
VIR_FREE(port);
/* Legacy compat syntax, used -1 for auto-port */
if (def->data.vnc.port == -1) {
-def->data.vnc.port = 0;
+if (flags & VIR_DOMAIN_XML_INACTIVE)
+def->data.vnc.port = 0;
def->data.vnc.autoport = 1;
}
} else {
@@ -1365,7 +1366,8 @@ virDomainGraphicsDefParseXML(virConnectPtr conn,
if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
if (STREQ(autoport, "yes")) {
-def->data.vnc.port = 0;
+if (flags & VIR_DOMAIN_XML_INACTIVE)
+def->data.vnc.port = 0;
def->data.vnc.autoport = 1;
}
VIR_FREE(autoport);
@@ -1699,11 +1701,12 @@ int virDomainDiskQSort(const void *a, const void *b)
#ifndef PROXY
static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
virCapsPtr caps,
-xmlXPathContextPtr ctxt)
+xmlXPathContextPtr ctxt, int flags)
{
xmlNodePtr *nodes = NULL, node = NULL;
char *tmp = NULL;
int i, n;
+long id = -1;
virDomainDefPtr def;
if (VIR_ALLOC(def) < 0) {
@@ -1711,7 +1714,11 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
"%s", _("failed to allocate space for xmlXPathContext"));
return NULL;
}
-def->id = -1;
+
+if (!(flags & VIR_DOMAIN_XML_INACTIVE))
+if((virXPathLong(conn, "string(./@id)", ctxt, &id)) < 0)
+id = -1;
+def->id = (int)id;
/* Find out what type of virtualization to use */
if (!(tmp = virXPathString(conn, "string(./@type)", ctxt))) {
@@ -2101,7 +2108,8 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
}
if (n > 0) {
virDomainGraphicsDefPtr graphics = virDomainGraphicsDefParseXML(conn,
-nodes[0]);
+nodes[0],
+
