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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]