From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
The Xen & VBox drivers deal with callbacks & dispatching of
events directly. All the other drivers use a timer to dispatch
events from a clean stack state, rather than deep inside the
drivers. Convert Xen & VBox over to virDomainEventState so
that they match behaviour of other drivers
* src/conf/domain_event.c: Return count of remaining
callbacks when unregistering event callback
* src/vbox/vbox_tmpl.c, src/xen/xen_driver.c,
src/xen/xen_driver.h: Convert to virDomainEventState
---
src/conf/domain_event.c | 58 ++++++++++++++++--
src/conf/domain_event.h | 6 +-
src/vbox/vbox_tmpl.c | 146 +++++++++++++++++++++++++----------------------
src/xen/xen_driver.c | 68 ++++++++++------------
src/xen/xen_driver.h | 4 +-
5 files changed, 164 insertions(+), 118 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 614ab97..0e481cf 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -133,6 +133,7 @@ virDomainEventCallbackListRemove(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
virConnectDomainEventCallback callback)
{
+ int ret = 0;
int i;
for (i = 0 ; i < cbList->count ; i++) {
if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) &&
@@ -156,7 +157,11 @@ virDomainEventCallbackListRemove(virConnectPtr conn,
}
cbList->count--;
- return 0;
+ for (i = 0 ; i < cbList->count ; i++) {
+ if (!cbList->callbacks[i]->deleted)
+ ret++;
+ }
+ return ret;
}
}
@@ -179,6 +184,7 @@ virDomainEventCallbackListRemoveID(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
int callbackID)
{
+ int ret = 0;
int i;
for (i = 0 ; i < cbList->count ; i++) {
if (cbList->callbacks[i]->callbackID == callbackID &&
@@ -201,7 +207,11 @@ virDomainEventCallbackListRemoveID(virConnectPtr conn,
}
cbList->count--;
- return 0;
+ for (i = 0 ; i < cbList->count ; i++) {
+ if (!cbList->callbacks[i]->deleted)
+ ret++;
+ }
+ return ret;
}
}
@@ -254,13 +264,18 @@ int virDomainEventCallbackListMarkDelete(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
virConnectDomainEventCallback callback)
{
+ int ret = 0;
int i;
for (i = 0 ; i < cbList->count ; i++) {
if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) &&
cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE &&
cbList->callbacks[i]->conn == conn) {
cbList->callbacks[i]->deleted = 1;
- return 0;
+ for (i = 0 ; i < cbList->count ; i++) {
+ if (!cbList->callbacks[i]->deleted)
+ ret++;
+ }
+ return ret;
}
}
@@ -274,12 +289,17 @@ int virDomainEventCallbackListMarkDeleteID(virConnectPtr conn,
virDomainEventCallbackListPtr cbList,
int callbackID)
{
+ int ret = 0;
int i;
for (i = 0 ; i < cbList->count ; i++) {
if (cbList->callbacks[i]->callbackID == callbackID &&
cbList->callbacks[i]->conn == conn) {
cbList->callbacks[i]->deleted = 1;
- return 0;
+ for (i = 0 ; i < cbList->count ; i++) {
+ if (!cbList->callbacks[i]->deleted)
+ ret++;
+ }
+ return ret;
}
}
@@ -1307,6 +1327,18 @@ virDomainEventStateFlush(virDomainEventStatePtr state,
virDomainEventStateUnlock(state);
}
+
+/**
+ * virDomainEventStateDeregister:
+ * @state: domain event state
+ * @conn: connection to associate with callback
+ * @callback: function to remove from event
+ *
+ * Unregister the function @callback with connection @conn,
+ * from @state, for lifecycle events.
+ *
+ * Returns: the number of lifecycle callbacks still registered, or -1 on error
+ */
int
virDomainEventStateDeregister(virConnectPtr conn,
virDomainEventStatePtr state,
@@ -1324,10 +1356,22 @@ virDomainEventStateDeregister(virConnectPtr conn,
return ret;
}
+
+/**
+ * virDomainEventStateDeregisterID:
+ * @state: domain event state
+ * @conn: connection to associate with callback
+ * @callbackID: ID of the function to remove from event
+ *
+ * Unregister the function @callbackID with connection @conn,
+ * from @state, for lifecycle events.
+ *
+ * Returns: the number of lifecycle callbacks still registered, or -1 on error
+ */
int
-virDomainEventStateDeregisterAny(virConnectPtr conn,
- virDomainEventStatePtr state,
- int callbackID)
+virDomainEventStateDeregisterID(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ int callbackID)
{
int ret;
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 3ba418e..10a55b0 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -242,9 +242,9 @@ virDomainEventStateDeregister(virConnectPtr conn,
virConnectDomainEventCallback callback)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int
-virDomainEventStateDeregisterAny(virConnectPtr conn,
- virDomainEventStatePtr state,
- int callbackID)
+virDomainEventStateDeregisterID(virConnectPtr conn,
+ virDomainEventStatePtr state,
+ int callbackID)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
#endif
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 9b74a7b..7f13f90 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -188,11 +188,9 @@ typedef struct {
#else /* !(VBOX_API_VERSION == 2002) */
- /* An array of callbacks */
- virDomainEventCallbackListPtr domainEventCallbacks;
-
+ /* Async event handling */
+ virDomainEventStatePtr domainEvents;
int fdWatch;
- int domainEventDispatching;
# if VBOX_API_VERSION <= 3002
/* IVirtualBoxCallback is used in VirtualBox 3.x only */
@@ -966,11 +964,52 @@ static void vboxUninitialize(vboxGlobalData *data) {
#if VBOX_API_VERSION == 2002
/* No domainEventCallbacks in 2.2.* version */
#else /* !(VBOX_API_VERSION == 2002) */
- VIR_FREE(data->domainEventCallbacks);
+ virDomainEventStateFree(data->domainEvents);
#endif /* !(VBOX_API_VERSION == 2002) */
VIR_FREE(data);
}
+
+#if VBOX_API_VERSION == 2002
+ /* No domainEventCallbacks in 2.2.* version */
+#else /* !(VBOX_API_VERSION == 2002) */
+
+static void
+vboxDomainEventDispatchFunc(virConnectPtr conn,
+ virDomainEventPtr event,
+ virConnectDomainEventGenericCallback cb,
+ void *cbopaque,
+ void *opaque)
+{
+ vboxGlobalData *data = opaque;
+
+ /*
+ * Release the lock while the callback is running so that
+ * we're re-entrant safe for callback work - the callback
+ * may want to invoke other virt functions & we have already
+ * protected the one piece of state we have - the callback
+ * list
+ */
+ vboxDriverUnlock(data);
+ virDomainEventDispatchDefaultFunc(conn, event, cb, cbopaque, NULL);
+ vboxDriverLock(data);
+}
+
+
+static void vboxDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque)
+{
+ virConnectPtr conn = opaque;
+ vboxGlobalData *data = conn->privateData;
+
+ vboxDriverLock(data);
+ virDomainEventStateFlush(data->domainEvents,
+ vboxDomainEventDispatchFunc,
+ data);
+ vboxDriverUnlock(data);
+}
+#endif /* !(VBOX_API_VERSION == 2002) */
+
+
static virDrvOpenStatus vboxOpen(virConnectPtr conn,
virConnectAuthPtr auth ATTRIBUTE_UNUSED,
unsigned int flags)
@@ -1035,8 +1074,11 @@ static virDrvOpenStatus vboxOpen(virConnectPtr conn,
#else /* !(VBOX_API_VERSION == 2002) */
- if (VIR_ALLOC(data->domainEventCallbacks) < 0) {
- virReportOOMError();
+ if (!(data->domainEvents = virDomainEventStateNew(vboxDomainEventFlush,
+ data,
+ NULL,
+ true))) {
+ vboxUninitialize(data);
return VIR_DRV_OPEN_ERROR;
}
@@ -6770,7 +6812,6 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED,
int event = 0;
int detail = 0;
- g_pVBoxGlobalData->domainEventDispatching = 1;
vboxDriverLock(g_pVBoxGlobalData);
VIR_DEBUG("IVirtualBoxCallback: %p, State: %d", pThis, state);
@@ -6818,20 +6859,12 @@ vboxCallbackOnMachineStateChange(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED,
ev = virDomainEventNewFromDom(dom, event, detail);
- if (ev) {
- virDomainEventDispatch(ev,
- g_pVBoxGlobalData->domainEventCallbacks,
- virDomainEventDispatchDefaultFunc,
- NULL);
- virDomainEventFree(ev);
- }
+ if (ev)
+ virDomainEventStateQueue(g_pVBoxGlobalData->domainEvents, ev);
}
}
- virDomainEventCallbackListPurgeMarked(g_pVBoxGlobalData->domainEventCallbacks);
-
vboxDriverUnlock(g_pVBoxGlobalData);
- g_pVBoxGlobalData->domainEventDispatching = 0;
return NS_OK;
}
@@ -6898,7 +6931,6 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED,
int event = 0;
int detail = 0;
- g_pVBoxGlobalData->domainEventDispatching = 1;
vboxDriverLock(g_pVBoxGlobalData);
VIR_DEBUG("IVirtualBoxCallback: %p, registered: %s", pThis, registered ? "true" : "false");
@@ -6931,20 +6963,12 @@ vboxCallbackOnMachineRegistered(IVirtualBoxCallback *pThis ATTRIBUTE_UNUSED,
ev = virDomainEventNewFromDom(dom, event, detail);
- if (ev) {
- virDomainEventDispatch(ev,
- g_pVBoxGlobalData->domainEventCallbacks,
- virDomainEventDispatchDefaultFunc,
- NULL);
- virDomainEventFree(ev);
- }
+ if (ev)
+ virDomainEventStateQueue(g_pVBoxGlobalData->domainEvents, ev);
}
}
- virDomainEventCallbackListPurgeMarked(g_pVBoxGlobalData->domainEventCallbacks);
-
vboxDriverUnlock(g_pVBoxGlobalData);
- g_pVBoxGlobalData->domainEventDispatching = 0;
return NS_OK;
}
@@ -7164,11 +7188,11 @@ static int vboxDomainEventRegister (virConnectPtr conn,
* later you can iterate over them
*/
- ret = virDomainEventCallbackListAdd(conn, data->domainEventCallbacks,
+ ret = virDomainEventCallbackListAdd(conn, data->domainEvents->callbacks,
callback, opaque, freecb);
VIR_DEBUG("virDomainEventCallbackListAdd (ret = %d) ( conn: %p, "
- "data->domainEventCallbacks: %p, callback: %p, opaque: %p, "
- "freecb: %p )", ret, conn, data->domainEventCallbacks, callback,
+ "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, "
+ "freecb: %p )", ret, conn, data->domainEvents->callbacks, callback,
opaque, freecb);
}
}
@@ -7188,31 +7212,23 @@ static int vboxDomainEventRegister (virConnectPtr conn,
static int vboxDomainEventDeregister (virConnectPtr conn,
virConnectDomainEventCallback callback) {
VBOX_OBJECT_CHECK(conn, int, -1);
+ int cnt;
/* Locking has to be there as callbacks are not
* really fully thread safe
*/
vboxDriverLock(data);
- if (data->domainEventDispatching)
- ret = virDomainEventCallbackListMarkDelete(conn, data->domainEventCallbacks,
- callback);
- else
- ret = virDomainEventCallbackListRemove(conn, data->domainEventCallbacks,
- callback);
+ cnt = virDomainEventStateDeregister(conn, data->domainEvents,
+ callback);
- if (data->vboxCallback) {
- /* check count here of how many times register was called
- * and only on the last de-register do the un-register call
- */
- if (data->domainEventCallbacks && virDomainEventCallbackListCount(data->domainEventCallbacks) == 0) {
- data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
- VBOX_RELEASE(data->vboxCallback);
+ if (data->vboxCallback && cnt == 0) {
+ data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
+ VBOX_RELEASE(data->vboxCallback);
- /* Remove the Event file handle on which we are listening as well */
- virEventRemoveHandle(data->fdWatch);
- data->fdWatch = -1;
- }
+ /* Remove the Event file handle on which we are listening as well */
+ virEventRemoveHandle(data->fdWatch);
+ data->fdWatch = -1;
}
vboxDriverUnlock(data);
@@ -7264,12 +7280,12 @@ static int vboxDomainEventRegisterAny(virConnectPtr conn,
* later you can iterate over them
*/
- ret = virDomainEventCallbackListAddID(conn, data->domainEventCallbacks,
+ ret = virDomainEventCallbackListAddID(conn, data->domainEvents->callbacks,
dom, eventID,
callback, opaque, freecb);
VIR_DEBUG("virDomainEventCallbackListAddID (ret = %d) ( conn: %p, "
- "data->domainEventCallbacks: %p, callback: %p, opaque: %p, "
- "freecb: %p )", ret, conn, data->domainEventCallbacks, callback,
+ "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, "
+ "freecb: %p )", ret, conn, data->domainEvents->callbacks, callback,
opaque, freecb);
}
}
@@ -7289,31 +7305,23 @@ static int vboxDomainEventRegisterAny(virConnectPtr conn,
static int vboxDomainEventDeregisterAny(virConnectPtr conn,
int callbackID) {
VBOX_OBJECT_CHECK(conn, int, -1);
+ int cnt;
/* Locking has to be there as callbacks are not
* really fully thread safe
*/
vboxDriverLock(data);
- if (data->domainEventDispatching)
- ret = virDomainEventCallbackListMarkDeleteID(conn, data->domainEventCallbacks,
- callbackID);
- else
- ret = virDomainEventCallbackListRemoveID(conn, data->domainEventCallbacks,
- callbackID);
+ cnt = virDomainEventStateDeregisterAny(conn, data->domainEvents,
+ callbackID);
- if (data->vboxCallback) {
- /* check count here of how many times register was called
- * and only on the last de-register do the un-register call
- */
- if (data->domainEventCallbacks && virDomainEventCallbackListCount(data->domainEventCallbacks) == 0) {
- data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
- VBOX_RELEASE(data->vboxCallback);
+ if (data->vboxCallback && cnt == 0) {
+ data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
+ VBOX_RELEASE(data->vboxCallback);
- /* Remove the Event file handle on which we are listening as well */
- virEventRemoveHandle(data->fdWatch);
- data->fdWatch = -1;
- }
+ /* Remove the Event file handle on which we are listening as well */
+ virEventRemoveHandle(data->fdWatch);
+ data->fdWatch = -1;
}
vboxDriverUnlock(data);
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index d394ff7..8c47ad5 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -63,6 +63,8 @@ static int
xenUnifiedDomainGetVcpus (virDomainPtr dom,
virVcpuInfoPtr info, int maxinfo,
unsigned char *cpumaps, int maplen);
+static void xenDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque);
+
/* The five Xen drivers below us. */
static struct xenUnifiedDriver const * const drivers[XEN_UNIFIED_NR_DRIVERS] = {
@@ -248,12 +250,13 @@ xenUnifiedXendProbe (void)
}
#endif
+
+
static virDrvOpenStatus
xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
{
int i, ret = VIR_DRV_OPEN_DECLINED;
xenUnifiedPrivatePtr priv;
- virDomainEventCallbackListPtr cbList;
#ifdef __sun
/*
@@ -323,17 +326,16 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
return VIR_DRV_OPEN_ERROR;
}
- /* Allocate callback list */
- if (VIR_ALLOC(cbList) < 0) {
- virReportOOMError();
+ if (!(priv->domainEvents = virDomainEventStateNew(xenDomainEventFlush,
+ priv,
+ NULL,
+ NULL))) {
virMutexDestroy(&priv->lock);
VIR_FREE(priv);
return VIR_DRV_OPEN_ERROR;
}
conn->privateData = priv;
- priv->domainEventCallbacks = cbList;
-
priv->handle = -1;
priv->xendConfigVersion = -1;
priv->xshandle = NULL;
@@ -423,7 +425,7 @@ xenUnifiedClose (virConnectPtr conn)
int i;
virCapabilitiesFree(priv->caps);
- virDomainEventCallbackListFree(priv->domainEventCallbacks);
+ virDomainEventStateFree(priv->domainEvents);
for (i = 0; i < XEN_UNIFIED_NR_DRIVERS; ++i)
if (priv->opened[i])
@@ -1845,7 +1847,7 @@ xenUnifiedDomainEventRegister(virConnectPtr conn,
return -1;
}
- ret = virDomainEventCallbackListAdd(conn, priv->domainEventCallbacks,
+ ret = virDomainEventCallbackListAdd(conn, priv->domainEvents->callbacks,
callback, opaque, freefunc);
xenUnifiedUnlock(priv);
@@ -1867,12 +1869,9 @@ xenUnifiedDomainEventDeregister(virConnectPtr conn,
return -1;
}
- if (priv->domainEventDispatching)
- ret = virDomainEventCallbackListMarkDelete(conn, priv->domainEventCallbacks,
- callback);
- else
- ret = virDomainEventCallbackListRemove(conn, priv->domainEventCallbacks,
- callback);
+ ret = virDomainEventStateDeregister(conn,
+ priv->domainEvents,
+ callback);
xenUnifiedUnlock(priv);
return ret;
@@ -1898,7 +1897,7 @@ xenUnifiedDomainEventRegisterAny(virConnectPtr conn,
return -1;
}
- ret = virDomainEventCallbackListAddID(conn, priv->domainEventCallbacks,
+ ret = virDomainEventCallbackListAddID(conn, priv->domainEvents->callbacks,
dom, eventID,
callback, opaque, freefunc);
@@ -1920,12 +1919,9 @@ xenUnifiedDomainEventDeregisterAny(virConnectPtr conn,
return -1;
}
- if (priv->domainEventDispatching)
- ret = virDomainEventCallbackListMarkDeleteID(conn, priv->domainEventCallbacks,
- callbackID);
- else
- ret = virDomainEventCallbackListRemoveID(conn, priv->domainEventCallbacks,
- callbackID);
+ ret = virDomainEventStateDeregisterAny(conn,
+ priv->domainEvents,
+ callbackID);
xenUnifiedUnlock(priv);
return ret;
@@ -2390,6 +2386,7 @@ xenUnifiedRemoveDomainInfo(xenUnifiedDomainInfoListPtr list,
return -1;
}
+
static void
xenUnifiedDomainEventDispatchFunc(virConnectPtr conn,
virDomainEventPtr event,
@@ -2411,6 +2408,19 @@ xenUnifiedDomainEventDispatchFunc(virConnectPtr conn,
xenUnifiedLock(priv);
}
+static void xenDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque)
+{
+ virConnectPtr conn = opaque;
+ xenUnifiedPrivatePtr priv = conn->privateData;
+
+ xenUnifiedLock(priv);
+ virDomainEventStateFlush(priv->domainEvents,
+ xenUnifiedDomainEventDispatchFunc,
+ priv);
+ xenUnifiedUnlock(priv);
+}
+
+
/**
* xenUnifiedDomainEventDispatch:
* @priv: the connection to dispatch events on
@@ -2427,21 +2437,7 @@ void xenUnifiedDomainEventDispatch (xenUnifiedPrivatePtr priv,
if (!priv)
return;
- priv->domainEventDispatching = 1;
-
- if (priv->domainEventCallbacks) {
- virDomainEventDispatch(event,
- priv->domainEventCallbacks,
- xenUnifiedDomainEventDispatchFunc,
- priv);
-
- /* Purge any deleted callbacks */
- virDomainEventCallbackListPurgeMarked(priv->domainEventCallbacks);
- }
-
- virDomainEventFree(event);
-
- priv->domainEventDispatching = 0;
+ virDomainEventStateQueue(priv->domainEvents, event);
}
void xenUnifiedLock(xenUnifiedPrivatePtr priv)
diff --git a/src/xen/xen_driver.h b/src/xen/xen_driver.h
index ca711d4..4122378 100644
--- a/src/xen/xen_driver.h
+++ b/src/xen/xen_driver.h
@@ -183,9 +183,7 @@ struct _xenUnifiedPrivate {
int nbNodeCells;
int nbNodeCpus;
- /* An list of callbacks */
- virDomainEventCallbackListPtr domainEventCallbacks;
- int domainEventDispatching;
+ virDomainEventStatePtr domainEvents;
/* Location of config files, either /etc
* or /var/lib/xen */
--
1.7.7.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list