Currently, push & pop from event queue (both server & client side) rely on lock from higher levels, e.g. on driver lock (qemu), private_data (remote), ...; This alone is not sufficient as not every function that interacts with this queue can/does lock, esp. in client where we have a different approach, "passing the buck". Therefore we need a separate lock just to protect event queue. For more info see: https://bugzilla.redhat.com/show_bug.cgi?id=743817 --- src/conf/domain_event.c | 65 +++++++++++++++++++++++++++++++++++++++++------ src/conf/domain_event.h | 1 + 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 3189346..6cc8168 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -547,6 +547,24 @@ virDomainEventQueuePtr virDomainEventQueueNew(void) return ret; } +static void +virDomainEventStateLock(virDomainEventStatePtr state) +{ + if (!state) + return; + + virMutexLock(&state->lock); +} + +static void +virDomainEventStateUnlock(virDomainEventStatePtr state) +{ + if (!state) + return; + + virMutexUnlock(&state->lock); +} + /** * virDomainEventStateFree: * @list: virDomainEventStatePtr to free @@ -564,6 +582,8 @@ virDomainEventStateFree(virDomainEventStatePtr state) if (state->timer != -1) virEventRemoveTimeout(state->timer); + + virMutexDestroy(&state->lock); VIR_FREE(state); } @@ -590,6 +610,13 @@ virDomainEventStateNew(virEventTimeoutCallback timeout_cb, goto error; } + if (virMutexInit(&state->lock) < 0) { + virReportSystemError(errno, "%s", + _("unable to initialize state mutex")); + VIR_FREE(state); + goto error; + } + if (VIR_ALLOC(state->callbacks) < 0) { virReportOOMError(); goto error; @@ -1161,18 +1188,26 @@ void virDomainEventStateQueue(virDomainEventStatePtr state, virDomainEventPtr event) { + int count; + if (state->timer < 0) { virDomainEventFree(event); return; } + virDomainEventStateLock(state); + if (virDomainEventQueuePush(state->queue, event) < 0) { VIR_DEBUG("Error adding event to queue"); virDomainEventFree(event); } - if (state->queue->count == 1) + count = state->queue->count; + virDomainEventStateUnlock(state); + + if (count == 1) virEventUpdateTimeout(state->timer, 0); + } void @@ -1182,6 +1217,7 @@ virDomainEventStateFlush(virDomainEventStatePtr state, { virDomainEventQueue tempQueue; + virDomainEventStateLock(state); state->isDispatching = true; /* Copy the queue, so we're reentrant safe when dispatchFunc drops the @@ -1190,6 +1226,7 @@ virDomainEventStateFlush(virDomainEventStatePtr state, tempQueue.events = state->queue->events; state->queue->count = 0; state->queue->events = NULL; + virDomainEventStateUnlock(state); virEventUpdateTimeout(state->timer, -1); virDomainEventQueueDispatch(&tempQueue, @@ -1198,9 +1235,11 @@ virDomainEventStateFlush(virDomainEventStatePtr state, opaque); /* Purge any deleted callbacks */ + virDomainEventStateLock(state); virDomainEventCallbackListPurgeMarked(state->callbacks); state->isDispatching = false; + virDomainEventStateUnlock(state); } int @@ -1208,11 +1247,16 @@ virDomainEventStateDeregister(virConnectPtr conn, virDomainEventStatePtr state, virConnectDomainEventCallback callback) { + int ret; + + virDomainEventStateLock(state); if (state->isDispatching) - return virDomainEventCallbackListMarkDelete(conn, - state->callbacks, callback); + ret = virDomainEventCallbackListMarkDelete(conn, + state->callbacks, callback); else - return virDomainEventCallbackListRemove(conn, state->callbacks, callback); + ret = virDomainEventCallbackListRemove(conn, state->callbacks, callback); + virDomainEventStateUnlock(state); + return ret; } int @@ -1220,10 +1264,15 @@ virDomainEventStateDeregisterAny(virConnectPtr conn, virDomainEventStatePtr state, int callbackID) { + int ret; + + virDomainEventStateLock(state); if (state->isDispatching) - return virDomainEventCallbackListMarkDeleteID(conn, - state->callbacks, callbackID); + ret = virDomainEventCallbackListMarkDeleteID(conn, + state->callbacks, callbackID); else - return virDomainEventCallbackListRemoveID(conn, - state->callbacks, callbackID); + ret = virDomainEventCallbackListRemoveID(conn, + state->callbacks, callbackID); + virDomainEventStateUnlock(state); + return ret; } diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index b06be16..08930ed 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -61,6 +61,7 @@ struct _virDomainEventState { int timer; /* Flag if we're in process of dispatching */ bool isDispatching; + virMutex lock; }; typedef struct _virDomainEventState virDomainEventState; typedef virDomainEventState *virDomainEventStatePtr; -- 1.7.3.4