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.
> 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 <[email protected]>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
