Fix potential events deadlock when unref'ing virConnectPtr

When the last reference to a virConnectPtr is released by
libvirtd, it was possible for a deadlock to occur in the
virDomainEventState functions. The virDomainEventStatePtr
holds a reference on virConnectPtr for each registered
callback. When removing a callback, the virUnrefConnect
function is run. If this causes the last reference on the
virConnectPtr to be released, then virReleaseConnect can
be run, which in turns calls qemudClose. This function has
a call to virDomainEventStateDeregisterConn which is intended
to remove all callbacks associated with the virConnectPtr
instance. This will try to grab a lock on virDomainEventState
but this lock is already held. Deadlock ensues

Thread 1 (Thread 0x7fcbb526a840 (LWP 23185)):

Since each callback associated with a virConnectPtr holds a
reference on virConnectPtr, it is impossible for the qemudClose
method to be invoked while any callbacks are still registered.
Thus the call to virDomainEventStateDeregisterConn must in fact
be a no-op. Thus it is possible to just remove all trace of
virDomainEventStateDeregisterConn and avoid the deadlock.

* src/conf/domain_event.c, src/conf/domain_event.h,
  src/libvirt_private.syms: Delete virDomainEventStateDeregisterConn
* src/libxl/libxl_driver.c, src/lxc/lxc_driver.c,
  src/qemu/qemu_driver.c, src/uml/uml_driver.c: Remove
  calls to virDomainEventStateDeregisterConn

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 923c58d30b94a08f517ab5188e3b799a4b2c5b25..4ecc4135028ff310f18ffc72e9e022c4cbcf3d90 100644 (file)
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -248,45 +248,6 @@ virDomainEventCallbackListRemoveID(virConnectPtr conn,
 }
 
 
-/**
- * virDomainEventCallbackListRemoveConn:
- * @conn: pointer to the connection
- * @cbList: the list
- *
- * Internal function to remove all of a given connection's callback
- * from a virDomainEventCallbackListPtr
- */
-static int
-virDomainEventCallbackListRemoveConn(virConnectPtr conn,
-                                     virDomainEventCallbackListPtr cbList)
-{
-    int old_count = cbList->count;
-    int i;
-    for (i = 0 ; i < cbList->count ; i++) {
-        if (cbList->callbacks[i]->conn == conn) {
-            virFreeCallback freecb = cbList->callbacks[i]->freecb;
-            if (freecb)
-                (*freecb)(cbList->callbacks[i]->opaque);
-            virUnrefConnect(cbList->callbacks[i]->conn);
-            VIR_FREE(cbList->callbacks[i]);
-
-            if (i < (cbList->count - 1))
-                memmove(cbList->callbacks + i,
-                        cbList->callbacks + i + 1,
-                        sizeof(*(cbList->callbacks)) *
-                                (cbList->count - (i + 1)));
-            cbList->count--;
-            i--;
-        }
-    }
-    if (cbList->count < old_count &&
-        VIR_REALLOC_N(cbList->callbacks, cbList->count) < 0) {
-        ; /* Failure to reduce memory allocation isn't fatal */
-    }
-    return 0;
-}
-
-
 static int
 virDomainEventCallbackListMarkDelete(virConnectPtr conn,
                                      virDomainEventCallbackListPtr cbList,
@@ -1607,28 +1568,6 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
 }
 
 
-/**
- * virDomainEventStateDeregisterConn:
- * @conn: connection to associate with callbacks
- * @state: domain event state
- *
- * Remove all callbacks from @state associated with the
- * connection @conn
- *
- * Returns 0 on success, -1 on error
- */
-int
-virDomainEventStateDeregisterConn(virConnectPtr conn,
-                                  virDomainEventStatePtr state)
-{
-    int ret;
-    virDomainEventStateLock(state);
-    ret = virDomainEventCallbackListRemoveConn(conn, state->callbacks);
-    virDomainEventStateUnlock(state);
-    return ret;
-}
-
-
 /**
  * virDomainEventStateEventID:
  * @conn: connection associated with the callback

diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index f7776c7b27f69a75ef43557cc19b815d72713884..d381aec010fc27cbd6d2025d52a99331cf49cb0b 100644 (file)
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -161,10 +161,6 @@ virDomainEventStateDeregisterID(virConnectPtr conn,
                                 int callbackID)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 int
-virDomainEventStateDeregisterConn(virConnectPtr conn,
-                                  virDomainEventStatePtr state)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-int
 virDomainEventStateEventID(virConnectPtr conn,
                            virDomainEventStatePtr state,
                            int callbackID)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f5c21844a4e51be17417a4663c8eb70cda89f369..6c907c4282b96b2085fae53b6c5b85d21df81e4a 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -522,7 +522,6 @@ virDomainEventRebootNewFromDom;
 virDomainEventRebootNewFromObj;
 virDomainEventStateDeregister;
 virDomainEventStateDeregisterID;
-virDomainEventStateDeregisterConn;
 virDomainEventStateEventID;
 virDomainEventStateRegister;
 virDomainEventStateRegisterID;

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 45bf1f84cabe9a7072c7b5ac25e491ff57a76935..fc90d1650cd591f76037e44121fd848cba0a3dcd 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1081,10 +1081,6 @@ libxlClose(virConnectPtr conn ATTRIBUTE_UNUSED)
 {
     libxlDriverPrivatePtr driver = conn->privateData;
 
-    libxlDriverLock(driver);
-    virDomainEventStateDeregisterConn(conn,
-                                      driver->domainEventState);
-    libxlDriverUnlock(driver);
     conn->privateData = NULL;
     return 0;
 }

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 4cccd532c0b7d7430d1ed31d06bc67469e55e30b..9aea5560c700a556c5d84042ecf70cc6e0fb40e3 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -178,8 +178,6 @@ static int lxcClose(virConnectPtr conn)
     lxc_driver_t *driver = conn->privateData;
 
     lxcDriverLock(driver);
-    virDomainEventStateDeregisterConn(conn,
-                                      driver->domainEventState);
     lxcProcessAutoDestroyRun(driver, conn);
     lxcDriverUnlock(driver);
 

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index efbc4214a97e13195d980dcae128cd6e6dd1d13e..39b27b18b01505a37e9d6037e9eead380ebb0284 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -962,8 +962,6 @@ static int qemudClose(virConnectPtr conn) {
 
     /* Get rid of callbacks registered for this conn */
     qemuDriverLock(driver);
-    virDomainEventStateDeregisterConn(conn,
-                                      driver->domainEventState);
     qemuDriverCloseCallbackRunAll(driver, conn);
     qemuDriverUnlock(driver);
 

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 8a39d734f76604c3c03ce41f84d23cf04db67bcf..65f9cbc9fbcf188dde2936deb11be2a75d3c1672 100644 (file)
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1188,8 +1188,6 @@ static int umlClose(virConnectPtr conn) {
     struct uml_driver *driver = conn->privateData;
 
     umlDriverLock(driver);
-    virDomainEventStateDeregisterConn(conn,
-                                      driver->domainEventState);
     umlProcessAutoDestroyRun(driver, conn);
     umlDriverUnlock(driver);