[libvirt] [PATCH] events: Propose a separate lock for event queue

[Michal Privoznik]

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