6:17 a.m.
From: "Daniel P. Berrange" <berrange(a)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