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

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...

The same is true for remoteDispatchConnectDomainEventDeregister() remoteDispatchConnectDomainEventRegisterAny() remoteDispatchConnectDomainEventDeregisterAny() remoteDispatchConnectDomainEventCallbackRegisterAny() remoteDispatchConnectDomainEventCallbackDeregisterAny() remoteDispatchConnectNetworkEventRegisterAny() remoteDispatchConnectNetworkEventDeregisterAny() remoteDispatchConnectStoragePoolEventRegisterAny() remoteDispatchConnectStoragePoolEventDeregisterAny() remoteDispatchConnectNodeDeviceEventRegisterAny() remoteDispatchConnectNodeDeviceEventDeregisterAny() remoteDispatchConnectSecretEventRegisterAny() remoteDispatchConnectSecretEventDeregisterAny() qemuDispatchConnectDomainMonitorEventRegister() qemuDispatchConnectDomainMonitorEventDeregister() With either all of them updated, if my understanding of the situation as described above is correct, or left as is otherwise, Reviewed-by: Andrea Bolognani <abologna(a)redhat.com&gt;