[libvirt] [PATCH v3 4/5] Update remote driver to support the connection close callbacks

[Jiri Denemark]

On Fri, Jul 27, 2012 at 11:39:55 +0100, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Update the remote driver to use the virNetClient close callback
> to trigger the virConnectPtr close callbacks
> 
> Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> ---
>  src/datatypes.h            |    2 ++
>  src/libvirt.c              |    3 ++-
>  src/remote/remote_driver.c |   31 +++++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/src/datatypes.h b/src/datatypes.h
> index 8ac9171..a303cdc 100644
> --- a/src/datatypes.h
> +++ b/src/datatypes.h
> @@ -191,6 +191,8 @@ struct _virConnect {
>      virConnectCloseFunc closeCallback;
>      void *closeOpaque;
>      virFreeCallback closeFreeCallback;
> +    bool closeDispatch;
> +    unsigned closeUnregisterCount;
>  
>      int refs;                 /* reference count */
>  };
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 160ace7..d352cfd 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -18711,7 +18711,8 @@ int virConnectUnregisterCloseCallback(virConnectPtr conn,
>      }
>  
>      conn->closeCallback = NULL;
> -    if (conn->closeFreeCallback)
> +    conn->closeUnregisterCount++;
> +    if (!conn->closeDispatch && conn->closeFreeCallback)
>          conn->closeFreeCallback(conn->closeOpaque);
>      conn->closeFreeCallback = NULL;
>      conn->closeOpaque = NULL;
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index a69e69a..9ac27b2 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -320,6 +320,32 @@ enum virDrvOpenRemoteFlags {
>  };
>  
>  
> +static void remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED,
> +                                  int reason,
> +                                  void *opaque)
> +{
> +    virConnectPtr conn = opaque;
> +
> +    virMutexLock(&conn->lock);
> +    if (conn->closeCallback) {
> +        virConnectCloseFunc closeCallback = conn->closeCallback;
> +        void *closeOpaque = conn->closeOpaque;
> +        virFreeCallback closeFreeCallback = conn->closeFreeCallback;
> +        unsigned closeUnregisterCount = conn->closeUnregisterCount;
> +
> +        VIR_DEBUG("Triggering connection close callback %p reason=%d",
> +                  conn->closeCallback, reason);
> +        conn->closeDispatch = true;
> +        virMutexUnlock(&conn->lock);
> +        closeCallback(conn, reason, closeOpaque);
> +        virMutexLock(&conn->lock);
> +        conn->closeDispatch = false;
> +        if (conn->closeUnregisterCount != closeUnregisterCount)
> +            closeFreeCallback(closeOpaque);
> +    }
> +    virMutexUnlock(&conn->lock);
> +}
> +
>  /*
>   * URIs that this driver needs to handle:
>   *

Looks good. However, do we need to protect against the following scenario:

- remoteClientCloseFunc locks conn, remembers stuff, unlocks conn
- virConnectUnregisterCloseCallback
- virConnectRegisterCloseCallback
- virConnectUnregisterCloseCallback
- remoteClientCloseFunc finishes

Now the second registered callback won't be unregistered. It doesn't seem like
something that could happen in the real world, though.

Jirka