[libvirt] [PATCH] Fix event-test segfault for mixed remote/local driver usage

[Daniel P. Berrange]

On Thu, Jan 14, 2010 at 03:19:06AM +0100, Matthias Bolte wrote:
> If examples/domain-events/events-c/event-test.c uses a local driver
> for the hyperviror and a remote driver for storage then the segfault
> occurs in remoteDomainEventDispatchFunc because conn->privateData is
> used.
> 
> The problem is as follows. The doRemoteOpen function registers
> remoteDomainEventFired as event handle and remoteDomainEventQueueFlush
> as event timeout. The doRemoteOpen function is used in remoteOpen and
> remoteOpenSecondaryDriver. So both handlers are registered for all
> types of remote drivers, but remoteDomainEventQueueFlush always uses
> conn->privateData. That's wrong when remoteDomainEventQueueFlush is
> called for any other driver than the hypervisor driver, it should use
> conn->storagePrivateData for the storage driver for example.
> 
> In the common case this doesn't result in a segfault because in the
> common case all drivers for a connection are either all remote or all
> local and conn->privateData and conn->storagePrivateData are the same.
> But in the local Xen case ('event-test xen:///' executed in Domain-0,
> with a libvirtd running in Domain-0) the hypervisor driver is local
> and the storage driver is remote. This results in a mismatch between
> conn->privateData and conn->storagePrivateData. Now the call to
> remoteDomainEventQueueFlush in the context of the storage driver
> results in a segfault, because it assumes that conn->privateData points
> to remote driver private data, but it points to Xen driver private
> data.

The big unanswered question here is why is the remote driver receiving
any events at all in the first place ?  Only the domain APIs emit 
events, and so those will all be coming out via the primary Xen driver.
So even if the xen:/// URI driver uses the remote driver a secondary
driver, it should never be receiving any events.

> 
> To fix this, the pointer to the private driver data gets passed as
> opaque parameter instead of the connection pointer. Because some
> functions called by the event handler functions need the connection
> pointer for other purposes than error reporting the connection
> pointer is passed as member of the private remote driver data struct.
> 
> Now remoteDomainEventFired and remoteDomainEventQueueFlush have the
> correct private data pointer for the respective calling context.

My inclination would be to skip registering the timer for the
remoteDomainEventQueueFlush method. Then inthe code which recieves
events, explicitly throw them away if eventFlushTimer == -1, and
log a warning message about unexpected events.

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