Re: [libvirt] PATCH 0/5: connection cloning support (WIP)
Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver). I've added a little debug. Apparently event->name is being NULLed out sometime after the event is put on the queue: DEBUG: remote_internal.c: remoteDomainEventFired (Event fired 0 3 1 1) DEBUG: remote_internal.c: processCallRecv (Do 4 0) DEBUG: remote_internal.c: processCallRecvLen (Got length, now need 64 total (60 more)) DEBUG: remote_internal.c: processCallRecv (Do 64 4) DEBUG: remote_internal.c: processCallAsyncEvent (Encountered an event while waiting for a response) DEBUG: remote_internal.c: get_nonnull_domain (domain.name: dsl) DEBUG: datatypes.c: virGetDomain (New hash entry 0x804c728) DEBUG: domain_event.c: virDomainEventNew (event: 0x804c770 ->name: dsl) DEBUG: libvirt.c: virDomainFree (domain=0x804c728) DEBUG: datatypes.c: virUnrefDomain (unref domain 0x804c728 dsl 1) DEBUG: datatypes.c: virReleaseDomain (release domain 0x804c728 dsl) DEBUG: datatypes.c: virReleaseDomain (unref connection 0x804b040 4) DEBUG: domain_event.c: virDomainEventQueuePush (event: 0x804c770 ->name: dsl) DEBUG: remote_internal.c: processCallRecv (Do 0 0) DEBUG: remote_internal.c: remoteDomainEventQueueFlush () DEBUG: domain_event.c: virDomainEventDispatchDefaultFunc (event: 0x804c770 ->name: (null)) virGetDomain: name is NULL Segmentation fault I'll continue looking into it. But please let me know if you're familiar with the problem ... Thanks, Dave On Tue, 2008-12-16 at 10:24 -0500, David Lively wrote:
Hi Daniels - Sorry for the delay. Life's been crazy. But now I'm finally looking into using this series of patches with my Java event implementation (which I've now redone via the java.nio.Select mechanism; ready to submit pending resolution of the RPC concurrency issues). I should have some feedback for you later today ...
Thanks, Dave
On Tue, 2008-12-09 at 12:08 +0000, Daniel P. Berrange wrote:
This series is a work-in-progress set of patches implementing connection cloning. The idea is that if you want to use a connection form multiple threads, you could do
virConnectPtr copy = virConnectClone(conn)
and use 'copy' from the other thread. This avoids the problem of having to make all the virError handling stuff thread-local whic is the blocker for allowing real mutlti-thread access to a single virConnectPtr object.
I believe this cloning support should be sufficient for the Java bindings need to use a thread for its event loop. The idea being that if you wanted to use an event loop in a background thread, you'd create a cloned object for use event loop triggered callbacks like domain events notifications.
I'd still like to do some experiments later making the single virConnectPtr fully thread safe, but that'll take a little more time. I'm hoping this cloning will address the Java needs right now...
Daniel
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote:
Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver). I've added a little debug. Apparently event->name is being NULLed out sometime after the event is put on the queue:
DEBUG: remote_internal.c: remoteDomainEventFired (Event fired 0 3 1 1) DEBUG: remote_internal.c: processCallRecv (Do 4 0) DEBUG: remote_internal.c: processCallRecvLen (Got length, now need 64 total (60 more)) DEBUG: remote_internal.c: processCallRecv (Do 64 4) DEBUG: remote_internal.c: processCallAsyncEvent (Encountered an event while waiting for a response) DEBUG: remote_internal.c: get_nonnull_domain (domain.name: dsl) DEBUG: datatypes.c: virGetDomain (New hash entry 0x804c728) DEBUG: domain_event.c: virDomainEventNew (event: 0x804c770 ->name: dsl) DEBUG: libvirt.c: virDomainFree (domain=0x804c728) DEBUG: datatypes.c: virUnrefDomain (unref domain 0x804c728 dsl 1) DEBUG: datatypes.c: virReleaseDomain (release domain 0x804c728 dsl) DEBUG: datatypes.c: virReleaseDomain (unref connection 0x804b040 4) DEBUG: domain_event.c: virDomainEventQueuePush (event: 0x804c770 ->name: dsl) DEBUG: remote_internal.c: processCallRecv (Do 0 0) DEBUG: remote_internal.c: remoteDomainEventQueueFlush () DEBUG: domain_event.c: virDomainEventDispatchDefaultFunc (event: 0x804c770 ->name: (null)) virGetDomain: name is NULL Segmentation fault
I'll continue looking into it. But please let me know if you're familiar with the problem ...
I've not come across that specific problem, but there are a definitely some locking bugs & refcounting bugs inthe patches I've posted so far. I'll post an updated series of patches which may address this. 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 :|
On Wed, 2008-12-17 at 13:58 +0000, Daniel P. Berrange wrote:
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote:
Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver).
I've not come across that specific problem, but there are a definitely some locking bugs & refcounting bugs inthe patches I've posted so far. I'll post an updated series of patches which may address this.
Daniel
It turned out to be a double-free. Trivial fix below. Dave diff --git a/src/remote_internal.c b/src/remote_internal.c index 9245a2a..d6b94ff 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -6423,10 +6423,10 @@ remoteDomainQueueEvent(virConnectPtr conn, XDR *xdr) return; if (virDomainEventQueuePush(priv->domainEvents, - event) < 0) + event) < 0) { DEBUG0("Error adding event to queue"); - - virDomainEventFree(event); + virDomainEventFree(event); + } } /** remoteDomainEventFired:
Ok, with that fix, my asynchronous Java event implementation seems to be working (though not fully tested) with these patches. But I'm not using connection cloning (yet). It's your concurrency control in the remote driver that makes this work. On the Java side, domain event callbacks from another thread work because of the (Java) locking of Connect objects. I don't think I should remove that, since libvirt still prohibits concurrent use of the same virConnect object (right?) I suppose I could clone the virConnect object before delivering an event to a Java Domain (so the Domain's Connect would wrap the clone). I don't think this does much in the remote case (since the underlying RPC pipe it still shared, albeit safely). But I suppose it might allow greater concurrency in the non-remote cases. Is this what you had in mind? Dave On Wed, 2008-12-17 at 11:21 -0500, David Lively wrote:
On Wed, 2008-12-17 at 13:58 +0000, Daniel P. Berrange wrote:
On Wed, Dec 17, 2008 at 07:44:15AM -0500, David Lively wrote:
Hi Daniel - When I apply these patches, I'm seeing segfaults on event delivery when just running the existing synchronous examples/domain-events/events-c/event-test.c (using the remote driver).
I've not come across that specific problem, but there are a definitely some locking bugs & refcounting bugs inthe patches I've posted so far. I'll post an updated series of patches which may address this.
Daniel
It turned out to be a double-free. Trivial fix below.
Dave
diff --git a/src/remote_internal.c b/src/remote_internal.c index 9245a2a..d6b94ff 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -6423,10 +6423,10 @@ remoteDomainQueueEvent(virConnectPtr conn, XDR *xdr) return;
if (virDomainEventQueuePush(priv->domainEvents, - event) < 0) + event) < 0) { DEBUG0("Error adding event to queue"); - - virDomainEventFree(event); + virDomainEventFree(event); + } }
/** remoteDomainEventFired:
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Dec 17, 2008 at 01:24:11PM -0500, David Lively wrote:
Ok, with that fix, my asynchronous Java event implementation seems to be working (though not fully tested) with these patches.
But I'm not using connection cloning (yet). It's your concurrency control in the remote driver that makes this work.
That's good, but i just threw away the connection cloning patch. We're going to make asingle virConnectPtr object properly threadsafe. This will also mean you don't need 'synchronized' annotations on any java APis
On the Java side, domain event callbacks from another thread work because of the (Java) locking of Connect objects. I don't think I should remove that, since libvirt still prohibits concurrent use of the same virConnect object (right?)
Until 0.6.0..
I suppose I could clone the virConnect object before delivering an event to a Java Domain (so the Domain's Connect would wrap the clone). I don't think this does much in the remote case (since the underlying RPC pipe it still shared, albeit safely). But I suppose it might allow greater concurrency in the non-remote cases. Is this what you had in mind?
I vote for not support events in another thread in Java until 0.6.0, when we can remove al the limitations on thread usage. 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 :|
On Wed, Dec 17, 2008 at 11:21:34AM -0500, David Lively wrote:
It turned out to be a double-free. Trivial fix below.
Dave
diff --git a/src/remote_internal.c b/src/remote_internal.c index 9245a2a..d6b94ff 100644 --- a/src/remote_internal.c +++ b/src/remote_internal.c @@ -6423,10 +6423,10 @@ remoteDomainQueueEvent(virConnectPtr conn, XDR *xdr) return;
if (virDomainEventQueuePush(priv->domainEvents, - event) < 0) + event) < 0) { DEBUG0("Error adding event to queue"); - - virDomainEventFree(event); + virDomainEventFree(event); + } }
/** remoteDomainEventFired:
Thanks, I've committed this patch 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 :|
participants (2)
-
Daniel P. Berrange -
David Lively