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

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

 



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@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]