On 13.10.2015 15:38, Erik Skultety wrote:
> As we need a client disconnect handler, we need a mechanism to register
> such handlers for a client.
> ---
> include/libvirt/libvirt-admin.h | 19 ++++++
> src/datatypes.c | 22 +++++++
> src/datatypes.h | 14 +++-
> src/libvirt-admin.c | 142
> ++++++++++++++++++++++++++++++++++++++++
> src/libvirt_admin_public.syms | 2 +
> tools/virt-admin.c | 5 ++
> 6 files changed, 203 insertions(+), 1 deletion(-)
>
> diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
> index 72671c6..4539ac6 100644
> --- a/include/libvirt/libvirt-admin.h
> +++ b/include/libvirt/libvirt-admin.h
> @@ -54,6 +54,25 @@ int virAdmConnectClose(virAdmConnectPtr conn);
> int virAdmConnectRef(virAdmConnectPtr conn);
> int virAdmConnectIsAlive(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/datatypes.c b/src/datatypes.c
> index 12bcfc1..9e374e4 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;
> }
>
> @@ -832,4 +838,20 @@ virAdmConnectDispose(void *obj)
>
> if (conn->privateDataFreeFunc)
> conn->privateDataFreeFunc(conn);
> +
> + virObjectUnref(conn->closeCallback);
> +}
> +
> +static void
> +virAdmConnectCloseCallbackDataDispose(void *obj)
> +{
> + virAdmConnectCloseCallbackDataPtr cb_data = obj;
> +
> + virObjectLock(cb_data);
> +
> + cb_data->callback = NULL;
This ^^ shouldn't be needed. We are the last ones to see cb_data alive.
> + if (cb_data->freeCallback)
> + cb_data->freeCallback(cb_data->opaque);
> +
> + virObjectUnlock(cb_data);
> }
> diff --git a/src/datatypes.h b/src/datatypes.h
> index be108fe..33df476 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:
> *
> @@ -400,6 +411,7 @@ struct _virAdmConnect {
>
> void *privateData;
> virFreeCallback privateDataFreeFunc;
> + virAdmConnectCloseCallbackDataPtr closeCallback;
> };
>
>
> diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
> index e12e2b8..9ad124e 100644
> --- a/src/libvirt-admin.c
> +++ b/src/libvirt-admin.c
> @@ -64,9 +64,31 @@ remoteAdminPrivDispose(void *opaque)
> remoteAdminPrivPtr priv = opaque;
>
> virObjectUnref(priv->program);
> + virNetClientClose(priv->client);
> virObjectUnref(priv->client);
> }
>
> +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);
> +}
>
Again, remoteAdmin** funcs should go into a separate file.
> static int
> callFull(virAdmConnectPtr conn ATTRIBUTE_UNUSED,
> @@ -137,6 +159,11 @@ remoteAdminConnectOpen(virAdmConnectPtr conn, unsigned
> int flags)
>
> args.flags = flags;
>
> + 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) {
> @@ -164,6 +191,9 @@ remoteAdminConnectClose(virAdmConnectPtr conn)
> goto done;
> }
>
> + virNetClientSetCloseCallback(priv->client, NULL, conn->closeCallback,
> + virObjectFreeCallback);
> +
> rv = 0;
>
> done:
> @@ -462,3 +492,115 @@ virAdmConnectIsAlive(virAdmConnectPtr conn)
> else
> return 0;
> }
> +
> +/**
> + * virAdmConnectRegisterCloseCallback:
> + * @conn: connection to admin server
> + * @cb: callback to be invoked upon connection close
> + * @opaque: user data to pass to @cb
> + * @freecb: callback to free @opaque
> + *
> + * Registers a callback to be invoked when the connection
> + * is closed. This callback is invoked when there is any
> + * condition that causes the socket connection to the
> + * hypervisor to be closed.
> + *
> + * The @freecb must not invoke any other libvirt public
> + * APIs, since it is not called from a re-entrant safe
> + * context.
> + *
> + * Returns 0 on success, -1 on error
> + */
> +int virAdmConnectRegisterCloseCallback(virAdmConnectPtr conn,
> + virAdmConnectCloseFunc cb,
> + void *opaque,
> + virFreeCallback freecb)
> +{
> + VIR_DEBUG("conn=%p", conn);
> +
> + virResetLastError();
> +
> + virCheckAdmConnectReturn(conn, -1);
> +
> + virObjectRef(conn);
> +
> + virObjectLock(conn);
> + virObjectLock(conn->closeCallback);
> +
> + virCheckNonNullArgGoto(cb, error);
> +
> + if (conn->closeCallback->callback) {
> + 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;
> +
> + virObjectUnlock(conn->closeCallback);
> + virObjectUnlock(conn);
> +
> + return 0;
> +
> + error:
> + virObjectUnlock(conn->closeCallback);
> + virObjectUnlock(conn);
> + virDispatchError(NULL);
> + virObjectUnref(conn);
> + return -1;
> +
> +}
> +
> +/**
> + * virAdmConnectUnregisterCloseCallback:
> + * @conn: pointer to connection object
> + * @cb: pointer to the current registered callback
> + *
> + * Unregisters the callback previously set with the
> + * virAdmConnectRegisterCloseCallback method. The callback
> + * will no longer receive notifications when the connection
> + * closes. If a virFreeCallback was provided at time of
> + * registration, it will be invoked.
> + *
> + * Returns 0 on success, -1 on error
> + */
> +int virAdmConnectUnregisterCloseCallback(virAdmConnectPtr conn,
> + virAdmConnectCloseFunc cb)
> +{
> + VIR_DEBUG("conn=%p", conn);
> +
> + virResetLastError();
> +
> + virCheckAdmConnectReturn(conn, -1);
> +
> + virObjectLock(conn);
> + virObjectLock(conn->closeCallback);
> +
> + virCheckNonNullArgGoto(cb, error);
> +
> + if (conn->closeCallback->callback != 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;
> +
> + virObjectUnlock(conn->closeCallback);
> + virObjectUnlock(conn);
> + virObjectUnref(conn);
> +
> + return 0;
> +
> + error:
> + virObjectUnlock(conn->closeCallback);
> + virObjectUnlock(conn);
> + virDispatchError(NULL);
> + return -1;
> +}
> diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms
> index 16cfd42..5b9ba51 100644
> --- a/src/libvirt_admin_public.syms
> +++ b/src/libvirt_admin_public.syms
> @@ -16,4 +16,6 @@ LIBVIRT_ADMIN_1.3.0 {
> virAdmConnectClose;
> virAdmConnectRef;
> virAdmConnectIsAlive;
> + virAdmConnectRegisterCloseCallback;
> + virAdmConnectUnregisterCloseCallback;
> };
> diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> index 4964e3d..679ae99 100644
> --- a/tools/virt-admin.c
> +++ b/tools/virt-admin.c
> @@ -91,6 +91,10 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
> vshError(ctl, "%s", _("Failed to connect to the admin server"));
> return NULL;
> } else {
> + if (virAdmConnectRegisterCloseCallback(priv->conn, NULL, NULL,
> + NULL) < 0)
> + vshError(ctl, "%s", _("Unable to register disconnect callback"));
> +
This can never succeed. virADmConnectRegisterCloseCallback checks for
NULL cb. So after this you will always see the error.
> if (priv->wantReconnect)
> vshPrint(ctl, "%s\n", _("Reconnected to the admin server"));
> else
> @@ -108,6 +112,7 @@ vshAdmDisconnectInternal(vshControl *ctl,
> virAdmConnectPtr *conn)
> if (!*conn)
> return ret;
>
> + virAdmConnectUnregisterCloseCallback(*conn, NULL);
> ret = virAdmConnectClose(*conn);
> if (ret < 0)
> vshError(ctl, "%s", _("Failed to disconnect from the admin server"));
>
Otherwise looking good.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
