factor out virConnectCloseCallbackDataPtr methods

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@virtuozzo.com>

diff --git a/src/datatypes.c b/src/datatypes.c
index da6ec371f42115527824a8b0fe53933732068f32..3e1d809b4007302db32b5e65febbf87891f651c8 100644 (file)
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -184,6 +184,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 31c636ca6c5da9bed9694f8e1b9229b4198b51f3..238317a9e3b23d29bea6fa8781908ece62de0a46 100644 (file)
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -639,4 +639,17 @@ virAdmConnectPtr virAdmConnectNew(void);
 
 virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn,
                                 const char *name);
+
+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 9c88426ec48df17dbacc3106f4418706afbaabcc..ced6a549a2cd6522c12fed6d70507c0a102aa3fa 100644 (file)
--- 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 7cf61cf562a407d7560da60aee7ce932cce00cfe..e0cf33fdc3fa800d99944f53010a3230faf04802 100644 (file)
--- 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 */