[libvirt] [PATCH v2 1/5] Add public API to register a callback to be invoked on connection close

[Jiri Denemark]

On Tue, Jul 24, 2012 at 14:17:01 +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Define new virConnect{Register,Unregister}CloseCallback() public APIs
> which allows registering/unregistering a callback to be invoked when
> the connection to a hypervisor is closed. The callback is provided
> with the reason for the close, which may be 'error', 'eof' or
> 'keepalive'.

The virConnectCloseReason enum also mentions 'client' reason.

> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index fcef461..d9f0397 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
...
> @@ -1148,6 +1166,27 @@ int virConnectSetKeepAlive(virConnectPtr conn,
>                             int interval,
>                             unsigned int count);
>  
> +typedef enum {
> +    VIR_CONNECT_CLOSE_REASON_ERROR     = 0, /* Misc I/O error */
> +    VIR_CONNECT_CLOSE_REASON_EOF       = 1, /* End-of-file from server */
> +    VIR_CONNECT_CLOSE_REASON_KEEPALIVE = 2, /* Keepalive timer triggered */
> +    VIR_CONNECT_CLOSE_REASON_CLIENT    = 3, /* Client requested it */
> +
> +# ifdef VIR_ENUM_SENTINELS
> +    VIR_CONNECT_CLOSE_REASON_LAST
> +# endif
> +} virConnectCloseReason;
...
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 8315b4f..9cfabc5 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -18613,6 +18613,117 @@ error:
>  
>  
>  /**
> + * virConnectRegisterCloseCallback:
> + * @conn: pointer to connection object
> + * @cb: callback to invoke upon 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.
> + *
> + * This function is only applicable to hypervisor drivers
> + * which maintain a persistent open connection. Drivers
> + * which open a new connection for every operation will
> + * not invoke this.
> + *
> + * 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 virConnectRegisterCloseCallback(virConnectPtr conn,
> +                                    virConnectCloseFunc cb,
> +                                    void *opaque,
> +                                    virFreeCallback freecb)
> +{
> +    VIR_DEBUG("conn=%p", conn);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECT(conn)) {
> +        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +
> +    virMutexLock(&conn->lock);
> +
> +    virCheckNonNullArgGoto(cb, error);
> +
> +    if (conn->closeCallback) {
> +        virLibConnError(VIR_ERR_OPERATION_INVALID, "%s",
> +                        _("A close callback is already registered"));
> +        goto error;
> +    }
> +
> +    conn->closeCallback = cb;
> +    conn->closeOpaque = opaque;
> +    conn->closeFreeCallback = freecb;
> +
> +    virMutexUnlock(&conn->lock);
> +
> +    return 0;
> +
> +error:
> +    virMutexUnlock(&conn->lock);
> +    virDispatchError(NULL);
> +    return -1;
> +}
> +
> +/**
> + * virConnectUnregisterCloseCallback:
> + * @conn: pointer to connection object
> + * @cb: pointer to the current registered callback
> + *
> + * Unregisters the callback previously set with the
> + * virConnectRegisterCloseCallback method. The callback
> + * will no longer received notifications when the connection

s/received/receive/

> + * closes.

Is it worth noting that freecb() is called on opaque or is it obvious enough?

> + *
> + * Returns 0 on success, -1 on error
> + */
> +int virConnectUnregisterCloseCallback(virConnectPtr conn,
> +                                      virConnectCloseFunc cb)
> +{
> +    VIR_DEBUG("conn=%p", conn);
> +
> +    virResetLastError();
> +
> +    if (!VIR_IS_CONNECT(conn)) {
> +        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> +        virDispatchError(NULL);
> +        return -1;
> +    }
> +
> +    virMutexLock(&conn->lock);
> +
> +    virCheckNonNullArgGoto(cb, error);
> +
> +    if (conn->closeCallback != cb) {
> +        virLibConnError(VIR_ERR_OPERATION_INVALID, "%s",
> +                        _("A different callback was requested"));
> +        goto error;
> +    }
> +
> +    conn->closeCallback = NULL;
> +    if (conn->closeFreeCallback)
> +        conn->closeFreeCallback(conn->closeOpaque);

I'd also reset closeFreeCallback and closeOpaque to NULL to avoid accessing
them by mistake in the future.

> +
> +    virMutexUnlock(&conn->lock);
> +
> +    return 0;
> +
> +error:
> +    virMutexUnlock(&conn->lock);
> +    virDispatchError(NULL);
> +    return -1;
> +}
> +
> +/**
>   * virDomainSetBlockIoTune:
>   * @dom: pointer to domain object
>   * @disk: path to the block device, or device shorthand
...

ACK with nits fixed or ignored as not being worth it.

Jirka