virObject is the base struct that manages reference-counting for all structs that need the ability of reference-counting. virAtomic provides atomic operations which are thread-safe. --- src/Makefile.am | 2 + src/libvirt_private.syms | 5 ++++ src/util/viratomic.c | 46 ++++++++++++++++++++++++++++++++++++++++ src/util/viratomic.h | 30 ++++++++++++++++++++++++++ src/util/virobject.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virobject.h | 39 ++++++++++++++++++++++++++++++++++ 6 files changed, 174 insertions(+), 0 deletions(-) create mode 100644 src/util/viratomic.c create mode 100644 src/util/viratomic.h create mode 100644 src/util/virobject.c create mode 100644 src/util/virobject.h diff --git a/src/Makefile.am b/src/Makefile.am index 9b54679..9e74060 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -81,6 +81,8 @@ UTIL_SOURCES = \ util/util.c util/util.h \ util/xml.c util/xml.h \ util/virtaudit.c util/virtaudit.h \ + util/viratomic.c util/viratomic.h \ + util/virobject.c util/virobject.h \ util/virterror.c util/virterror_internal.h EXTRA_DIST += util/threads-pthread.c util/threads-win32.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 65a86d3..98f8d6f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -993,3 +993,8 @@ virXPathUInt; virXPathULong; virXPathULongHex; virXPathULongLong; + +# object.h +virObjectInit; +virObjectRef; +virObjectUnref; diff --git a/src/util/viratomic.c b/src/util/viratomic.c new file mode 100644 index 0000000..629f189 --- /dev/null +++ b/src/util/viratomic.c @@ -0,0 +1,46 @@ +/* + * viratomic.c: atomic operations + * + * Copyright (C) 2011 Hu Tao + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Hu Tao <hutao@cn.fujitsu.com> + */ + +#include "viratomic.h" + +#ifdef WIN32 +long virAtomicInc(long *value) +{ + return InterlockedIncrement(value); +} + +long virAtomicDec(long *value) +{ + return InterlockedDecrement(value); +} +#else /* WIN32 */ +long virAtomicInc(long *value) +{ + return __sync_add_and_fetch(value, 1); +} + +long virAtomicDec(long *value) +{ + return __sync_sub_and_fetch(value, 1); +} +#endif /* WIN32 */ diff --git a/src/util/viratomic.h b/src/util/viratomic.h new file mode 100644 index 0000000..a10cb65 --- /dev/null +++ b/src/util/viratomic.h @@ -0,0 +1,30 @@ +/* + * viratomic.h: atomic operations + * + * Copyright (C) 2011 Hu Tao + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Hu Tao <hutao@cn.fujitsu.com> + */ + +#ifndef __VIR_ATOMIC_H +#define __VIR_ATOMIC_H + +long virAtomicInc(long *value); +long virAtomicDec(long *value); + +#endif /* __VIR_ATOMIC_H */ diff --git a/src/util/virobject.c b/src/util/virobject.c new file mode 100644 index 0000000..aeedbef --- /dev/null +++ b/src/util/virobject.c @@ -0,0 +1,52 @@ +/* + * virobject.c: base object that manages reference-counting + * + * Copyright (C) 2011 Hu Tao + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Hu Tao <hutao@cn.fujitsu.com> + */ + +#include "viratomic.h" +#include "virobject.h" +#include "logging.h" + +int virObjectInit(virObjectPtr obj, void (*free)(virObjectPtr obj)) +{ + if (!free) { + VIR_ERROR0("method free is required."); + return -1; + } + + obj->ref = 1; + obj->free = free; + + return 0; +} + +void virObjectRef(virObjectPtr obj) +{ + sa_assert(obj->ref > 0); + virAtomicInc(&obj->ref); +} + +void virObjectUnref(virObjectPtr obj) +{ + sa_assert(obj->ref > 0); + if (virAtomicDec(&obj->ref) == 0) + obj->free(obj); +} diff --git a/src/util/virobject.h b/src/util/virobject.h new file mode 100644 index 0000000..cd7d3e8 --- /dev/null +++ b/src/util/virobject.h @@ -0,0 +1,39 @@ +/* + * virobject.h: base object that manages reference-counting + * + * Copyright (C) 2011 Hu Tao + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Authors: + * Hu Tao <hutao@cn.fujitsu.com> + */ + +#ifndef __VIR_OBJECT_H +#define __VIR_OBJECT_H + +typedef struct _virObject virObject; +typedef virObject *virObjectPtr; + +struct _virObject { + long ref; + void (*free)(virObjectPtr obj); +}; + +int virObjectInit(virObjectPtr obj, void (*free)(virObjectPtr obj)); +void virObjectRef(virObjectPtr obj); +void virObjectUnref(virObjectPtr obj); + +#endif /* __VIR_OBJECT_H */ -- 1.7.3.1 -- Thanks, Hu Tao

--- daemon/dispatch.c | 2 - daemon/libvirtd.c | 62 +++++++++++++++++++++++++++++++++++++++------------- daemon/libvirtd.h | 5 ++- 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/daemon/dispatch.c b/daemon/dispatch.c index 4814017..b2d6927 100644 --- a/daemon/dispatch.c +++ b/daemon/dispatch.c @@ -524,9 +524,7 @@ remoteDispatchClientCall (struct qemud_server *server, */ rv = (data->fn)(server, client, conn, &msg->hdr, &rerr, &args, &ret); - virMutexLock(&server->lock); virMutexLock(&client->lock); - virMutexUnlock(&server->lock); xdr_free (data->args_filter, (char*)&args); diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 42cbe5d..d3ba97b 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -270,6 +270,8 @@ static void sig_fatal(int sig, siginfo_t * siginfo ATTRIBUTE_UNUSED, static void qemudDispatchClientEvent(int watch, int fd, int events, void *opaque); static void qemudDispatchServerEvent(int watch, int fd, int events, void *opaque); static int qemudStartWorker(struct qemud_server *server, struct qemud_worker *worker); +static void qemudClientRef(struct qemud_client *client); +static void qemudClientUnref(struct qemud_client *client); void qemudClientMessageQueuePush(struct qemud_client_message **queue, @@ -1338,6 +1340,8 @@ int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid) { #endif +static void qemudFreeClient(virObjectPtr obj); + static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket *sock) { int fd; virSocketAddr addr; @@ -1411,6 +1415,10 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket VIR_ERROR0(_("cannot initialize mutex")); goto error; } + if (virObjectInit(&client->obj, qemudFreeClient)) { + VIR_ERROR0(_("cannot initialize virobject")); + goto error; + } client->magic = QEMUD_CLIENT_MAGIC; client->fd = fd; @@ -1576,12 +1584,10 @@ static struct qemud_client *qemudPendingJob(struct qemud_server *server) { int i; for (i = 0 ; i < server->nclients ; i++) { - virMutexLock(&server->clients[i]->lock); if (server->clients[i]->dx) { - /* Delibrately don't unlock client - caller wants the lock */ + qemudClientRef(server->clients[i]); return server->clients[i]; } - virMutexUnlock(&server->clients[i]->lock); } return NULL; } @@ -1604,31 +1610,36 @@ static void *qemudWorker(void *data) } } if (worker->quitRequest) { - virMutexUnlock(&client->lock); virMutexUnlock(&server->lock); + qemudClientUnref(client); return NULL; } worker->processingCall = 1; virMutexUnlock(&server->lock); - /* We own a locked client now... */ - client->refs++; + virMutexLock(&client->lock); /* Remove our message from dispatch queue while we use it */ msg = qemudClientMessageQueueServe(&client->dx); + if (!msg) { + virMutexUnlock(&client->lock); + qemudClientUnref(client); + continue; + } + /* This function drops the lock during dispatch, * and re-acquires it before returning */ if (remoteDispatchClientRequest (server, client, msg) < 0) { VIR_FREE(msg); qemudDispatchClientFailure(client); - client->refs--; virMutexUnlock(&client->lock); + qemudClientUnref(client); continue; } - client->refs--; virMutexUnlock(&client->lock); + qemudClientUnref(client); virMutexLock(&server->lock); worker->processingCall = 0; @@ -2155,6 +2166,7 @@ qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) { virMutexLock(&server->clients[i]->lock); if (server->clients[i]->watch == watch) { client = server->clients[i]; + qemudClientRef(client); break; } virMutexUnlock(&server->clients[i]->lock); @@ -2168,6 +2180,7 @@ qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) { if (client->fd != fd) { virMutexUnlock(&client->lock); + qemudClientUnref(client); return; } @@ -2190,6 +2203,7 @@ qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) { qemudDispatchClientFailure(client); virMutexUnlock(&client->lock); + qemudClientUnref(client); } @@ -2306,7 +2320,9 @@ static void qemudInactiveTimer(int timerid, void *data) { } } -static void qemudFreeClient(struct qemud_client *client) { +static void qemudFreeClient(virObjectPtr obj) { + struct qemud_client *client = (struct qemud_client *)obj; + while (client->rx) { struct qemud_client_message *msg = qemudClientMessageQueueServe(&client->rx); @@ -2333,6 +2349,16 @@ static void qemudFreeClient(struct qemud_client *client) { VIR_FREE(client); } +static void qemudClientRef(struct qemud_client *client) +{ + virObjectRef(&client->obj); +} + +static void qemudClientUnref(struct qemud_client *client) +{ + virObjectUnref(&client->obj); +} + static void *qemudRunLoop(void *opaque) { struct qemud_server *server = opaque; int timerid = -1; @@ -2396,13 +2422,17 @@ static void *qemudRunLoop(void *opaque) { reprocess: for (i = 0 ; i < server->nclients ; i++) { - int inactive; + struct qemud_client *client = NULL; virMutexLock(&server->clients[i]->lock); - inactive = server->clients[i]->fd == -1 - && server->clients[i]->refs == 0; - virMutexUnlock(&server->clients[i]->lock); - if (inactive) { - qemudFreeClient(server->clients[i]); + if (server->clients[i]->fd == -1) { + client = server->clients[i]; + server->clients[i] = NULL; + } + if (!client) { + virMutexUnlock(&server->clients[i]->lock); + } else { + virMutexUnlock(&client->lock); + qemudClientUnref(client); server->nclients--; if (i < server->nclients) memmove(server->clients + i, @@ -2451,7 +2481,7 @@ cleanup: } VIR_FREE(server->workers); for (i = 0; i < server->nclients; i++) - qemudFreeClient(server->clients[i]); + qemudClientUnref(server->clients[i]); server->nclients = 0; VIR_SHRINK_N(server->clients, server->nclients_max, server->nclients_max); diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index d37c3fd..5ac4eb3 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -49,6 +49,7 @@ # include "logging.h" # include "threads.h" # include "network.h" +# include "virobject.h" # if WITH_DTRACE # ifndef LIBVIRTD_PROBES_H @@ -186,6 +187,8 @@ struct qemud_client_stream { /* Stores the per-client connection state */ struct qemud_client { + virObject obj; /* should be the first member */ + virMutex lock; int magic; @@ -243,8 +246,6 @@ struct qemud_client { * called, it will be set back to NULL if that succeeds. */ virConnectPtr conn; - int refs; - }; # define QEMUD_CLIENT_MAGIC 0x7788aaee -- 1.7.3.1 -- Thanks, Hu Tao

This patch also eliminates a dead-lock bug in qemuDomainObjBeginJobWithDriver: if virCondWaitUntil() timeouts, the thread tries to acquire qemu driver lock while holding virDomainObj lock. --- src/conf/domain_conf.c | 56 ++++---- src/conf/domain_conf.h | 6 +- src/openvz/openvz_conf.c | 8 +- src/qemu/qemu_domain.c | 32 ++--- src/qemu/qemu_domain.h | 2 +- src/qemu/qemu_driver.c | 304 ++++++++++++++++++++------------------------- src/qemu/qemu_migration.c | 45 +++---- src/qemu/qemu_process.c | 33 ++--- src/vmware/vmware_conf.c | 2 +- 9 files changed, 215 insertions(+), 273 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 90a1317..fc76a00 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -47,6 +47,7 @@ #include "storage_file.h" #include "files.h" #include "bitmap.h" +#include "virobject.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -395,9 +396,7 @@ static void virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { virDomainObjPtr obj = payload; - virDomainObjLock(obj); - if (virDomainObjUnref(obj) > 0) - virDomainObjUnlock(obj); + virDomainObjUnref(obj); } int virDomainObjListInit(virDomainObjListPtr doms) @@ -437,7 +436,7 @@ virDomainObjPtr virDomainFindByID(const virDomainObjListPtr doms, virDomainObjPtr obj; obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id); if (obj) - virDomainObjLock(obj); + virDomainObjRef(obj); return obj; } @@ -452,7 +451,7 @@ virDomainObjPtr virDomainFindByUUID(const virDomainObjListPtr doms, obj = virHashLookup(doms->objs, uuidstr); if (obj) - virDomainObjLock(obj); + virDomainObjRef(obj); return obj; } @@ -476,7 +475,7 @@ virDomainObjPtr virDomainFindByName(const virDomainObjListPtr doms, virDomainObjPtr obj; obj = virHashSearch(doms->objs, virDomainObjListSearchName, name); if (obj) - virDomainObjLock(obj); + virDomainObjRef(obj); return obj; } @@ -967,6 +966,12 @@ static void virDomainObjFree(virDomainObjPtr dom) { if (!dom) return; + virDomainObjUnref(dom); +} + +static void doDomainObjFree(virObjectPtr obj) +{ + virDomainObjPtr dom = (virDomainObjPtr)obj; VIR_DEBUG("obj=%p", dom); virDomainDefFree(dom->def); @@ -984,21 +989,13 @@ static void virDomainObjFree(virDomainObjPtr dom) void virDomainObjRef(virDomainObjPtr dom) { - dom->refs++; - VIR_DEBUG("obj=%p refs=%d", dom, dom->refs); + virObjectRef(&dom->obj); } -int virDomainObjUnref(virDomainObjPtr dom) +void virDomainObjUnref(virDomainObjPtr dom) { - dom->refs--; - VIR_DEBUG("obj=%p refs=%d", dom, dom->refs); - if (dom->refs == 0) { - virDomainObjUnlock(dom); - virDomainObjFree(dom); - return 0; - } - return dom->refs; + virObjectUnref(&dom->obj); } static virDomainObjPtr virDomainObjNew(virCapsPtr caps) @@ -1010,6 +1007,11 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps) return NULL; } + if (virObjectInit(&domain->obj, doDomainObjFree)) { + VIR_FREE(domain); + return NULL; + } + if (caps->privateDataAllocFunc && !(domain->privateData = (caps->privateDataAllocFunc)())) { virReportOOMError(); @@ -1027,9 +1029,7 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps) return NULL; } - virDomainObjLock(domain); domain->state = VIR_DOMAIN_SHUTOFF; - domain->refs = 1; virDomainSnapshotObjListInit(&domain->snapshots); @@ -1075,8 +1075,10 @@ virDomainObjPtr virDomainAssignDef(virCapsPtr caps, domain->def = def; virUUIDFormat(def->uuid, uuidstr); + virDomainObjRef(domain); if (virHashAddEntry(doms->objs, uuidstr, domain) < 0) { - VIR_FREE(domain); + virDomainObjUnref(domain); + virDomainObjFree(domain); return NULL; } @@ -1149,9 +1151,7 @@ virDomainObjGetPersistentDef(virCapsPtr caps, } /* - * The caller must hold a lock on the driver owning 'doms', - * and must also have locked 'dom', to ensure no one else - * is either waiting for 'dom' or still usingn it + * The caller must hold a lock on the driver owning 'doms'. */ void virDomainRemoveInactive(virDomainObjListPtr doms, virDomainObjPtr dom) @@ -1159,9 +1159,8 @@ void virDomainRemoveInactive(virDomainObjListPtr doms, char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->def->uuid, uuidstr); - virDomainObjUnlock(dom); - virHashRemoveEntry(doms->objs, uuidstr); + virDomainObjUnref(dom); } @@ -6146,7 +6145,7 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, error: /* obj was never shared, so unref should return 0 */ - ignore_value(virDomainObjUnref(obj)); + virDomainObjUnref(obj); return NULL; } @@ -8449,10 +8448,12 @@ static virDomainObjPtr virDomainLoadConfig(virCapsPtr caps, if (!(dom = virDomainAssignDef(caps, doms, def, false))) goto error; + virDomainObjLock(dom); dom->autostart = autostart; if (notify) (*notify)(dom, newVM, opaque); + virDomainObjUnlock(dom); VIR_FREE(configFile); VIR_FREE(autostartLink); @@ -8501,9 +8502,8 @@ static virDomainObjPtr virDomainLoadStatus(virCapsPtr caps, return obj; error: - /* obj was never shared, so unref should return 0 */ if (obj) - ignore_value(virDomainObjUnref(obj)); + virDomainObjUnref(obj); VIR_FREE(statusFile); return NULL; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 10e73cb..3218672 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -40,6 +40,7 @@ # include "nwfilter_conf.h" # include "macvtap.h" # include "sysinfo.h" +# include "virobject.h" /* Private component of virDomainXMLFlags */ typedef enum { @@ -1144,8 +1145,8 @@ struct _virDomainDef { typedef struct _virDomainObj virDomainObj; typedef virDomainObj *virDomainObjPtr; struct _virDomainObj { + virObject obj; virMutex lock; - int refs; int pid; int state; @@ -1226,8 +1227,7 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, void virDomainDefFree(virDomainDefPtr vm); void virDomainObjRef(virDomainObjPtr vm); -/* Returns 1 if the object was freed, 0 if more refs exist */ -int virDomainObjUnref(virDomainObjPtr vm) ATTRIBUTE_RETURN_CHECK; +void virDomainObjUnref(virDomainObjPtr vm); /* live == true means def describes an active domain (being migrated or * restored) as opposed to a new persistent configuration of the domain */ diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 88cd4c8..c08ed3b 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -472,6 +472,11 @@ int openvzLoadDomains(struct openvz_driver *driver) { if (VIR_ALLOC(dom) < 0) goto no_memory; + if (virObjectInit(&dom->obj, NULL)) { + VIR_FREE(dom); + goto cleanup; + } + if (virMutexInit(&dom->lock) < 0) { openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot initialize mutex")); @@ -489,7 +494,6 @@ int openvzLoadDomains(struct openvz_driver *driver) { else dom->state = VIR_DOMAIN_RUNNING; - dom->refs = 1; dom->pid = veid; dom->def->id = dom->state == VIR_DOMAIN_SHUTOFF ? -1 : veid; /* XXX OpenVZ doesn't appear to have concept of a transient domain */ @@ -554,7 +558,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { VIR_FREE(outbuf); /* dom hasn't been shared yet, so unref should return 0 */ if (dom) - ignore_value(virDomainObjUnref(dom)); + virDomainObjUnref(dom); return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c2a1f9a..3a3c953 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -457,13 +457,13 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) } then = timeval_to_ms(now) + QEMU_JOB_WAIT_TIME; - virDomainObjRef(obj); + virDomainObjLock(obj); while (priv->jobActive) { if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) { - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(obj)); - if (errno == ETIMEDOUT) + int err = errno; + virDomainObjUnlock(obj); + if (err == ETIMEDOUT) qemuReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", _("cannot acquire state change lock")); else @@ -482,12 +482,10 @@ int qemuDomainObjBeginJob(virDomainObjPtr obj) } /* - * obj must be locked before calling, qemud_driver must be locked - * * This must be called by anything that will change the VM state * in any way, or anything that will use the QEMU monitor. */ -int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, +int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver ATTRIBUTE_UNUSED, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; @@ -501,20 +499,18 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, } then = timeval_to_ms(now) + QEMU_JOB_WAIT_TIME; - virDomainObjRef(obj); - qemuDriverUnlock(driver); + virDomainObjLock(obj); while (priv->jobActive) { if (virCondWaitUntil(&priv->jobCond, &obj->lock, then) < 0) { - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(obj)); - if (errno == ETIMEDOUT) + int err = errno; + virDomainObjUnlock(obj); + if (err == ETIMEDOUT) qemuReportError(VIR_ERR_OPERATION_TIMEOUT, "%s", _("cannot acquire state change lock")); else virReportSystemError(errno, "%s", _("cannot acquire job mutex")); - qemuDriverLock(driver); return -1; } } @@ -524,10 +520,6 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, priv->jobStart = timeval_to_ms(now); memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); - virDomainObjUnlock(obj); - qemuDriverLock(driver); - virDomainObjLock(obj); - return 0; } @@ -540,7 +532,7 @@ int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, * Returns remaining refcount on 'obj', maybe 0 to indicated it * was deleted */ -int qemuDomainObjEndJob(virDomainObjPtr obj) +void qemuDomainObjEndJob(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; @@ -551,7 +543,7 @@ int qemuDomainObjEndJob(virDomainObjPtr obj) memset(&priv->jobInfo, 0, sizeof(priv->jobInfo)); virCondSignal(&priv->jobCond); - return virDomainObjUnref(obj); + virDomainObjUnlock(obj); } @@ -655,7 +647,7 @@ void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver, virDomainObjLock(obj); /* Safe to ignore value, since we incremented ref in * qemuDomainObjEnterRemoteWithDriver */ - ignore_value(virDomainObjUnref(obj)); + virDomainObjUnref(obj); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 8258900..12fd21d 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -97,7 +97,7 @@ void qemuDomainSetNamespaceHooks(virCapsPtr caps); int qemuDomainObjBeginJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; int qemuDomainObjBeginJobWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; -int qemuDomainObjEndJob(virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; +void qemuDomainObjEndJob(virDomainObjPtr obj); void qemuDomainObjEnterMonitor(virDomainObjPtr obj); void qemuDomainObjExitMonitor(virDomainObjPtr obj); void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 48fe266..db89402 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -139,7 +139,6 @@ qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaq struct qemuAutostartData *data = opaque; virErrorPtr err; - virDomainObjLock(vm); virResetLastError(); if (qemuDomainObjBeginJobWithDriver(data->driver, vm) < 0) { err = virGetLastError(); @@ -156,12 +155,8 @@ qemuAutostartDomain(void *payload, const void *name ATTRIBUTE_UNUSED, void *opaq err ? err->message : _("unknown error")); } - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); } - - if (vm) - virDomainObjUnlock(vm); } @@ -1055,12 +1050,13 @@ static virDomainPtr qemudDomainLookupByID(virConnectPtr conn, goto cleanup; } + virDomainObjLock(vm); dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + virDomainObjUnlock(vm); + virDomainObjUnref(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); return dom; } @@ -1082,12 +1078,13 @@ static virDomainPtr qemudDomainLookupByUUID(virConnectPtr conn, goto cleanup; } + virDomainObjLock(vm); dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + virDomainObjUnlock(vm); + virDomainObjUnref(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); return dom; } @@ -1107,12 +1104,13 @@ static virDomainPtr qemudDomainLookupByName(virConnectPtr conn, goto cleanup; } + virDomainObjLock(vm); dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; + virDomainObjUnlock(vm); + virDomainObjUnref(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); return dom; } @@ -1133,11 +1131,13 @@ static int qemuDomainIsActive(virDomainPtr dom) _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } + + virDomainObjLock(obj); ret = virDomainObjIsActive(obj); + virDomainObjUnlock(obj); + virDomainObjUnref(obj); cleanup: - if (obj) - virDomainObjUnlock(obj); return ret; } @@ -1157,11 +1157,13 @@ static int qemuDomainIsPersistent(virDomainPtr dom) _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } + virDomainObjLock(obj); ret = obj->persistent; + virDomainObjUnlock(obj); cleanup: if (obj) - virDomainObjUnlock(obj); + virDomainObjUnref(obj); return ret; } @@ -1181,11 +1183,13 @@ static int qemuDomainIsUpdated(virDomainPtr dom) _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } + virDomainObjLock(obj); ret = obj->updated; + virDomainObjUnlock(obj); cleanup: if (obj) - virDomainObjUnlock(obj); + virDomainObjUnref(obj); return ret; } @@ -1239,39 +1243,57 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, qemuDriverLock(driver); if (!(def = virDomainDefParseString(driver->caps, xml, - VIR_DOMAIN_XML_INACTIVE))) + VIR_DOMAIN_XML_INACTIVE))) { + qemuDriverUnlock(driver); goto cleanup; + } - if (virSecurityManagerVerify(driver->securityManager, def) < 0) + if (virSecurityManagerVerify(driver->securityManager, def) < 0) { + qemuDriverUnlock(driver); goto cleanup; + } - if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) + if (virDomainObjIsDuplicate(&driver->domains, def, 1) < 0) { + qemuDriverUnlock(driver); goto cleanup; + } - if (qemudCanonicalizeMachine(driver, def) < 0) + if (qemudCanonicalizeMachine(driver, def) < 0) { + qemuDriverUnlock(driver); goto cleanup; + } - if (qemuDomainAssignPCIAddresses(def) < 0) + if (qemuDomainAssignPCIAddresses(def) < 0) { + qemuDriverUnlock(driver); goto cleanup; + } if (!(vm = virDomainAssignDef(driver->caps, &driver->domains, - def, false))) + def, false))) { + qemuDriverUnlock(driver); goto cleanup; + } + + qemuDriverUnlock(driver); def = NULL; - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) { + virDomainObjUnref(vm); goto cleanup; /* XXXX free the 'vm' we created ? */ + } if (qemuProcessStart(conn, driver, vm, NULL, (flags & VIR_DOMAIN_START_PAUSED) != 0, -1, NULL, VIR_VM_OP_CREATE) < 0) { qemuAuditDomainStart(vm, "booted", false); - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, - vm); - vm = NULL; + qemuDomainObjEndJob(vm); + qemuDriverLock(driver); + virDomainRemoveInactive(&driver->domains, + vm); + qemuDriverUnlock(driver); + virDomainObjUnref(vm); goto cleanup; } @@ -1283,17 +1305,13 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml, dom = virGetDomain(conn, vm->def->name, vm->def->uuid); if (dom) dom->id = vm->def->id; - if (vm && - qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); + virDomainObjUnref(vm); cleanup: virDomainDefFree(def); - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); - qemuDriverUnlock(driver); return dom; } @@ -1307,13 +1325,14 @@ static int qemudDomainSuspend(virDomainPtr dom) { qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, @@ -1354,16 +1373,12 @@ static int qemudDomainSuspend(virDomainPtr dom) { } endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); - + virDomainObjUnref(vm); if (event) qemuDomainEventQueue(driver, event); - qemuDriverUnlock(driver); return ret; } @@ -1376,6 +1391,7 @@ static int qemudDomainResume(virDomainPtr dom) { qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1409,15 +1425,12 @@ static int qemudDomainResume(virDomainPtr dom) { ret = 0; endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); + virDomainObjUnref(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); - qemuDriverUnlock(driver); return ret; } @@ -1437,7 +1450,7 @@ static int qemudDomainShutdown(virDomainPtr dom) { virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } if (qemuDomainObjBeginJob(vm) < 0) @@ -1455,12 +1468,10 @@ static int qemudDomainShutdown(virDomainPtr dom) { qemuDomainObjExitMonitor(vm); endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return ret; } @@ -1473,12 +1484,13 @@ static int qemudDomainDestroy(virDomainPtr dom) { qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) @@ -1497,24 +1509,24 @@ static int qemudDomainDestroy(virDomainPtr dom) { qemuAuditDomainStop(vm, "destroyed"); if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, - vm); + qemuDomainObjEndJob(vm); + virDomainObjUnref(vm); + qemuDriverLock(driver); + virDomainRemoveInactive(&driver->domains, vm); + qemuDriverUnlock(driver); vm = NULL; } ret = 0; endjob: - if (vm && - qemuDomainObjEndJob(vm) == 0) - vm = NULL; + if (vm) + qemuDomainObjEndJob(vm); cleanup: if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); if (event) qemuDomainEventQueue(driver, event); - qemuDriverUnlock(driver); return ret; } @@ -1535,12 +1547,14 @@ static char *qemudDomainGetOSType(virDomainPtr dom) { goto cleanup; } + virDomainObjLock(vm); if (!(type = strdup(vm->def->os.type))) virReportOOMError(); + virDomainObjUnlock(vm); cleanup: if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return type; } @@ -1562,11 +1576,13 @@ static unsigned long qemudDomainGetMaxMemory(virDomainPtr dom) { goto cleanup; } + virDomainObjLock(vm); ret = vm->def->mem.max_balloon; + virDomainObjUnlock(vm); cleanup: if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return ret; } @@ -1594,18 +1610,18 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } + if (qemuDomainObjBeginJob(vm) < 0) + goto cleanup; + if (newmem > vm->def->mem.max_balloon) { qemuReportError(VIR_ERR_INVALID_ARG, "%s", _("cannot set memory higher than max memory")); - goto cleanup; + goto endjob; } - if (qemuDomainObjBeginJob(vm) < 0) - goto cleanup; - if (!virDomainObjIsActive(vm) && (flags & VIR_DOMAIN_MEM_LIVE)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -1647,12 +1663,10 @@ static int qemudDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, ret = 0; endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return ret; } @@ -1676,9 +1690,12 @@ static int qemudDomainGetInfo(virDomainPtr dom, virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } + if (qemuDomainObjBeginJob(vm) < 0) + goto cleanup; + info->state = vm->state; if (!virDomainObjIsActive(vm)) { @@ -1687,7 +1704,7 @@ static int qemudDomainGetInfo(virDomainPtr dom, if (qemudGetProcessInfo(&(info->cpuTime), NULL, vm->pid, 0) < 0) { qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("cannot read cputime for domain")); - goto cleanup; + goto endjob; } } @@ -1700,8 +1717,6 @@ static int qemudDomainGetInfo(virDomainPtr dom, (vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)) { info->memory = vm->def->mem.max_balloon; } else if (!priv->jobActive) { - if (qemuDomainObjBeginJob(vm) < 0) - goto cleanup; if (!virDomainObjIsActive(vm)) err = 0; else { @@ -1709,10 +1724,6 @@ static int qemudDomainGetInfo(virDomainPtr dom, err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); qemuDomainObjExitMonitor(vm); } - if (qemuDomainObjEndJob(vm) == 0) { - vm = NULL; - goto cleanup; - } if (err < 0) goto cleanup; @@ -1731,9 +1742,10 @@ static int qemudDomainGetInfo(virDomainPtr dom, info->nrVirtCpu = vm->def->vcpus; ret = 0; +endjob: + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return ret; } @@ -2006,9 +2018,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, - vm); + qemuDomainObjEndJob(vm); + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } @@ -2021,8 +2032,8 @@ endjob: VIR_WARN0("Unable to resume guest CPUs after save failure"); } } - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + + qemuDomainObjEndJob(vm); } cleanup: @@ -2303,6 +2314,7 @@ static int qemudDomainCoreDump(virDomainPtr dom, qemuDriverLock(driver); vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); if (!vm) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -2311,11 +2323,12 @@ static int qemudDomainCoreDump(virDomainPtr dom, _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } - priv = vm->privateData; if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) goto cleanup; + priv = vm->privateData; + if (!virDomainObjIsActive(vm)) { qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); @@ -2367,20 +2380,20 @@ endjob: } } - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; - else if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + qemuDomainObjEndJob(vm); + if ((ret == 0) && (flags & VIR_DUMP_CRASH) && !vm->persistent) { + qemuDriverLock(driver); virDomainRemoveInactive(&driver->domains, vm); + qemuDriverUnlock(driver); vm = NULL; } cleanup: - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); - qemuDriverUnlock(driver); + if (vm) + virDomainObjUnref(vm); return ret; } @@ -2403,9 +2416,6 @@ static void processWatchdogEvent(void *data, void *opaque) break; } - qemuDriverLock(driver); - virDomainObjLock(wdEvent->vm); - if (qemuDomainObjBeginJobWithDriver(driver, wdEvent->vm) < 0) break; @@ -2429,10 +2439,7 @@ static void processWatchdogEvent(void *data, void *opaque) qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Resuming after dump failed")); - if (qemuDomainObjEndJob(wdEvent->vm) > 0) - virDomainObjUnlock(wdEvent->vm); - - qemuDriverUnlock(driver); + qemuDomainObjEndJob(wdEvent->vm); VIR_FREE(dumpfile); } @@ -2533,7 +2540,7 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } if (qemuDomainObjBeginJob(vm) < 0) @@ -2608,12 +2615,10 @@ qemudDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, ret = virDomainSaveConfig(driver->configDir, persistentDef); endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return ret; } @@ -2645,7 +2650,7 @@ qemudDomainPinVcpu(virDomainPtr dom, virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } if (!virDomainObjIsActive(vm)) { @@ -2690,8 +2695,7 @@ qemudDomainPinVcpu(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return ret; } @@ -2717,7 +2721,7 @@ qemudDomainGetVcpus(virDomainPtr dom, virUUIDFormat(dom->uuid, uuidstr); qemuReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; + return -1; } if (!virDomainObjIsActive(vm)) { @@ -2780,8 +2784,7 @@ qemudDomainGetVcpus(virDomainPtr dom, ret = maxinfo; cleanup: - if (vm) - virDomainObjUnlock(vm); + virDomainObjUnref(vm); return ret; } @@ -3164,9 +3167,8 @@ qemuDomainRestore(virConnectPtr conn, ret = qemuDomainSaveImageStartVM(conn, driver, vm, &fd, &header, path); - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; - else if (ret < 0 && !vm->persistent) { + qemuDomainObjEndJob(vm); + if (ret < 0 && !vm->persistent) { virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } @@ -3174,8 +3176,6 @@ qemuDomainRestore(virConnectPtr conn, cleanup: virDomainDefFree(def); VIR_FORCE_CLOSE(fd); - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -3254,10 +3254,7 @@ static char *qemudDomainDumpXML(virDomainPtr dom, qemuDomainObjEnterMonitorWithDriver(driver, vm); err = qemuMonitorGetBalloonInfo(priv->mon, &balloon); qemuDomainObjExitMonitorWithDriver(driver, vm); - if (qemuDomainObjEndJob(vm) == 0) { - vm = NULL; - goto cleanup; - } + qemuDomainObjEndJob(vm); if (err < 0) goto cleanup; if (err > 0) @@ -3269,8 +3266,6 @@ static char *qemudDomainDumpXML(virDomainPtr dom, ret = qemuDomainFormatXML(driver, vm, flags); cleanup: - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -3487,12 +3482,9 @@ qemudDomainStartWithFlags(virDomainPtr dom, unsigned int flags) (flags & VIR_DOMAIN_START_PAUSED) != 0); endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -3859,8 +3851,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom, ret = -1; endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: if (cgroup) @@ -3868,8 +3859,6 @@ cleanup: qemuCapsFree(qemuCaps); virDomainDeviceDefFree(dev); - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -3993,8 +3982,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom, ret = -1; endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: if (cgroup) @@ -4002,8 +3990,6 @@ cleanup: qemuCapsFree(qemuCaps); virDomainDeviceDefFree(dev); - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -4083,14 +4069,11 @@ static int qemudDomainDetachDevice(virDomainPtr dom, ret = -1; endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: qemuCapsFree(qemuCaps); virDomainDeviceDefFree(dev); - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -4802,12 +4785,9 @@ qemudDomainBlockStats (virDomainPtr dom, qemuDomainObjExitMonitor(vm); endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); return ret; } @@ -4906,12 +4886,9 @@ qemudDomainMemoryStats (virDomainPtr dom, "%s", _("domain is not running")); } - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); return ret; } @@ -5064,15 +5041,12 @@ qemudDomainMemoryPeek (virDomainPtr dom, ret = 0; endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: VIR_FORCE_CLOSE(fd); unlink (tmp); VIR_FREE(tmp); - if (vm) - virDomainObjUnlock(vm); return ret; } @@ -5218,16 +5192,13 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom, qemuDomainObjExitMonitor(vm); } - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); } else { ret = 0; } cleanup: VIR_FORCE_CLOSE(fd); - if (vm) - virDomainObjUnlock(vm); return ret; } @@ -6067,8 +6038,8 @@ cleanup: _("resuming after snapshot failed")); } - if (qemuDomainObjEndJob(vm) == 0) - *vmptr = NULL; + qemuDomainObjEndJob(vm); + *vmptr = NULL; return ret; } @@ -6438,9 +6409,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, vm); - vm = NULL; + qemuDomainObjEndJob(vm); + virDomainRemoveInactive(&driver->domains, vm); goto cleanup; } } @@ -6454,14 +6424,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, ret = 0; endjob: - if (vm && qemuDomainObjEndJob(vm) == 0) - vm = NULL; + if (vm) + qemuDomainObjEndJob(vm); cleanup: if (event) qemuDomainEventQueue(driver, event); - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; @@ -6675,12 +6643,9 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot, ret = qemuDomainSnapshotDiscard(driver, vm, snap); endjob: - if (qemuDomainObjEndJob(vm) == 0) - vm = NULL; + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } @@ -6727,14 +6692,9 @@ static int qemuDomainMonitorCommand(virDomainPtr domain, const char *cmd, qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorArbitraryCommand(priv->mon, cmd, result, hmp); qemuDomainObjExitMonitorWithDriver(driver, vm); - if (qemuDomainObjEndJob(vm) == 0) { - vm = NULL; - goto cleanup; - } + qemuDomainObjEndJob(vm); cleanup: - if (vm) - virDomainObjUnlock(vm); qemuDriverUnlock(driver); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 43741e1..462e6be 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -336,8 +336,8 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, qemuAuditDomainStart(vm, "migrated", false); qemuProcessStop(driver, vm, 0); if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, vm); + qemuDomainObjEndJob(vm); + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } virReportSystemError(errno, "%s", @@ -353,9 +353,10 @@ qemuMigrationPrepareTunnel(struct qemud_driver *driver, ret = 0; endjob: - if (vm && - qemuDomainObjEndJob(vm) == 0) + if (vm) { + qemuDomainObjEndJob(vm); vm = NULL; + } /* We set a fake job active which is held across * API calls until the finish() call. This prevents @@ -374,11 +375,8 @@ cleanup: virDomainDefFree(def); VIR_FORCE_CLOSE(dataFD[0]); VIR_FORCE_CLOSE(dataFD[1]); - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); - qemuDriverUnlock(driver); return ret; } @@ -530,8 +528,8 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, * should have already done that. */ if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, vm); + qemuDomainObjEndJob(vm); + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } goto endjob; @@ -544,9 +542,10 @@ qemuMigrationPrepareDirect(struct qemud_driver *driver, ret = 0; endjob: - if (vm && - qemuDomainObjEndJob(vm) == 0) + if (vm) { + qemuDomainObjEndJob(vm); vm = NULL; + } /* We set a fake job active which is held across * API calls until the finish() call. This prevents @@ -565,8 +564,6 @@ cleanup: virDomainDefFree(def); if (ret != 0) VIR_FREE(*uri_out); - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); return ret; @@ -1090,8 +1087,8 @@ int qemuMigrationPerform(struct qemud_driver *driver, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) { virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm); - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, vm); + qemuDomainObjEndJob(vm); + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } ret = 0; @@ -1112,13 +1109,12 @@ endjob: VIR_DOMAIN_EVENT_RESUMED, VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } - if (vm && - qemuDomainObjEndJob(vm) == 0) + if (vm) { + qemuDomainObjEndJob(vm); vm = NULL; + } cleanup: - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); return ret; @@ -1266,20 +1262,19 @@ qemuMigrationFinish(struct qemud_driver *driver, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); if (!vm->persistent) { - if (qemuDomainObjEndJob(vm) > 0) - virDomainRemoveInactive(&driver->domains, vm); + qemuDomainObjEndJob(vm); + virDomainRemoveInactive(&driver->domains, vm); vm = NULL; } } endjob: - if (vm && - qemuDomainObjEndJob(vm) == 0) + if (vm) { + qemuDomainObjEndJob(vm); vm = NULL; + } cleanup: - if (vm) - virDomainObjUnlock(vm); if (event) qemuDomainEventQueue(driver, event); return dom; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 48ecd5c..244b22a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -604,8 +604,8 @@ static void qemuProcessHandleMonitorDestroy(qemuMonitorPtr mon, priv = vm->privateData; if (priv->mon == mon) priv->mon = NULL; - if (virDomainObjUnref(vm) > 0) - virDomainObjUnlock(vm); + virDomainObjUnlock(vm); + virDomainObjUnref(vm); } static qemuMonitorCallbacks monitorCallbacks = { @@ -644,7 +644,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) /* Safe to ignore value since ref count was incremented above */ if (priv->mon == NULL) - ignore_value(virDomainObjUnref(vm)); + virDomainObjUnref(vm); if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0) { VIR_ERROR(_("Failed to clear security context for monitor for %s"), @@ -1940,10 +1940,6 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa priv = obj->privateData; - /* Hold an extra reference because we can't allow 'vm' to be - * deleted if qemuConnectMonitor() failed */ - virDomainObjRef(obj); - /* XXX check PID liveliness & EXE path */ if (qemuConnectMonitor(driver, obj) < 0) goto error; @@ -1975,8 +1971,7 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa if (obj->def->id >= driver->nextvmid) driver->nextvmid = obj->def->id + 1; - if (virDomainObjUnref(obj) > 0) - virDomainObjUnlock(obj); + virDomainObjUnlock(obj); qemuCapsFree(qemuCaps); return; @@ -1984,21 +1979,17 @@ qemuProcessReconnect(void *payload, const void *name ATTRIBUTE_UNUSED, void *opa error: qemuCapsFree(qemuCaps); if (!virDomainObjIsActive(obj)) { - if (virDomainObjUnref(obj) > 0) - virDomainObjUnlock(obj); + virDomainObjUnlock(obj); return; } - if (virDomainObjUnref(obj) > 0) { - /* We can't get the monitor back, so must kill the VM - * to remove danger of it ending up running twice if - * user tries to start it again later */ - qemuProcessStop(driver, obj, 0); - if (!obj->persistent) - virDomainRemoveInactive(&driver->domains, obj); - else - virDomainObjUnlock(obj); - } + /* We can't get the monitor back, so must kill the VM + * to remove danger of it ending up running twice if + * user tries to start it again later */ + qemuProcessStop(driver, obj, 0); + virDomainObjUnlock(obj); + if (!obj->persistent) + virDomainRemoveInactive(&driver->domains, obj); } /** diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 6339248..6b59b18 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -205,7 +205,7 @@ cleanup: VIR_FREE(vmx); /* any non-NULL vm here has not been shared, so unref will return 0 */ if (vm) - ignore_value(virDomainObjUnref(vm)); + virDomainObjUnref(vm); return ret; } -- 1.7.3.1 -- Thanks, Hu Tao

--- src/qemu/qemu_domain.c | 30 ++----------- src/qemu/qemu_migration.c | 2 - src/qemu/qemu_monitor.c | 109 ++++++++++++++++++++++++-------------------- src/qemu/qemu_monitor.h | 4 +- src/qemu/qemu_process.c | 32 +++++++------- 5 files changed, 81 insertions(+), 96 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3a3c953..d11dc5f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -560,9 +560,8 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - qemuMonitorLock(priv->mon); - qemuMonitorRef(priv->mon); virDomainObjUnlock(obj); + qemuMonitorLock(priv->mon); } @@ -573,18 +572,9 @@ void qemuDomainObjEnterMonitor(virDomainObjPtr obj) void qemuDomainObjExitMonitor(virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - int refs; - - refs = qemuMonitorUnref(priv->mon); - - if (refs > 0) - qemuMonitorUnlock(priv->mon); + qemuMonitorUnlock(priv->mon); virDomainObjLock(obj); - - if (refs == 0) { - priv->mon = NULL; - } } @@ -601,10 +591,8 @@ void qemuDomainObjEnterMonitorWithDriver(struct qemud_driver *driver, { qemuDomainObjPrivatePtr priv = obj->privateData; - qemuMonitorLock(priv->mon); - qemuMonitorRef(priv->mon); virDomainObjUnlock(obj); - qemuDriverUnlock(driver); + qemuMonitorLock(priv->mon); } @@ -617,19 +605,9 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - int refs; - - refs = qemuMonitorUnref(priv->mon); - - if (refs > 0) - qemuMonitorUnlock(priv->mon); - qemuDriverLock(driver); + qemuMonitorUnlock(priv->mon); virDomainObjLock(obj); - - if (refs == 0) { - priv->mon = NULL; - } } void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 462e6be..6af2e24 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -224,11 +224,9 @@ qemuMigrationWaitForCompletion(struct qemud_driver *driver, virDomainObjPtr vm) } virDomainObjUnlock(vm); - qemuDriverUnlock(driver); nanosleep(&ts, NULL); - qemuDriverLock(driver); virDomainObjLock(vm); } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 2d28f8d..98c55e7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -37,6 +37,7 @@ #include "memory.h" #include "logging.h" #include "files.h" +#include "virobject.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -44,11 +45,10 @@ #define DEBUG_RAW_IO 0 struct _qemuMonitor { + virObject obj; virMutex lock; /* also used to protect fd */ virCond notify; - int refs; - int fd; int watch; int hasSendFD; @@ -203,35 +203,61 @@ void qemuMonitorUnlock(qemuMonitorPtr mon) } -static void qemuMonitorFree(qemuMonitorPtr mon) +static void doMonitorFree(virObjectPtr obj) { + qemuMonitorPtr mon = (qemuMonitorPtr)obj; + VIR_DEBUG("mon=%p", mon); if (mon->cb && mon->cb->destroy) (mon->cb->destroy)(mon, mon->vm); - if (virCondDestroy(&mon->notify) < 0) - {} + ignore_value(virCondDestroy(&mon->notify)); virMutexDestroy(&mon->lock); VIR_FREE(mon->buffer); VIR_FREE(mon); } -int qemuMonitorRef(qemuMonitorPtr mon) +static qemuMonitorPtr qemuMonitorAlloc(void) { - mon->refs++; - return mon->refs; -} + qemuMonitorPtr mon; -int qemuMonitorUnref(qemuMonitorPtr mon) -{ - mon->refs--; + if (VIR_ALLOC(mon) < 0) { + virReportOOMError(); + return NULL; + } - if (mon->refs == 0) { - qemuMonitorUnlock(mon); - qemuMonitorFree(mon); - return 0; + if (virObjectInit(&mon->obj, doMonitorFree)) { + VIR_FREE(mon); + return NULL; + } + + if (virMutexInit(&mon->lock) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor mutex")); + VIR_FREE(mon); + return NULL; } - return mon->refs; + if (virCondInit(&mon->notify) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("cannot initialize monitor condition")); + virMutexDestroy(&mon->lock); + VIR_FREE(mon); + return NULL; + } + + mon->fd = -1; + + return mon; +} + +void qemuMonitorRef(qemuMonitorPtr mon) +{ + virObjectRef(&mon->obj); +} + +void qemuMonitorUnref(qemuMonitorPtr mon) +{ + virObjectUnref(&mon->obj); } static void @@ -239,9 +265,7 @@ qemuMonitorUnwatch(void *monitor) { qemuMonitorPtr mon = monitor; - qemuMonitorLock(mon); - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnref(mon); } static int @@ -521,7 +545,6 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* lock access to the monitor and protect fd */ qemuMonitorLock(mon); - qemuMonitorRef(mon); #if DEBUG_IO VIR_DEBUG("Monitor %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif @@ -598,13 +621,11 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); VIR_DEBUG("Triggering EOF callback error? %d", failed); (eofNotify)(mon, vm, failed); } else { - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); } } @@ -623,30 +644,13 @@ qemuMonitorOpen(virDomainObjPtr vm, return NULL; } - if (VIR_ALLOC(mon) < 0) { - virReportOOMError(); + mon = qemuMonitorAlloc(); + if (!mon) return NULL; - } - if (virMutexInit(&mon->lock) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor mutex")); - VIR_FREE(mon); - return NULL; - } - if (virCondInit(&mon->notify) < 0) { - qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize monitor condition")); - virMutexDestroy(&mon->lock); - VIR_FREE(mon); - return NULL; - } - mon->fd = -1; - mon->refs = 1; mon->vm = vm; mon->json = json; mon->cb = cb; - qemuMonitorLock(mon); switch (config->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -679,20 +683,25 @@ qemuMonitorOpen(virDomainObjPtr vm, } + /* mon will be accessed by qemuMonitorIO which is called in + * event thread, so ref it before passing it to the thread. + * + * Note: mon is unrefed in qemuMonitorUnwatch + */ + qemuMonitorRef(mon); if ((mon->watch = virEventAddHandle(mon->fd, VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_READABLE, qemuMonitorIO, mon, qemuMonitorUnwatch)) < 0) { + qemuMonitorUnref(mon); qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to register monitor events")); goto cleanup; } - qemuMonitorRef(mon); VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); - qemuMonitorUnlock(mon); return mon; @@ -703,8 +712,8 @@ cleanup: * so kill the callbacks now. */ mon->cb = NULL; - qemuMonitorUnlock(mon); qemuMonitorClose(mon); + qemuMonitorUnref(mon); return NULL; } @@ -724,8 +733,8 @@ void qemuMonitorClose(qemuMonitorPtr mon) VIR_FORCE_CLOSE(mon->fd); } - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); + qemuMonitorUnref(mon); } @@ -778,7 +787,7 @@ int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon, if ((mon)->cb && (mon)->cb->callback) \ (ret) = ((mon)->cb->callback)(mon, __VA_ARGS__); \ qemuMonitorLock(mon); \ - ignore_value(qemuMonitorUnref(mon)); \ + qemuMonitorUnref(mon); \ } while (0) int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c90219b..4498c49 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -132,8 +132,8 @@ int qemuMonitorCheckHMP(qemuMonitorPtr mon, const char *cmd); void qemuMonitorLock(qemuMonitorPtr mon); void qemuMonitorUnlock(qemuMonitorPtr mon); -int qemuMonitorRef(qemuMonitorPtr mon); -int qemuMonitorUnref(qemuMonitorPtr mon) ATTRIBUTE_RETURN_CHECK; +void qemuMonitorRef(qemuMonitorPtr mon); +void qemuMonitorUnref(qemuMonitorPtr mon); /* These APIs are for use by the internal Text/JSON monitor impl code only */ int qemuMonitorSend(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 244b22a..4b9087f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -107,7 +107,6 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); - qemuDriverLock(driver); virDomainObjLock(vm); if (!virDomainObjIsActive(vm)) { @@ -133,15 +132,17 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, qemuProcessStop(driver, vm, 0); qemuAuditDomainStop(vm, hasError ? "failed" : "shutdown"); - if (!vm->persistent) + if (!vm->persistent) { + qemuDriverLock(driver); virDomainRemoveInactive(&driver->domains, vm); - else - virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + } + + virDomainObjUnlock(vm); if (event) { qemuDomainEventQueue(driver, event); } - qemuDriverUnlock(driver); } @@ -602,8 +603,10 @@ static void qemuProcessHandleMonitorDestroy(qemuMonitorPtr mon, virDomainObjLock(vm); priv = vm->privateData; - if (priv->mon == mon) + if (priv->mon == mon) { priv->mon = NULL; + qemuMonitorUnref(mon); + } virDomainObjUnlock(vm); virDomainObjUnref(vm); } @@ -633,19 +636,11 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) goto error; } - /* Hold an extra reference because we can't allow 'vm' to be - * deleted while the monitor is active */ - virDomainObjRef(vm); - priv->mon = qemuMonitorOpen(vm, priv->monConfig, priv->monJSON, &monitorCallbacks); - /* Safe to ignore value since ref count was incremented above */ - if (priv->mon == NULL) - virDomainObjUnref(vm); - if (virSecurityManagerClearSocketLabel(driver->securityManager, vm) < 0) { VIR_ERROR(_("Failed to clear security context for monitor for %s"), vm->def->name); @@ -657,6 +652,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) goto error; } + qemuMonitorRef(priv->mon); qemuDomainObjEnterMonitorWithDriver(driver, vm); ret = qemuMonitorSetCapabilities(priv->mon); @@ -2471,8 +2467,12 @@ void qemuProcessStop(struct qemud_driver *driver, _("Failed to send SIGTERM to %s (%d)"), vm->def->name, vm->pid); - if (priv->mon) - qemuMonitorClose(priv->mon); + if (priv->mon) { + qemuMonitorPtr mon = priv->mon; + priv->mon = NULL; + qemuMonitorClose(mon); + qemuMonitorUnref(mon); + } if (priv->monConfig) { if (priv->monConfig->type == VIR_DOMAIN_CHR_TYPE_UNIX) -- 1.7.3.1 -- Thanks, Hu Tao