[libvirt] [PATCH] daemon: fixup refcounting in close callback handling

[Erik Skultety]

On 03/03/16 09:01, Nikolay Shirokovskiy wrote:
> remoteDispatchConnectCloseCallbackRegister introduced in
> f484310add53ebdc26a6fdcb88bc398750325b7e has problems. 

8-10 symbols should be enough for most projects :)

It refcounts
> network client object and in case of NOOP driver operations
> for close callback registering/unregistering (any driveres besides

s/driveres besides/driver except for

> vz) nobody will unref it later. As a result client connection
> will not be disposed and driver connection will not be closed.
> 
> The fix is easy. We don't need to refcount at all. We don't get
> dangling pointer because in remoteClientFreeFunc which is called
> upon disposing this network client object we unregistering close
> callback.
> 

s/unregistering/unregister the

Yes, exactly.

> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  daemon/remote.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 04d8ada..17783fa 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -3382,11 +3382,9 @@ remoteDispatchConnectCloseCallbackRegister(virNetServerPtr server ATTRIBUTE_UNUS
>          goto cleanup;
>      }
>  
> -    // on behalf of close callback
> -    virObjectRef(client);
>      if (virConnectRegisterCloseCallback(priv->conn,
>                                          remoteRelayConnectionClosedEvent,
> -                                        client, virObjectFreeCallback) < 0)
> +                                        client, NULL) < 0)

I'm sorry, I completely missed the client parameter yesterday, thus my
original patch I sent you, as you very correctly pointed out, couldn't
work, ever.

ACK, I slightly adjusted the commit message and pushed.

Regards,
Erik


>          goto cleanup;
>  
>      priv->closeRegistered = true;
>