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

[Eric Blake]

On 07/19/2012 09:04 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Define a new virConnectSetCloseCallback() public API which allows
> registering 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'.
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  include/libvirt/libvirt.h.in |   40 +++++++++++++++++++++++--------
>  src/datatypes.c              |    4 ++++
>  src/datatypes.h              |    5 ++++
>  src/libvirt.c                |   53 ++++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms      |    6 +++++
>  5 files changed, 98 insertions(+), 10 deletions(-)

>  
> +typedef enum {
> +    VIR_CONNECT_CLOSE_REASON_ERROR     = 1, /* Misc I/O error */

Any reason you skipped 0?  It will make wrapping this in a VIR_ENUM harder.


> +int virConnectSetCloseCallback(virConnectPtr conn,
> +                               virConnectCloseFunc cb,
> +                               void *opaque,
> +                               virFreeCallback freecb);

Do we want a 'flags' argument?

> +++ b/src/datatypes.c
> @@ -115,6 +115,10 @@ virReleaseConnect(virConnectPtr conn) {
>  
>      virMutexLock(&conn->lock);
>  
> +    if (conn->closeOpaque &&

NACK to this half of the condition.  A client should be allowed to pass
NULL as their opaque data.

> +int virConnectSetCloseCallback(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);
> +
> +    if (conn->closeOpaque &&
> +        conn->closeFreeCallback)
> +        conn->closeFreeCallback(conn->closeOpaque);

Again, no need to check closeOpaque, NULL is valid there.

> +
> +    conn->closeCallback = cb;
> +    conn->closeOpaque = opaque;
> +    conn->closeFreeCallback = freecb;

So the user can call this as many times as they want to override or
uninstall an existing registered callback?  I guess that works.

ACK, once you allow a NULL opaque pointer, and answer why a flags is not
useful (or else add a flags).

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120719/d7d0e57f/attachment-0001.sig>