[PATCH v5 01/10] factor out virConnectCloseCallbackDataPtr methods

[Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>]

Make register and unregister functions return void because
we can check the state of callback object beforehand via
virConnectCloseCallbackDataGetCallback. This can be done
without race conditions if we use higher level locks for registering
and unregistering. The fact they return void simplifies
task of consistent registering/unregistering.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
---
 src/datatypes.c            | 80 ++++++++++++++++++++++++++++++++++++++++++++++
 src/datatypes.h            | 12 +++++++
 src/libvirt-host.c         | 29 +++--------------
 src/remote/remote_driver.c | 17 ++--------
 4 files changed, 99 insertions(+), 39 deletions(-)

diff --git a/src/datatypes.c b/src/datatypes.c
index c832d80..030a0d2 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -180,6 +180,86 @@ virConnectCloseCallbackDataDispose(void *obj)
     virObjectUnlock(cb);
 }
 
+void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close,
+                                         virConnectPtr conn,
+                                         virConnectCloseFunc cb,
+                                         void *opaque,
+                                         virFreeCallback freecb)
+{
+    virObjectLock(close);
+
+    if (close->callback != NULL) {
+        VIR_WARN("Attempt to register callback on armed"
+                 " close callback object %p", close);
+        goto cleanup;
+        return;
+    }
+
+    close->conn = conn;
+    virObjectRef(close->conn);
+    close->callback = cb;
+    close->opaque = opaque;
+    close->freeCallback = freecb;
+
+ cleanup:
+
+    virObjectUnlock(close);
+}
+
+void virConnectCloseCallbackDataUnregister(virConnectCloseCallbackDataPtr close,
+                                           virConnectCloseFunc cb)
+{
+    virObjectLock(close);
+
+    if (close->callback != cb) {
+        VIR_WARN("Attempt to unregister different callback on "
+                 " close callback object %p", close);
+        goto cleanup;
+    }
+
+    close->callback = NULL;
+    if (close->freeCallback)
+        close->freeCallback(close->opaque);
+    close->freeCallback = NULL;
+    virObjectUnref(close->conn);
+
+ cleanup:
+
+    virObjectUnlock(close);
+}
+
+void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close,
+                                     int reason)
+{
+    virObjectLock(close);
+
+    if (!close->callback)
+        goto exit;
+
+    VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p",
+              close->callback, reason, close->opaque);
+    close->callback(close->conn, reason, close->opaque);
+
+    if (close->freeCallback)
+        close->freeCallback(close->opaque);
+    close->callback = NULL;
+    close->freeCallback = NULL;
+
+ exit:
+    virObjectUnlock(close);
+}
+
+virConnectCloseFunc
+virConnectCloseCallbackDataGetCallback(virConnectCloseCallbackDataPtr close)
+{
+    virConnectCloseFunc cb;
+
+    virObjectLock(close);
+    cb = close->callback;
+    virObjectUnlock(close);
+
+    return cb;
+}
 
 /**
  * virGetDomain:
diff --git a/src/datatypes.h b/src/datatypes.h
index 1b1777d..2ee77cd 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -601,4 +601,16 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
 
 virAdmConnectPtr virAdmConnectNew(void);
 
+void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close,
+                                         virConnectPtr conn,
+                                         virConnectCloseFunc cb,
+                                         void *opaque,
+                                         virFreeCallback freecb);
+void virConnectCloseCallbackDataUnregister(virConnectCloseCallbackDataPtr close,
+                                           virConnectCloseFunc cb);
+void virConnectCloseCallbackDataCall(virConnectCloseCallbackDataPtr close,
+                                     int reason);
+virConnectCloseFunc
+virConnectCloseCallbackDataGetCallback(virConnectCloseCallbackDataPtr close);
+
 #endif /* __VIR_DATATYPES_H__ */
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index 9c88426..ced6a54 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -1216,35 +1216,25 @@ virConnectRegisterCloseCallback(virConnectPtr conn,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-
-    virObjectRef(conn);
-
     virObjectLock(conn);
-    virObjectLock(conn->closeCallback);
 
     virCheckNonNullArgGoto(cb, error);
 
-    if (conn->closeCallback->callback) {
+    if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != NULL) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("A close callback is already registered"));
         goto error;
     }
 
-    conn->closeCallback->conn = conn;
-    conn->closeCallback->callback = cb;
-    conn->closeCallback->opaque = opaque;
-    conn->closeCallback->freeCallback = freecb;
+    virConnectCloseCallbackDataRegister(conn->closeCallback, conn, cb,
+                                        opaque, freecb);
 
-    virObjectUnlock(conn->closeCallback);
     virObjectUnlock(conn);
-
     return 0;
 
  error:
-    virObjectUnlock(conn->closeCallback);
     virObjectUnlock(conn);
     virDispatchError(conn);
-    virObjectUnref(conn);
     return -1;
 }
 
@@ -1271,31 +1261,22 @@ virConnectUnregisterCloseCallback(virConnectPtr conn,
     virResetLastError();
 
     virCheckConnectReturn(conn, -1);
-
     virObjectLock(conn);
-    virObjectLock(conn->closeCallback);
 
     virCheckNonNullArgGoto(cb, error);
 
-    if (conn->closeCallback->callback != cb) {
+    if (virConnectCloseCallbackDataGetCallback(conn->closeCallback) != cb) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("A different callback was requested"));
         goto error;
     }
 
-    conn->closeCallback->callback = NULL;
-    if (conn->closeCallback->freeCallback)
-        conn->closeCallback->freeCallback(conn->closeCallback->opaque);
-    conn->closeCallback->freeCallback = NULL;
+    virConnectCloseCallbackDataUnregister(conn->closeCallback, cb);
 
-    virObjectUnlock(conn->closeCallback);
     virObjectUnlock(conn);
-    virObjectUnref(conn);
-
     return 0;
 
  error:
-    virObjectUnlock(conn->closeCallback);
     virObjectUnlock(conn);
     virDispatchError(conn);
     return -1;
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7cf61cf..e0cf33f 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -535,21 +535,8 @@ remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED,
                       int reason,
                       void *opaque)
 {
-    virConnectCloseCallbackDataPtr cbdata = opaque;
-
-    virObjectLock(cbdata);
-
-    if (cbdata->callback) {
-        VIR_DEBUG("Triggering connection close callback %p reason=%d, opaque=%p",
-                  cbdata->callback, reason, cbdata->opaque);
-        cbdata->callback(cbdata->conn, reason, cbdata->opaque);
-
-        if (cbdata->freeCallback)
-            cbdata->freeCallback(cbdata->opaque);
-        cbdata->callback = NULL;
-        cbdata->freeCallback = NULL;
-    }
-    virObjectUnlock(cbdata);
+    virConnectCloseCallbackDataCall((virConnectCloseCallbackDataPtr)opaque,
+                                    reason);
 }
 
 /* helper macro to ease extraction of arguments from the URI */
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list