Re: [libvirt] [PATCH 1/3] Domain events - primary implementation
On Wed, Oct 15, 2008 at 04:26:40PM -0400, Ben Guthro wrote:
I've been off implementing all of these changes, and I came
across a point I
would like to discuss, and get your opinion on.
...
> In fat I think this scheme ought to let you do away with the mutex
> locking completely. The contract for virConnectPtr dictates that
> you are forbidden to use a single virConnectPtr object from more than
> one thread at once, so if we're queueing & dispatching events from
> and timeout handler, we shouldn't ever get a reentrancy/locking
> problem.
We potentially have a race condition for pulling data off the wire by the following
functions:
call()
remoteDomainEventFired()
These 2 functions lock on not the connection lock, but the
private_data->lock.
Since the remoteDomainEventFired() is called from a client app
via HandleImpl - it has no knowledge of that the opaque pointer
being passed is a conn.
There is nothing constraining the EventImpl from making a callback
while using a conn ptr in another thread.
So - if that callback happens concurrent with an explicit use of
the conn ptr bad things will happen.
Our threading rule is that a single virConnectPtr must only ever be
used one from thread. So if the application which registers for async
events knows that it will be using the connection from a different
thread than the main loop, it is supposed to open a 2nd virConnectPtr
object to deal with this.
This is a slightly tedious restriction to have, but I don't see a way
around it other than explicitly doing the work to make it safe for a
single virConnectPtr to be used from many threads. This woud thread
that we add explicit locking into all the internal drivers. The work
I'm doing to make QEMU / libvirtd properly threaded is a step towards
this possibility, though not sufficient on its own.
So if I'm understanding your scenario I think we can just document that
its the app's responsibility to ensure the event loop isn't running in
a separate thread from the code doing synchronous method calls. Even
without events, apps like virt-manager have to handle this todo allow
things like save/restore to be done in background safely.
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 :|