This way we get reference counting and we can get rid of locking function. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/bhyve/bhyve_driver.c | 2 +- src/conf/object_event.c | 99 +++++++++++++++----------------------- src/conf/object_event.h | 1 - src/libvirt_private.syms | 1 - src/libxl/libxl_driver.c | 2 +- src/lxc/lxc_driver.c | 2 +- src/network/bridge_driver.c | 2 +- src/node_device/node_device_udev.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/remote/remote_driver.c | 2 +- src/storage/storage_driver.c | 2 +- src/test/test_driver.c | 2 +- src/uml/uml_driver.c | 2 +- src/vbox/vbox_common.c | 2 +- src/vz/vz_driver.c | 2 +- src/xen/xen_driver.c | 2 +- 16 files changed, 52 insertions(+), 75 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 49b9e1a60626..04be78b675f7 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -1198,7 +1198,7 @@ bhyveStateCleanup(void) virObjectUnref(bhyve_driver->xmlopt); virSysinfoDefFree(bhyve_driver->hostsysinfo); virObjectUnref(bhyve_driver->closeCallbacks); - virObjectEventStateFree(bhyve_driver->domainEventState); + virObjectUnref(bhyve_driver->domainEventState); virMutexDestroy(&bhyve_driver->lock); VIR_FREE(bhyve_driver); diff --git a/src/conf/object_event.c b/src/conf/object_event.c index e5af4be68a7e..b859835b47a1 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -32,6 +32,7 @@ #include "datatypes.h" #include "viralloc.h" #include "virerror.h" +#include "virobject.h" #include "virstring.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -71,6 +72,7 @@ typedef struct _virObjectEventQueue virObjectEventQueue; typedef virObjectEventQueue *virObjectEventQueuePtr; struct _virObjectEventState { + virObjectLockable parent; /* The list of domain event callbacks */ virObjectEventCallbackListPtr callbacks; /* The queue of object events */ @@ -79,22 +81,31 @@ struct _virObjectEventState { int timer; /* Flag if we're in process of dispatching */ bool isDispatching; - virMutex lock; }; static virClassPtr virObjectEventClass; +static virClassPtr virObjectEventStateClass; static void virObjectEventDispose(void *obj); +static void virObjectEventStateDispose(void *obj); static int virObjectEventOnceInit(void) { + if (!(virObjectEventStateClass = + virClassNew(virClassForObjectLockable(), + "virObjectEventState", + sizeof(virObjectEventState), + virObjectEventStateDispose))) + return -1; + if (!(virObjectEventClass = virClassNew(virClassForObject(), "virObjectEvent", sizeof(virObjectEvent), virObjectEventDispose))) return -1; + return 0; } @@ -504,51 +515,23 @@ virObjectEventQueueNew(void) /** - * virObjectEventStateLock: - * @state: the event state object - * - * Lock event state before calling functions from object_event_private.h. - */ -static void -virObjectEventStateLock(virObjectEventStatePtr state) -{ - virMutexLock(&state->lock); -} - - -/** - * virObjectEventStateUnlock: - * @state: the event state object - * - * Unlock event state after calling functions from object_event_private.h. - */ -static void -virObjectEventStateUnlock(virObjectEventStatePtr state) -{ - virMutexUnlock(&state->lock); -} - - -/** - * virObjectEventStateFree: + * virObjectEventStateDispose: * @list: virObjectEventStatePtr to free * * Free a virObjectEventStatePtr and its members, and unregister the timer. */ -void -virObjectEventStateFree(virObjectEventStatePtr state) +static void +virObjectEventStateDispose(void *obj) { - if (!state) - return; + virObjectEventStatePtr state = obj; + + VIR_DEBUG("obj=%p", state); virObjectEventCallbackListFree(state->callbacks); virObjectEventQueueFree(state->queue); if (state->timer != -1) virEventRemoveTimeout(state->timer); - - virMutexDestroy(&state->lock); - VIR_FREE(state); } @@ -583,15 +566,11 @@ virObjectEventStateNew(void) { virObjectEventStatePtr state = NULL; - if (VIR_ALLOC(state) < 0) - goto error; + if (virObjectEventInitialize() < 0) + return NULL; - if (virMutexInit(&state->lock) < 0) { - virReportSystemError(errno, "%s", - _("unable to initialize state mutex")); - VIR_FREE(state); - goto error; - } + if (!(state = virObjectLockableNew(virObjectEventStateClass))) + return NULL; if (VIR_ALLOC(state->callbacks) < 0) goto error; @@ -604,7 +583,7 @@ virObjectEventStateNew(void) return state; error: - virObjectEventStateFree(state); + virObjectUnref(state); return NULL; } @@ -727,9 +706,9 @@ virObjectEventStateDispatchCallbacks(virObjectEventStatePtr state, continue; /* Drop the lock whle dispatching, for sake of re-entrancy */ - virObjectEventStateUnlock(state); + virObjectUnlock(state); event->dispatch(cb->conn, event, cb->cb, cb->opaque); - virObjectEventStateLock(state); + virObjectLock(state); } } @@ -773,7 +752,7 @@ virObjectEventStateQueueRemote(virObjectEventStatePtr state, return; } - virObjectEventStateLock(state); + virObjectLock(state); event->remoteID = remoteID; if (virObjectEventQueuePush(state->queue, event) < 0) { @@ -783,7 +762,7 @@ virObjectEventStateQueueRemote(virObjectEventStatePtr state, if (state->queue->count == 1) virEventUpdateTimeout(state->timer, 0); - virObjectEventStateUnlock(state); + virObjectUnlock(state); } @@ -809,7 +788,7 @@ virObjectEventStateFlush(virObjectEventStatePtr state) { virObjectEventQueue tempQueue; - virObjectEventStateLock(state); + virObjectLock(state); state->isDispatching = true; /* Copy the queue, so we're reentrant safe when dispatchFunc drops the @@ -829,7 +808,7 @@ virObjectEventStateFlush(virObjectEventStatePtr state) virObjectEventCallbackListPurgeMarked(state->callbacks); state->isDispatching = false; - virObjectEventStateUnlock(state); + virObjectUnlock(state); } @@ -884,7 +863,7 @@ virObjectEventStateRegisterID(virConnectPtr conn, { int ret = -1; - virObjectEventStateLock(state); + virObjectLock(state); if ((state->callbacks->count == 0) && (state->timer == -1) && @@ -911,7 +890,7 @@ virObjectEventStateRegisterID(virConnectPtr conn, } cleanup: - virObjectEventStateUnlock(state); + virObjectUnlock(state); return ret; } @@ -934,7 +913,7 @@ virObjectEventStateDeregisterID(virConnectPtr conn, { int ret; - virObjectEventStateLock(state); + virObjectLock(state); if (state->isDispatching) ret = virObjectEventCallbackListMarkDeleteID(conn, state->callbacks, @@ -950,7 +929,7 @@ virObjectEventStateDeregisterID(virConnectPtr conn, virObjectEventQueueClear(state->queue); } - virObjectEventStateUnlock(state); + virObjectUnlock(state); return ret; } @@ -978,11 +957,11 @@ virObjectEventStateCallbackID(virConnectPtr conn, { int ret = -1; - virObjectEventStateLock(state); + virObjectLock(state); ret = virObjectEventCallbackLookup(conn, state->callbacks, NULL, klass, eventID, callback, true, remoteID); - virObjectEventStateUnlock(state); + virObjectUnlock(state); if (ret < 0) virReportError(VIR_ERR_INVALID_ARG, @@ -1016,7 +995,7 @@ virObjectEventStateEventID(virConnectPtr conn, size_t i; virObjectEventCallbackListPtr cbList = state->callbacks; - virObjectEventStateLock(state); + virObjectLock(state); for (i = 0; i < cbList->count; i++) { virObjectEventCallbackPtr cb = cbList->callbacks[i]; @@ -1030,7 +1009,7 @@ virObjectEventStateEventID(virConnectPtr conn, break; } } - virObjectEventStateUnlock(state); + virObjectUnlock(state); if (ret < 0) virReportError(VIR_ERR_INVALID_ARG, @@ -1060,7 +1039,7 @@ virObjectEventStateSetRemote(virConnectPtr conn, { size_t i; - virObjectEventStateLock(state); + virObjectLock(state); for (i = 0; i < state->callbacks->count; i++) { virObjectEventCallbackPtr cb = state->callbacks->callbacks[i]; @@ -1072,5 +1051,5 @@ virObjectEventStateSetRemote(virConnectPtr conn, break; } } - virObjectEventStateUnlock(state); + virObjectUnlock(state); } diff --git a/src/conf/object_event.h b/src/conf/object_event.h index b0201ddd5a12..7a9995e122e7 100644 --- a/src/conf/object_event.h +++ b/src/conf/object_event.h @@ -40,7 +40,6 @@ typedef struct _virObjectEventState virObjectEventState; typedef virObjectEventState *virObjectEventStatePtr; -void virObjectEventStateFree(virObjectEventStatePtr state); virObjectEventStatePtr virObjectEventStateNew(void); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b88e903744ec..d92d3d865307 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -818,7 +818,6 @@ virNWFilterVarValueGetSimple; # conf/object_event.h virObjectEventStateDeregisterID; virObjectEventStateEventID; -virObjectEventStateFree; virObjectEventStateNew; virObjectEventStateQueue; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index b66cb1f7ef35..89afbbd454a1 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -479,7 +479,7 @@ libxlStateCleanup(void) virObjectUnref(libxl_driver->migrationPorts); virLockManagerPluginUnref(libxl_driver->lockManager); - virObjectEventStateFree(libxl_driver->domainEventState); + virObjectUnref(libxl_driver->domainEventState); virSysinfoDefFree(libxl_driver->hostsysinfo); virMutexDestroy(&libxl_driver->lock); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index a9e664cfb538..cf30a6638b6f 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1781,7 +1781,7 @@ static int lxcStateCleanup(void) virNWFilterUnRegisterCallbackDriver(&lxcCallbackDriver); virObjectUnref(lxc_driver->domains); - virObjectEventStateFree(lxc_driver->domainEventState); + virObjectUnref(lxc_driver->domainEventState); virObjectUnref(lxc_driver->closeCallbacks); diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 72687dc227a5..b2af48272541 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -777,7 +777,7 @@ networkStateCleanup(void) if (!network_driver) return -1; - virObjectEventStateFree(network_driver->networkEventState); + virObjectUnref(network_driver->networkEventState); /* free inactive networks */ virObjectUnref(network_driver->networks); diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 520269fbe94c..4b813127cb13 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1269,7 +1269,7 @@ static int nodeStateCleanup(void) nodeDeviceLock(); - virObjectEventStateFree(driver->nodeDeviceEventState); + virObjectUnref(driver->nodeDeviceEventState); priv = driver->privateData; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 29a7e3fae5e2..5c4ed1467728 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1094,7 +1094,7 @@ qemuStateCleanup(void) ebtablesContextFree(qemu_driver->ebtables); /* Free domain callback list */ - virObjectEventStateFree(qemu_driver->domainEventState); + virObjectUnref(qemu_driver->domainEventState); virLockManagerPluginUnref(qemu_driver->lockManager); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f6c6940095ac..a3cd7cd63223 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1343,7 +1343,7 @@ doRemoteClose(virConnectPtr conn, struct private_data *priv) /* See comment for remoteType. */ VIR_FREE(priv->type); - virObjectEventStateFree(priv->eventState); + virObjectUnref(priv->eventState); priv->eventState = NULL; return ret; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6f1e3727d6e8..4f990f4c4827 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -347,7 +347,7 @@ storageStateCleanup(void) storageDriverLock(); - virObjectEventStateFree(driver->storageEventState); + virObjectUnref(driver->storageEventState); /* free inactive pools */ virStoragePoolObjListFree(&driver->pools); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a382d89a310a..dd28dc28b219 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -153,7 +153,7 @@ testDriverFree(testDriverPtr driver) virObjectUnref(driver->networks); virInterfaceObjListFree(&driver->ifaces); virStoragePoolObjListFree(&driver->pools); - virObjectEventStateFree(driver->eventState); + virObjectUnref(driver->eventState); virMutexUnlock(&driver->lock); virMutexDestroy(&driver->lock); diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 95acb2209693..768ce5295219 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -685,7 +685,7 @@ umlStateCleanup(void) virObjectUnref(uml_driver->domains); - virObjectEventStateFree(uml_driver->domainEventState); + virObjectUnref(uml_driver->domainEventState); VIR_FREE(uml_driver->logDir); VIR_FREE(uml_driver->configDir); diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1472639a0daf..ab1a4c46f196 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -387,7 +387,7 @@ static void vboxUninitialize(vboxGlobalData *data) virObjectUnref(data->caps); virObjectUnref(data->xmlopt); if (gVBoxAPI.domainEventCallbacks) - virObjectEventStateFree(data->domainEvents); + virObjectUnref(data->domainEvents); VIR_FREE(data); } diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index c8841e47a002..b7c37bb964ed 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -171,7 +171,7 @@ static void vzDriverDispose(void * obj) virObjectUnref(driver->domains); virObjectUnref(driver->caps); virObjectUnref(driver->xmlopt); - virObjectEventStateFree(driver->domainEventState); + virObjectUnref(driver->domainEventState); virSysinfoDefFree(driver->hostsysinfo); } diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 8b41974eb4a1..165f37c20db9 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -571,7 +571,7 @@ xenUnifiedConnectClose(virConnectPtr conn) virObjectUnref(priv->caps); virObjectUnref(priv->xmlopt); - virObjectEventStateFree(priv->domainEvents); + virObjectUnref(priv->domainEvents); #if WITH_XEN_INOTIFY if (priv->opened[XEN_UNIFIED_INOTIFY_OFFSET]) -- 2.10.1

There should be one more reference because it is being kept in the list of callbacks as an opaque. We also unref it properly using virObjectFreeCallback. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/object_event.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/conf/object_event.c b/src/conf/object_event.c index b859835b47a1..5994c2574d6f 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -870,12 +870,14 @@ virObjectEventStateRegisterID(virConnectPtr conn, (state->timer = virEventAddTimeout(-1, virObjectEventTimer, state, - NULL)) < 0) { + virObjectFreeCallback)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("could not initialize domain event timer")); goto cleanup; } + virObjectRef(state); + ret = virObjectEventCallbackListAddID(conn, state->callbacks, key, filter, filter_opaque, klass, eventID, -- 2.10.1

There is a repeating pattern of code that removes the timer if it's not needed. So let's move it to a new function. We'll also use it later. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/object_event.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/conf/object_event.c b/src/conf/object_event.c index 5994c2574d6f..b71960f3399d 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -784,6 +784,25 @@ virObjectEventStateQueue(virObjectEventStatePtr state, static void +virObjectEventStateCleanupTimer(virObjectEventStatePtr state, bool clear_queue) +{ + /* There are still some callbacks, keep the timer. */ + if (state->callbacks->count) + return; + + /* The timer is not registered, nothing to do. */ + if (state->timer == -1) + return; + + virEventRemoveTimeout(state->timer); + state->timer = -1; + + if (clear_queue) + virObjectEventQueueClear(state->queue); +} + + +static void virObjectEventStateFlush(virObjectEventStatePtr state) { virObjectEventQueue tempQueue; @@ -884,12 +903,8 @@ virObjectEventStateRegisterID(virConnectPtr conn, cb, opaque, freecb, legacy, callbackID, serverFilter); - if (ret == -1 && - state->callbacks->count == 0 && - state->timer != -1) { - virEventRemoveTimeout(state->timer); - state->timer = -1; - } + if (ret < 0) + virObjectEventStateCleanupTimer(state, false); cleanup: virObjectUnlock(state); @@ -924,12 +939,7 @@ virObjectEventStateDeregisterID(virConnectPtr conn, ret = virObjectEventCallbackListRemoveID(conn, state->callbacks, callbackID); - if (state->callbacks->count == 0 && - state->timer != -1) { - virEventRemoveTimeout(state->timer); - state->timer = -1; - virObjectEventQueueClear(state->queue); - } + virObjectEventStateCleanupTimer(state, true); virObjectUnlock(state); return ret; -- 2.10.1

To go with the Deregister version as well. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/libvirt_private.syms | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d92d3d865307..fd63f99b568c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -820,6 +820,7 @@ virObjectEventStateDeregisterID; virObjectEventStateEventID; virObjectEventStateNew; virObjectEventStateQueue; +virObjectEventStateRegisterID; # conf/secret_conf.h -- 2.10.1