[libvirt] [PATCH 2/2] Java bindings for domain events

[Daniel P. Berrange]

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