Re: [libvirt] [PATCH] events: Don't fail on registering events for two different domains

On Wed, Jun 27, 2012 at 02:44:02PM +0200, Michal Privoznik wrote: > On 27.06.2012 14:36, Eric Blake wrote: >> On 06/27/2012 06:12 AM, Michal Privoznik wrote: >>> virConnectDomainEventRegisterAny() takes a domain as an argument. >>> So it should be possible to register the same event (be it >>> VIR_DOMAIN_EVENT_ID_LIFECYCLE for example) for two different domains. >>> That is, we need to take domain into account when searching for >>> duplicate event being already registered. >>> --- >>> src/conf/domain_event.c | 6 +++++- >>> 1 files changed, 5 insertions(+), 1 deletions(-) >>> >>> diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c >>> index 4ecc413..3cfd940 100644 >>> --- a/src/conf/domain_event.c >>> +++ b/src/conf/domain_event.c >>> @@ -363,7 +363,11 @@ virDomainEventCallbackListAddID(virConnectPtr conn, >>> for (i = 0 ; i < cbList->count ; i++) { >>> if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) && >>> cbList->callbacks[i]->eventID == eventID && >>> - cbList->callbacks[i]->conn == conn) { >>> + cbList->callbacks[i]->conn == conn && >>> + ((dom && cbList->callbacks[i]->dom && >>> + memcmp(cbList->callbacks[i]->dom->uuid, >>> + dom->uuid, VIR_UUID_BUFLEN) == 0) || >>> + (!dom && !cbList->callbacks[i]->dom))) { >> >> This misses the case of registering a catchall against NULL domain then >> attempting to re-register the same event against a specific domain >> (which one of the two would fire?). It also misses the case of >> registering a domain-specific handler, then attempting to broaden things >> into a global handler. > > Yes, but that's intentional. In both cases both events are fired. > >> >> I think the idea of double registration makes sense (in particular, if I >> have a per-domain callback, but then want to switch to global, I would >> rather have a window with both handlers at once than a window with no >> handler at all), but I have not tested whether we actually handle it by >> firing both the global and domain-specific callback. > > I've tested it and it works. How ? The remote driver does not ever pass the virDomainPtr arg over the wire, and it restricts itself to 1 single callback per event type. Only the client side drivers (test, esx, virtualbox) could possibly work, but not KVM, LXC, Xen, UML Daniel