On 06/27/2012 06:45 AM, Daniel P. Berrange wrote:
> If we are happy with double registration, then your patch looks
correct
> on the registration side (although it may be incomplete, if we mishandle
> double firing). On the other hand, if we want to prevent double
> registration, then you need to further forbid registration when ((dom &&
> !cbList->callbacks[i]->dom) || (!dom &&
cblist->callbacks[i]->dom)).
You are correct that this patch doesn't actually fix the problem
of registering handlers for multiple domains. The core issue is
that libvirtd tracks all registered callbacks, so that it can later
unregister them. But it only tracks 1 callback per event type. So
I don't see how this patch could have actually been tested.
Indeed the wire protocol does not even pass the 'virDomainPtr' over
the network, so this can't possibly work with a non-NULL domain;
In the case of qemu, there's two layers of handlers:
- libvirt.so registers a local handler (either global or per-domain),
and remembers which user callback to fire
- the remote hypervisor driver then sends an RPC to register a global
handler, but with the callback being a function in the remote driver
- when an event is triggered remotely, the global handler registered via
RPC sends an RPC event back
- libvirt.so then looks at the event (which _does_ have domain
information) and decides whether to fire the local callback(s) based on
whether the local callback is global or has a per-domain match
In other words, I see no reason why the current RPC design shouldn't
work; you can still have both global and per-domain callbacks from the
point of view of the application, even though the RPC side is using only
a single global callback (which is basically installed on a
reference-counting principle - if there is one or more local handlers,
then the global remote handler exists)
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org