From: "Daniel P. Berrange" <berrange@redhat.com> 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. 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 --- src/conf/domain_event.c | 61 ---------------------------------------------- src/conf/domain_event.h | 4 --- src/libvirt_private.syms | 1 - src/libxl/libxl_driver.c | 4 --- src/lxc/lxc_driver.c | 2 -- src/qemu/qemu_driver.c | 2 -- src/uml/uml_driver.c | 2 -- 7 files changed, 76 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 923c58d..4ecc413 100644 --- 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, @@ -1608,28 +1569,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 * @state: domain event state diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index f7776c7..d381aec 100644 --- 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 f5c2184..6c907c4 100644 --- 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 45bf1f8..fc90d16 100644 --- 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 4cccd53..9aea556 100644 --- 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 efbc421..39b27b1 100644 --- 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 8a39d73..65f9cbc 100644 --- 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); -- 1.7.10.1