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>

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:

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__ */

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;

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 */