[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] Java bindings for domain events



On Fri, Nov 21, 2008 at 03:27:49PM -0500, David Lively wrote:
> 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:
> 
> >   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).

To me this is a killer argument for virConnectPtr being made threadsafe.
We're creating problems for ourselves by trying to hack around it.

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

I am :-) 

The common "design pattern" is java is heavily multi-threaded and 
doing a half-way house solution which makes only some scenrios in
the API thread-safe is still going to leave the Java libvirt API 
limited in comparison with native Java APIs & desirable application
use cases. 

We already have no choice but to make the QEMU, LXC, UML and OpenVZ
drivers fully threadsafe in order to allow the libvirtd daemon to be
threaded for sake of scability, and I have more or less completed
this work already. Making the remaining Xen driver  thread-safe too 
is trivial by comparison, since it has very little state. The main 
remaining issue is the global & per-connection error variables which
are not thread safe. If we added a 3rd thread-local variable which 
was set in parallel with these existing ones we could have a properly
threadsafe API, and simply say don't access the global error objects
if using the API in a multi-thread context.

This isn't just a problem for Java either - we jump through some 
horrible hoops in virt-manger python code because of the thread 
restrictions in the our public API.

The only alternative to a fully threaded API is a the idea of adding
a virConnectClone() operation which gives you a duplicate handled on
to the existing connection for use in a separate thread. The more I
consider this though, the less useful it seems - we'd still have the
same issues with the global error object, and we'd have todo internal
synchronization which is just as complex as the fully threadsafe
work. 

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

The datatypes.c code used to use the libxml mutex definitions but
I removed them because it was not a complete solution. We need to have
more than just the mutex datatypes & libxml does not provide wrappers
for all the thread APIs & data types we require, so we have no choice
but to use proper pthread types, and a pthreads API library on Win32)
Rich Jones has already packaged up such a library for Win32 in Fedora:

http://annexia.org/tmp/mingw/fedora-10/src/SRPMS/mingw32-pthreads-2.8.0-2.fc10.src.rpm

> 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).

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]