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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org