On Tue, Nov 18, 2008 at 11:18:38AM -0500, David Lively wrote: > This patch allows the remote driver to work with an asynchronous > EventImpl (it's the only one using an externally-supplied one), assuming > libvirt is compiled with pthread support. (Without pthreads, this code > is harmless in a single-threaded environment.) I don't like this patch, since it is only making one tiny part of the API thread-safe, for one tiny use case. eg, if someone uses events from the Xen driver in Java with threads, they'll crash & burn, since only the remote driver is being protected. IMHO, if we want to allow multi-threaded access to a single virConnectPtr object then we need to do the full job and make the whole thing safe, including adding thread-local error objects, to replace the inherantly non-thread safe global & per-virConnect error objects. The other alternative that we've discussed in the past is to add the ability to 'clone' an existing virConnectPtr object. The idea being that a 2nd thread could be given a clone, and safely use that. Internally we'd share & synchronize on relevant state information, so we didn't actually need to setup a separate network connection & re-auth for each. As it stands, this patch adding semi-thread safe access will make it harder for us to pursue this 2nd option of cloning virConnect, without breaking the Java bindings in future. > Basically it uses a mutex to protect reads from the RPC socket in such a > way that message reads (in their entirety) are done atomically > (otherwise the remoteDomainEventFired() can race the call() code that > reads replies & events). Why doesn't the Java code simply synchronize on the virConnect object before invoking the FD event callbacks. That will ensure another thread has finished whatever API call it was doing > In addition, I update the EventImpl handle to prevent > remoteDomainEventFired() from being called everytime a reply is sent. > (This helps us dispatch events in a timely manner, though it's not > strictly necessary. Without it, any events coming in during a call() > won't be dispatched until the call drops the socket lock (because > remoteDomainEventFired() will be stuck awaiting the lock). I don't really like the idea of applying any of this patch, since I think it'll cause us unexpected complications when doing proper thread support in the next release of libvirt, and risk us breaking the java bindings. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list