[libvirt] [PATCH libvirt v2 9/9] remote: shrink the critical sections

John Ferlan jferlan at redhat.com
Thu Apr 26 15:08:32 UTC 2018



On 04/12/2018 08:41 AM, Marc Hartmayer wrote:
> No lock is required to access 'priv->conn' therefore we can shrink the
> critical sections.
> 
> Signed-off-by: Marc Hartmayer <mhartmay at linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
> ---
>  src/remote/remote_daemon_dispatch.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

While this follows other usages, that means virMutexUnlock in cleanup:
may be called without holding the lock.

Perhaps this and other *Register/*Deregister API's should take the lock
around anything that could end up in cleanup - perhaps even the same
history as the previous patch

John

> diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
> index 041e7c7440bf..d6699418392e 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -3841,8 +3841,6 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server,
>          virNetServerClientGetPrivateData(client);
>      virNetServerProgramPtr program;
>  
> -    virMutexLock(&priv->lock);
> -
>      if (!priv->conn) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
>          goto cleanup;
> @@ -3853,6 +3851,8 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server,
>          goto cleanup;
>      }
>  
> +    virMutexLock(&priv->lock);
> +
>      if (VIR_ALLOC(callback) < 0)
>          goto cleanup;
>      callback->client = virObjectRef(client);
> @@ -3887,13 +3887,13 @@ remoteDispatchConnectUnregisterCloseCallback(virNetServerPtr server ATTRIBUTE_UN
>      struct daemonClientPrivate *priv =
>          virNetServerClientGetPrivateData(client);
>  
> -    virMutexLock(&priv->lock);
> -
>      if (!priv->conn) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
>          goto cleanup;
>      }
>  
> +    virMutexLock(&priv->lock);
> +
>      if (virConnectUnregisterCloseCallback(priv->conn,
>                                            remoteRelayConnectionClosedEvent) < 0)
>          goto cleanup;
> 




More information about the libvir-list mailing list