Re: [libvirt] [PATCH 35/41] remote: change hand written methods to not directly access connection

On Sun, Jul 28, 2019 at 08:19:40PM +0200, Andrea Bolognani wrote: > On Tue, 2019-07-23 at 17:03 +0100, Daniel P. Berrangé wrote: > [...] > > +++ b/src/remote/remote_daemon_dispatch.c > > @@ -4210,14 +4128,13 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED > > daemonClientEventCallbackPtr ref; > > struct daemonClientPrivate *priv = > > virNetServerClientGetPrivateData(client); > > - > > - if (!priv->conn) { > > - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); > > - goto cleanup; > > - } > > + virConnectPtr conn = remoteGetHypervisorConn(client); > > > > virMutexLock(&priv->lock); > > > > + if (!conn) > > + goto cleanup; > > + > > Shouldn't this be *before* the virMutexLock() call? As far as I can > tell, that would match the existing behavior... Looking at this I think the original code is broken. The "cleanup:" label calls virMutexUnlock(). So the original code was jumping to the cleanup label with an unlocked mutex and then unlocking it again.