Re: [libvirt] [PATCH 2/2] Java bindings for domain events
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 :|