[libvirt] PATCH: Fix remote domain events deadlock

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

There is a deadlock in the remote driver's dispatch code for domain events.
If your callback triggers a API call that does an RPC call, then the driver
will deadlock. The fix is the same as we did in the QEMU driver. We releae
the lock & re-aquire it, with some extreme cleverness to protect against
people deleting callbacks during the course of dispatch

Daniel

commit e2e6f795dd1832227c7d5c7832596be3642fe6e4
Author: Daniel P. Berrange <berrange@xxxxxxxxxx>
Date:   Tue Jul 28 17:45:19 2009 +0100

    Fix deadlock in remote driver domain events
    
    * src/remote_internal.c: Release driver lock when dispatching events
      to callbacks

diff --git a/src/remote_internal.c b/src/remote_internal.c
index f20ed6e..a58b768 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -167,6 +167,8 @@ struct private_data {
     virDomainEventQueuePtr domainEvents;
     /* Timer for flushing domainEvents queue */
     int eventFlushTimer;
+    /* Flag if we're in process of dispatching */
+    int domainEventDispatching;
 
     /* Self-pipe to wakeup threads waiting in poll() */
     int wakeupSendFD;
@@ -6288,18 +6290,26 @@ static int remoteDomainEventDeregister (virConnectPtr conn,
 
     remoteDriverLock(priv);
 
-    if (virDomainEventCallbackListRemove(conn, priv->callbackList,
-                                         callback) < 0) {
-         error (conn, VIR_ERR_RPC, _("removing cb fron list"));
-         goto done;
-    }
-
-    if ( priv->callbackList->count == 0 ) {
-        /* Tell the server when we are the last callback deregistering */
-        if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER,
-                (xdrproc_t) xdr_void, (char *) NULL,
-                (xdrproc_t) xdr_void, (char *) NULL) == -1)
+    if (priv->domainEventDispatching) {
+        if (virDomainEventCallbackListMarkDelete(conn, priv->callbackList,
+                                                 callback) < 0) {
+            error (conn, VIR_ERR_RPC, _("marking cb for deletion"));
             goto done;
+        }
+    } else {
+        if (virDomainEventCallbackListRemove(conn, priv->callbackList,
+                                             callback) < 0) {
+            error (conn, VIR_ERR_RPC, _("removing cb from list"));
+            goto done;
+        }
+
+        if ( priv->callbackList->count == 0 ) {
+            /* Tell the server when we are the last callback deregistering */
+            if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER,
+                      (xdrproc_t) xdr_void, (char *) NULL,
+                      (xdrproc_t) xdr_void, (char *) NULL) == -1)
+                goto done;
+        }
     }
 
     rv = 0;
@@ -7309,18 +7319,54 @@ done:
     remoteDriverUnlock(priv);
 }
 
+static void remoteDomainEventDispatchFunc(virConnectPtr conn,
+                                          virDomainEventPtr event,
+                                          virConnectDomainEventCallback cb,
+                                          void *cbopaque,
+                                          void *opaque)
+{
+    struct private_data *priv = opaque;
+
+    /* Drop the lock whle dispatching, for sake of re-entrancy */
+    remoteDriverUnlock(priv);
+    virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL);
+    remoteDriverLock(priv);
+}
+
 void
 remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque)
 {
     virConnectPtr conn = opaque;
     struct private_data *priv = conn->privateData;
+    virDomainEventQueue tempQueue;
 
     remoteDriverLock(priv);
 
-    virDomainEventQueueDispatch(priv->domainEvents, priv->callbackList,
-                                virDomainEventDispatchDefaultFunc, NULL);
+    priv->domainEventDispatching = 1;
+
+    /* Copy the queue, so we're reentrant safe */
+    tempQueue.count = priv->domainEvents->count;
+    tempQueue.events = priv->domainEvents->events;
+    priv->domainEvents->count = 0;
+    priv->domainEvents->events = NULL;
+
+    virDomainEventQueueDispatch(&tempQueue, priv->callbackList,
+                                remoteDomainEventDispatchFunc, priv);
     virEventUpdateTimeout(priv->eventFlushTimer, -1);
 
+    /* Purge any deleted callbacks */
+    virDomainEventCallbackListPurgeMarked(priv->callbackList);
+
+    if ( priv->callbackList->count == 0 ) {
+        /* Tell the server when we are the last callback deregistering */
+        if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER,
+                  (xdrproc_t) xdr_void, (char *) NULL,
+                  (xdrproc_t) xdr_void, (char *) NULL) == -1)
+            VIR_WARN0("Failed to de-register events");
+    }
+
+    priv->domainEventDispatching = 0;
+
     remoteDriverUnlock(priv);
 }
 


-- 
|: 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list