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). -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list