[libvirt] [PATCH 2/8] Return count of callbacks when registering callbacks

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


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

When registering a callback for a particular event some callers
need to know how many callbacks already exist for that event.
While it is possible to ask for a count, this is not free from
race conditions when threaded. Thus the API for registering
callbacks should return the count of callbacks

* src/conf/domain_event.c, src/conf/domain_event.h,
  src/libvirt_private.syms: Return count of callbacks when
  registering callbacks
* src/libxl/libxl_driver.c, src/libxl/libxl_driver.c,
  src/qemu/qemu_driver.c, src/remote/remote_driver.c,
  src/remote/remote_driver.c, src/uml/uml_driver.c,
  src/vbox/vbox_tmpl.c, src/xen/xen_driver.c: Update
  for change in APIs
---
 src/conf/domain_event.c    |   23 ++++++++++++++++++-----
 src/conf/domain_event.h    |    3 ++-
 src/libvirt_private.syms   |    2 +-
 src/libxl/libxl_driver.c   |   15 ++++++++-------
 src/lxc/lxc_driver.c       |   15 ++++++++-------
 src/qemu/qemu_driver.c     |   15 ++++++++-------
 src/remote/remote_driver.c |   33 ++++++++++++++++-----------------
 src/test/test_driver.c     |   15 ++++++++-------
 src/uml/uml_driver.c       |   15 ++++++++-------
 src/vbox/vbox_tmpl.c       |   15 ++++++++-------
 src/xen/xen_driver.c       |   13 +++++++------
 11 files changed, 92 insertions(+), 72 deletions(-)

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index 0e481cf..00c5dbf 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -357,7 +357,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
     return virDomainEventCallbackListAddID(conn, cbList, NULL,
                                            VIR_DOMAIN_EVENT_ID_LIFECYCLE,
                                            VIR_DOMAIN_EVENT_CALLBACK(callback),
-                                           opaque, freecb);
+                                           opaque, freecb, NULL);
 }
 
 
@@ -368,6 +368,7 @@ virDomainEventCallbackListAdd(virConnectPtr conn,
  * @eventID: the event ID
  * @callback: the callback to add
  * @opaque: opaque data tio pass to callback
+ * @callbackID: filled with callback ID
  *
  * Internal function to add a callback from a virDomainEventCallbackListPtr
  */
@@ -378,10 +379,12 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
                                 int eventID,
                                 virConnectDomainEventGenericCallback callback,
                                 void *opaque,
-                                virFreeCallback freecb)
+                                virFreeCallback freecb,
+                                int *callbackID)
 {
     virDomainEventCallbackPtr event;
     int i;
+    int ret = 0;
 
     /* Check incoming */
     if ( !cbList ) {
@@ -427,7 +430,17 @@ virDomainEventCallbackListAddID(virConnectPtr conn,
 
     event->callbackID = cbList->nextID++;
 
-    return event->callbackID;
+    for (i = 0 ; i < cbList->count ; i++) {
+        if (cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE &&
+            cbList->callbacks[i]->conn == conn &&
+            !cbList->callbacks[i]->deleted)
+            ret++;
+    }
+
+    if (callbackID)
+        *callbackID = event->callbackID;
+
+    return ret;
 
 no_memory:
     virReportOOMError();
@@ -1364,9 +1377,9 @@ virDomainEventStateDeregister(virConnectPtr conn,
  * @callbackID: ID of the function to remove from event
  *
  * Unregister the function @callbackID with connection @conn,
- * from @state, for lifecycle events.
+ * from @state, for events.
  *
- * Returns: the number of lifecycle callbacks still registered, or -1 on error
+ * Returns: the number of callbacks still registered, or -1 on error
  */
 int
 virDomainEventStateDeregisterID(virConnectPtr conn,
diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h
index 10a55b0..83656e6 100644
--- a/src/conf/domain_event.h
+++ b/src/conf/domain_event.h
@@ -80,7 +80,8 @@ int virDomainEventCallbackListAddID(virConnectPtr conn,
                                     int eventID,
                                     virConnectDomainEventGenericCallback cb,
                                     void *opaque,
-                                    virFreeCallback freecb)
+                                    virFreeCallback freecb,
+                                    int *callbackID)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(5);
 
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a81c230..3d1d7d2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -510,7 +510,7 @@ virDomainEventRebootNew;
 virDomainEventRebootNewFromDom;
 virDomainEventRebootNewFromObj;
 virDomainEventStateDeregister;
-virDomainEventStateDeregisterAny;
+virDomainEventStateDeregisterID;
 virDomainEventStateFlush;
 virDomainEventStateFree;
 virDomainEventStateNew;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 7cc32ad..46504dc 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3851,10 +3851,11 @@ libxlDomainEventRegisterAny(virConnectPtr conn, virDomainPtr dom, int eventID,
     int ret;
 
     libxlDriverLock(driver);
-    ret = virDomainEventCallbackListAddID(conn,
-                                          driver->domainEventState->callbacks,
-                                          dom, eventID, callback, opaque,
-                                          freecb);
+    if (virDomainEventCallbackListAddID(conn,
+                                        driver->domainEventState->callbacks,
+                                        dom, eventID, callback, opaque,
+                                        freecb, &ret) < 0)
+        ret = -1;
     libxlDriverUnlock(driver);
 
     return ret;
@@ -3868,9 +3869,9 @@ libxlDomainEventDeregisterAny(virConnectPtr conn, int callbackID)
     int ret;
 
     libxlDriverLock(driver);
-    ret = virDomainEventStateDeregisterAny(conn,
-                                           driver->domainEventState,
-                                           callbackID);
+    ret = virDomainEventStateDeregisterID(conn,
+                                          driver->domainEventState,
+                                          callbackID);
     libxlDriverUnlock(driver);
 
     return ret;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b16cfd8..6b17c75 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -2164,10 +2164,11 @@ lxcDomainEventRegisterAny(virConnectPtr conn,
     int ret;
 
     lxcDriverLock(driver);
-    ret = virDomainEventCallbackListAddID(conn,
-                                          driver->domainEventState->callbacks,
-                                          dom, eventID,
-                                          callback, opaque, freecb);
+    if (virDomainEventCallbackListAddID(conn,
+                                        driver->domainEventState->callbacks,
+                                        dom, eventID,
+                                        callback, opaque, freecb, &ret) < 0)
+        ret = -1;
     lxcDriverUnlock(driver);
 
     return ret;
@@ -2182,9 +2183,9 @@ lxcDomainEventDeregisterAny(virConnectPtr conn,
     int ret;
 
     lxcDriverLock(driver);
-    ret = virDomainEventStateDeregisterAny(conn,
-                                           driver->domainEventState,
-                                           callbackID);
+    ret = virDomainEventStateDeregisterID(conn,
+                                          driver->domainEventState,
+                                          callbackID);
     lxcDriverUnlock(driver);
 
     return ret;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 10a289e..7423340 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8180,10 +8180,11 @@ qemuDomainEventRegisterAny(virConnectPtr conn,
     int ret;
 
     qemuDriverLock(driver);
-    ret = virDomainEventCallbackListAddID(conn,
-                                          driver->domainEventState->callbacks,
-                                          dom, eventID,
-                                          callback, opaque, freecb);
+    if (virDomainEventCallbackListAddID(conn,
+                                        driver->domainEventState->callbacks,
+                                        dom, eventID,
+                                        callback, opaque, freecb, &ret) < 0)
+        ret = -1;
     qemuDriverUnlock(driver);
 
     return ret;
@@ -8198,9 +8199,9 @@ qemuDomainEventDeregisterAny(virConnectPtr conn,
     int ret;
 
     qemuDriverLock(driver);
-    ret = virDomainEventStateDeregisterAny(conn,
-                                           driver->domainEventState,
-                                           callbackID);
+    ret = virDomainEventStateDeregisterID(conn,
+                                          driver->domainEventState,
+                                          callbackID);
     qemuDriverUnlock(driver);
 
     return ret;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ff2d4b4..a0cf7d3 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3120,6 +3120,7 @@ static int remoteDomainEventRegister(virConnectPtr conn,
 {
     int rv = -1;
     struct private_data *priv = conn->privateData;
+    int count;
 
     remoteDriverLock(priv);
 
@@ -3128,15 +3129,13 @@ static int remoteDomainEventRegister(virConnectPtr conn,
          goto done;
     }
 
-    if (virDomainEventCallbackListAdd(conn, priv->domainEventState->callbacks,
-                                      callback, opaque, freecb) < 0) {
+    if ((count = virDomainEventCallbackListAdd(conn, priv->domainEventState->callbacks,
+                                               callback, opaque, freecb)) < 0) {
          remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
          goto done;
     }
 
-    if (virDomainEventCallbackListCountID(conn,
-                                          priv->domainEventState->callbacks,
-                                          VIR_DOMAIN_EVENT_ID_LIFECYCLE) == 1) {
+    if (count == 1) {
         /* Tell the server when we are the first callback deregistering */
         if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_REGISTER,
                 (xdrproc_t) xdr_void, (char *) NULL,
@@ -3760,6 +3759,7 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
     struct private_data *priv = conn->privateData;
     remote_domain_events_register_any_args args;
     int callbackID;
+    int count;
 
     remoteDriverLock(priv);
 
@@ -3768,19 +3768,18 @@ static int remoteDomainEventRegisterAny(virConnectPtr conn,
          goto done;
     }
 
-    if ((callbackID = virDomainEventCallbackListAddID(conn,
-                                                      priv->domainEventState->callbacks,
-                                                      dom, eventID,
-                                                      callback, opaque, freecb)) < 0) {
-         remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
-         goto done;
+    if ((count = virDomainEventCallbackListAddID(conn,
+                                                 priv->domainEventState->callbacks,
+                                                 dom, eventID,
+                                                 callback, opaque, freecb,
+                                                 &callbackID)) < 0) {
+        remoteError(VIR_ERR_RPC, "%s", _("adding cb to list"));
+        goto done;
     }
 
     /* If this is the first callback for this eventID, we need to enable
      * events on the server */
-    if (virDomainEventCallbackListCountID(conn,
-                                          priv->domainEventState->callbacks,
-                                          eventID) == 1) {
+    if (count == 1) {
         args.eventID = eventID;
 
         if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_REGISTER_ANY,
@@ -3818,9 +3817,9 @@ static int remoteDomainEventDeregisterAny(virConnectPtr conn,
         goto done;
     }
 
-    if (virDomainEventStateDeregisterAny(conn,
-                                         priv->domainEventState,
-                                         callbackID) < 0)
+    if (virDomainEventStateDeregisterID(conn,
+                                        priv->domainEventState,
+                                        callbackID) < 0)
         goto done;
 
     /* If that was the last callback for this eventID, we need to disable
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 89f7df1..f32990b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5469,10 +5469,11 @@ testDomainEventRegisterAny(virConnectPtr conn,
     int ret;
 
     testDriverLock(driver);
-    ret = virDomainEventCallbackListAddID(conn,
-                                          driver->domainEventState->callbacks,
-                                          dom, eventID,
-                                          callback, opaque, freecb);
+    if (virDomainEventCallbackListAddID(conn,
+                                        driver->domainEventState->callbacks,
+                                        dom, eventID,
+                                        callback, opaque, freecb, &ret) < 0)
+        ret = -1;
     testDriverUnlock(driver);
 
     return ret;
@@ -5486,9 +5487,9 @@ testDomainEventDeregisterAny(virConnectPtr conn,
     int ret;
 
     testDriverLock(driver);
-    ret = virDomainEventStateDeregisterAny(conn,
-                                           driver->domainEventState,
-                                           callbackID);
+    ret = virDomainEventStateDeregisterID(conn,
+                                          driver->domainEventState,
+                                          callbackID);
     testDriverUnlock(driver);
 
     return ret;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index faea313..420488d 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -2483,10 +2483,11 @@ umlDomainEventRegisterAny(virConnectPtr conn,
     int ret;
 
     umlDriverLock(driver);
-    ret = virDomainEventCallbackListAddID(conn,
-                                          driver->domainEventState->callbacks,
-                                          dom, eventID,
-                                          callback, opaque, freecb);
+    if (virDomainEventCallbackListAddID(conn,
+                                        driver->domainEventState->callbacks,
+                                        dom, eventID,
+                                        callback, opaque, freecb, &ret) < 0)
+        ret = -1;
     umlDriverUnlock(driver);
 
     return ret;
@@ -2501,9 +2502,9 @@ umlDomainEventDeregisterAny(virConnectPtr conn,
     int ret;
 
     umlDriverLock(driver);
-    ret = virDomainEventStateDeregisterAny(conn,
-                                           driver->domainEventState,
-                                           callbackID);
+    ret = virDomainEventStateDeregisterID(conn,
+                                          driver->domainEventState,
+                                          callbackID);
     umlDriverUnlock(driver);
 
     return ret;
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 7f13f90..670de9c 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -7280,13 +7280,14 @@ static int vboxDomainEventRegisterAny(virConnectPtr conn,
              * later you can iterate over them
              */
 
-            ret = virDomainEventCallbackListAddID(conn, data->domainEvents->callbacks,
-                                                  dom, eventID,
-                                                  callback, opaque, freecb);
+            if (virDomainEventCallbackListAddID(conn, data->domainEvents->callbacks,
+                                                dom, eventID,
+                                                callback, opaque, freecb, &ret) < 0)
+                ret = -1;
             VIR_DEBUG("virDomainEventCallbackListAddID (ret = %d) ( conn: %p, "
-                  "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, "
-                  "freecb: %p )", ret, conn, data->domainEvents->callbacks, callback,
-                  opaque, freecb);
+                      "data->domainEvents->callbacks: %p, callback: %p, opaque: %p, "
+                      "freecb: %p )", ret, conn, data->domainEvents->callbacks, callback,
+                      opaque, freecb);
         }
     }
 
@@ -7312,7 +7313,7 @@ static int vboxDomainEventDeregisterAny(virConnectPtr conn,
      */
     vboxDriverLock(data);
 
-    cnt = virDomainEventStateDeregisterAny(conn, data->domainEvents,
+    cnt = virDomainEventStateDeregisterID(conn, data->domainEvents,
                                            callbackID);
 
     if (data->vboxCallback && cnt == 0) {
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 8c47ad5..b11dd9e 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1897,9 +1897,10 @@ xenUnifiedDomainEventRegisterAny(virConnectPtr conn,
         return -1;
     }
 
-    ret = virDomainEventCallbackListAddID(conn, priv->domainEvents->callbacks,
-                                          dom, eventID,
-                                          callback, opaque, freefunc);
+    if (virDomainEventCallbackListAddID(conn, priv->domainEvents->callbacks,
+                                        dom, eventID,
+                                        callback, opaque, freefunc, &ret) < 0)
+        ret = -1;
 
     xenUnifiedUnlock(priv);
     return (ret);
@@ -1919,9 +1920,9 @@ xenUnifiedDomainEventDeregisterAny(virConnectPtr conn,
         return -1;
     }
 
-    ret = virDomainEventStateDeregisterAny(conn,
-                                           priv->domainEvents,
-                                           callbackID);
+    ret = virDomainEventStateDeregisterID(conn,
+                                          priv->domainEvents,
+                                          callbackID);
 
     xenUnifiedUnlock(priv);
     return ret;
-- 
1.7.7.4




More information about the libvir-list mailing list