Convert Xen & VBox drivers to use virDomainEventState

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

@@ -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,6 +1356,18 @@ virDomainEventStateDeregister(virConnectPtr conn,
     return ret;
 }
 
+
+/**
+ * virDomainEventStateDeregisterAny:
+ * @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,

src/vbox/vbox_tmpl.c

@@ -188,11 +188,9 @@ typedef struct {
 
 #else /* !(VBOX_API_VERSION == 2002) */
 
-    /* An array of callbacks */
+    /* Async event handling */
-    virDomainEventCallbackListPtr domainEventCallbacks;
+    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) {
+    if (!(data->domainEvents = virDomainEventStateNew(vboxDomainEventFlush,
-        virReportOOMError();
+                                                      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) {
+            if (ev)
-                virDomainEventDispatch(ev,
+                virDomainEventStateQueue(g_pVBoxGlobalData->domainEvents, ev);
-                                       g_pVBoxGlobalData->domainEventCallbacks,
-                                       virDomainEventDispatchDefaultFunc,
-                                       NULL);
-                virDomainEventFree(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) {
+            if (ev)
-                virDomainEventDispatch(ev,
+                virDomainEventStateQueue(g_pVBoxGlobalData->domainEvents, ev);
-                                       g_pVBoxGlobalData->domainEventCallbacks,
-                                       virDomainEventDispatchDefaultFunc,
-                                       NULL);
-                virDomainEventFree(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, "
+                  "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, "
-                  "freecb: %p )", ret, conn, data->domainEventCallbacks, callback,
+                  "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)
+    cnt = virDomainEventStateDeregister(conn, data->domainEvents,
-        ret = virDomainEventCallbackListMarkDelete(conn, data->domainEventCallbacks,
+                                        callback);
-                                                   callback);
-    else
-        ret = virDomainEventCallbackListRemove(conn, data->domainEventCallbacks,
-                                               callback);
 
-    if (data->vboxCallback) {
+    if (data->vboxCallback && cnt == 0) {
-        /* check count here of how many times register was called
+        data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
-         * and only on the last de-register do the un-register call
+        VBOX_RELEASE(data->vboxCallback);
-         */
-        if (data->domainEventCallbacks && virDomainEventCallbackListCount(data->domainEventCallbacks) == 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 */
+        /* Remove the Event file handle on which we are listening as well */
-            virEventRemoveHandle(data->fdWatch);
+        virEventRemoveHandle(data->fdWatch);
-            data->fdWatch = -1;
+        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, "
+                  "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, "
-                  "freecb: %p )", ret, conn, data->domainEventCallbacks, callback,
+                  "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)
+    cnt = virDomainEventStateDeregisterAny(conn, data->domainEvents,
-        ret = virDomainEventCallbackListMarkDeleteID(conn, data->domainEventCallbacks,
+                                           callbackID);
-                                                     callbackID);
-    else
-        ret = virDomainEventCallbackListRemoveID(conn, data->domainEventCallbacks,
-                                                 callbackID);
 
-    if (data->vboxCallback) {
+    if (data->vboxCallback && cnt == 0) {
-        /* check count here of how many times register was called
+        data->vboxObj->vtbl->UnregisterCallback(data->vboxObj, data->vboxCallback);
-         * and only on the last de-register do the un-register call
+        VBOX_RELEASE(data->vboxCallback);
-         */
-        if (data->domainEventCallbacks && virDomainEventCallbackListCount(data->domainEventCallbacks) == 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 */
+        /* Remove the Event file handle on which we are listening as well */
-            virEventRemoveHandle(data->fdWatch);
+        virEventRemoveHandle(data->fdWatch);
-            data->fdWatch = -1;
+        data->fdWatch = -1;
-        }
     }
 
     vboxDriverUnlock(data);

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 (!(priv->domainEvents = virDomainEventStateNew(xenDomainEventFlush,
-    if (VIR_ALLOC(cbList) < 0) {
+                                                      priv,
-        virReportOOMError();
+                                                      NULL,
+                                                      false))) {
         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 = virDomainEventStateDeregister(conn,
-        ret = virDomainEventCallbackListMarkDelete(conn, priv->domainEventCallbacks,
+                                        priv->domainEvents,
-                                                   callback);
+                                        callback);
-    else
-        ret = virDomainEventCallbackListRemove(conn, priv->domainEventCallbacks,
-                                               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 = virDomainEventStateDeregisterAny(conn,
-        ret = virDomainEventCallbackListMarkDeleteID(conn, priv->domainEventCallbacks,
+                                           priv->domainEvents,
-                                                     callbackID);
+                                           callbackID);
-    else
-        ret = virDomainEventCallbackListRemoveID(conn, priv->domainEventCallbacks,
-                                                 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;
+    virDomainEventStateQueue(priv->domainEvents, event);
-
-    if (priv->domainEventCallbacks) {
-        virDomainEventDispatch(event,
-                               priv->domainEventCallbacks,
-                               xenUnifiedDomainEventDispatchFunc,
-                               priv);
-
-        /* Purge any deleted callbacks */
-        virDomainEventCallbackListPurgeMarked(priv->domainEventCallbacks);
-    }
-
-    virDomainEventFree(event);
-
-    priv->domainEventDispatching = 0;
 }
 
 void xenUnifiedLock(xenUnifiedPrivatePtr priv)

src/xen/xen_driver.h

@@ -183,9 +183,7 @@ struct _xenUnifiedPrivate {
     int nbNodeCells;
     int nbNodeCpus;
 
-    /* An list of callbacks */
+    virDomainEventStatePtr domainEvents;
-    virDomainEventCallbackListPtr domainEventCallbacks;
-    int domainEventDispatching;
 
     /* Location of config files, either /etc
      * or /var/lib/xen */