Re: [libvirt] [PATCH 1/2] Java bindings for domain events
On Fri, 2008-11-21 at 14:49 +0100, Daniel Veillard wrote:
On Wed, Nov 19, 2008 at 11:22:31AM -0500, David Lively wrote:
> On Wed, 2008-11-19 at 10:35 -0500, David Lively wrote:
> > The patch already synchronizes operations using virConnect objects with
> > each other. To avoid making illegal EventImpl callbacks from Java for
> > the current libvirt, I have to lock every Connect object known to Java
> > and hold off creating new connections (via open & friends) around an
> > EventImpl callback. This sounds rather appalling to me, but it's
> > starting to sound like the only practical route in the short term
> > (unless it turns out we can rely on pthreads in WIN32 ...).
>
note that libxml2 that we rely on has a fully ported mutex basic
API
in libxml/threads.h
....
in case you really want to do the exclusive locking at the C level
while still being portable.
Thanks for the pointer. This could be used. I'd assume we'd want to
change the mutex usage in datatypes.c as well, for consistency.
Still it's probably better to try to implement most of this at
the
Java level, at least IMHO,
Just to be clear, I've been planning on keeping the per-Connect Java
synchronization from the original patch. (And I think you guys have
bought into that much, so far.)
So here we're discussing the concurrency requirements imposed upon the
callbacks made by an asynchronous external (non-libvirtd) EventImpl.
Currently, we must make sure that no other connection is in use (and no
other EventImpl callback is in progress).
I'm arguing this is far too restrictive since it basically means locking
ALL connections before an event can be recognized. So if one connection
is performing some long-running operation (say something
storage-related), no events can be delivered to *any* connection until
the operation completes (and then we'd better hope another long-running
operation hasn't been started on another connection in the mean time).
Keep in mind I'm not proposing we make all libvirt C public interfaces
threadsafe. We only need to make sure the paths reachable from external
event impls (in the remote and xen-inotify drivers) are threadsafe. I
believe the patch I submitted (2/2 in this series) does this for the
remote driver, albeit via the PTHREADS_MUTEX macros that currently have
no Win32 impl. (And I'll do the same for the xen-inotify driver once
it's in. I haven't looked into it yet ...)
I'll happily change the existing mutex usage in datatypes.c to the
libxml one. And I'll use the same mutex impl in the remote and
xen-inotify drivers. But I'd rather not bother until you guys agree
this is desirable ...
Thanks,
Dave
P.S. I realize this isn't really a practical problem right now. It's
more of a future limit on scalability. But because it's part of a new
interface (virEventRegisterImpl), it seems worth it to try and specify
this nicely now (as opposed to relaxing the concurrency requirements in
a later version, which is a kind of API change).