[libvirt] [PATCH v2 7/9] admin: Add support for connection close callbacks

[Martin Kletzander]

On Fri, Oct 16, 2015 at 08:12:24PM +0200, Erik Skultety wrote:
>As we need a client disconnect handler, we also need a mechanism to register
>such handlers for a client. This patch introduced both the close callbacks and
>also the client vshAdmCatchDisconnect handler to be registered with it. By
>registering the handler we still need to make sure the client can react to
>daemon's events like disconnect or keepalive, so asynchronous I/O event polling
>is necessary to be enabled too.
>---
> include/libvirt/libvirt-admin.h |  19 +++++++
> src/admin/admin_remote.c        |  36 +++++++++++++
> src/datatypes.c                 |  20 +++++++
> src/datatypes.h                 |  14 ++++-
> src/libvirt-admin.c             | 112 ++++++++++++++++++++++++++++++++++++++++
> src/libvirt_admin_public.syms   |   2 +
> tools/virt-admin.c              |  50 ++++++++++++++++++
> 7 files changed, 252 insertions(+), 1 deletion(-)
>
>diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
>index aae10b4..1688201 100644
>--- a/include/libvirt/libvirt-admin.h
>+++ b/include/libvirt/libvirt-admin.h
>@@ -56,6 +56,25 @@ int virAdmConnectIsAlive(virAdmConnectPtr conn);
>
> char * virAdmConnectGetURI(virAdmConnectPtr conn);
>
>+/**
>+ * virAdmConnectCloseFunc:
>+ * @conn: virAdmConnect connection
>+ * @reason: reason why the connection was closed (see virConnectCloseReason)
>+ * @opaque: opaque client data
>+ *
>+ * A callback to be registered, in case a connection was closed.
>+ */
>+typedef void (*virAdmConnectCloseFunc)(virAdmConnectPtr conn,
>+                                       int reason,
>+                                       void *opaque);
>+
>+int virAdmConnectRegisterCloseCallback(virAdmConnectPtr conn,
>+                                       virAdmConnectCloseFunc cb,
>+                                       void *opaque,
>+                                       virFreeCallback freecb);
>+int virAdmConnectUnregisterCloseCallback(virAdmConnectPtr conn,
>+                                         virAdmConnectCloseFunc cb);
>+
> # ifdef __cplusplus
> }
> # endif
>diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c
>index 2c02ba6..741b9a1 100644
>--- a/src/admin/admin_remote.c
>+++ b/src/admin/admin_remote.c
>@@ -101,6 +101,28 @@ call(virAdmConnectPtr conn,
>
> #include "admin_client.h"
>
>+static void
>+remoteAdminClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED,
>+                           int reason,
>+                           void *opaque)
>+{
>+    virAdmConnectCloseCallbackDataPtr 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);
>+}
>+
> static int
> remoteAdminConnectOpen(virAdmConnectPtr conn, unsigned int flags)
> {
>@@ -112,6 +134,17 @@ remoteAdminConnectOpen(virAdmConnectPtr conn, unsigned int flags)
>
>     args.flags = flags;
>
>+    if (virNetClientRegisterAsyncIO(priv->client) < 0) {
>+        VIR_DEBUG("Failed to add event watch, disabling events and support for"
>+                  " keepalive messages");
>+        virResetLastError();
>+    }
>+
>+    virObjectRef(conn->closeCallback);
>+    virNetClientSetCloseCallback(priv->client, remoteAdminClientCloseFunc,
>+                                 conn->closeCallback,
>+                                 virObjectFreeCallback);
>+
>     if (call(conn, 0, ADMIN_PROC_CONNECT_OPEN,
>              (xdrproc_t)xdr_admin_connect_open_args, (char *)&args,
>              (xdrproc_t)xdr_void, (char *)NULL) == -1) {
>@@ -139,6 +172,9 @@ remoteAdminConnectClose(virAdmConnectPtr conn)
>         goto done;
>     }
>
>+    virNetClientSetCloseCallback(priv->client, NULL, conn->closeCallback,
>+                                 virObjectFreeCallback);

Again, yes, we have this code in place already, in the remote driver,
and I understand it actually means RemoveCloseCallback, but why aren't
we passing NULL as all three parameters after the client?

>+
>     rv = 0;
>
>  done:
>diff --git a/src/datatypes.c b/src/datatypes.c
>index aeac301..603b168 100644
>--- a/src/datatypes.c
>+++ b/src/datatypes.c
>@@ -60,8 +60,10 @@ static void virStorageVolDispose(void *obj);
> static void virStoragePoolDispose(void *obj);
>
> virClassPtr virAdmConnectClass;
>+virClassPtr virAdmConnectCloseCallbackDataClass;
>
> static void virAdmConnectDispose(void *obj);
>+static void virAdmConnectCloseCallbackDataDispose(void *obj);
>
> static int
> virDataTypesOnceInit(void)
>@@ -91,6 +93,7 @@ virDataTypesOnceInit(void)
>     DECLARE_CLASS(virStoragePool);
>
>     DECLARE_CLASS_LOCKABLE(virAdmConnect);
>+    DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData);
>
> #undef DECLARE_CLASS_COMMON
> #undef DECLARE_CLASS_LOCKABLE
>@@ -822,6 +825,9 @@ virAdmConnectNew(void)
>     if (!(ret = virObjectLockableNew(virAdmConnectClass)))
>         return NULL;
>
>+    if (!(ret->closeCallback = virObjectLockableNew(virAdmConnectCloseCallbackDataClass)))
>+        return NULL;
>+
>     return ret;
> }
>
>@@ -834,4 +840,18 @@ virAdmConnectDispose(void *obj)
>         conn->privateDataFreeFunc(conn);
>
>     virURIFree(conn->uri);
>+    virObjectUnref(conn->closeCallback);
>+}
>+
>+static void
>+virAdmConnectCloseCallbackDataDispose(void *obj)
>+{
>+    virAdmConnectCloseCallbackDataPtr cb_data = obj;
>+
>+    virObjectLock(cb_data);
>+
>+    if (cb_data->freeCallback)
>+        cb_data->freeCallback(cb_data->opaque);
>+
>+    virObjectUnlock(cb_data);
> }
>diff --git a/src/datatypes.h b/src/datatypes.h
>index 95aa49e..aa160a8 100644
>--- a/src/datatypes.h
>+++ b/src/datatypes.h
>@@ -331,9 +331,11 @@ extern virClassPtr virAdmConnectClass;
>
> typedef struct _virConnectCloseCallbackData virConnectCloseCallbackData;
> typedef virConnectCloseCallbackData *virConnectCloseCallbackDataPtr;
>+typedef struct _virAdmConnectCloseCallbackData virAdmConnectCloseCallbackData;
>+typedef virAdmConnectCloseCallbackData *virAdmConnectCloseCallbackDataPtr;
>
> /**
>- * Internal structure holding data related to connection close callbacks.
>+ * Internal structures holding data related to connection close callbacks.
>  */
> struct _virConnectCloseCallbackData {
>     virObjectLockable parent;
>@@ -344,6 +346,15 @@ struct _virConnectCloseCallbackData {
>     virFreeCallback freeCallback;
> };
>
>+struct _virAdmConnectCloseCallbackData {
>+    virObjectLockable parent;
>+
>+    virAdmConnectPtr conn;
>+    virAdmConnectCloseFunc callback;
>+    void *opaque;
>+    virFreeCallback freeCallback;
>+};
>+
> /**
>  * _virConnect:
>  *
>@@ -401,6 +412,7 @@ struct _virAdmConnect {
>
>     void *privateData;
>     virFreeCallback privateDataFreeFunc;

I'd add one empty line here.

Otherwise looks fine to me (with the adjustments that will be needed
after previous patches are fixed, of course).

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151103/aebf43d0/attachment-0001.sig>