[libvirt] [PATCH 1/8] Convert Xen & VBox drivers to use virDomainEventState

Daniel P. Berrange berrange at redhat.com
Wed Dec 14 00:38:18 UTC 2011


From: "Daniel P. Berrange" <berrange at redhat.com>

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




More information about the libvir-list mailing list