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

[Daniel P. Berrange]

On Wed, Jun 27, 2012 at 02:51:43PM +0200, Michal Privoznik wrote:
> On 27.06.2012 14:46, Daniel P. Berrange wrote:
> > 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
> > 
> 
> I am attaching the reproducer. My output:
> 
> zippy at bart ~/work/tmp $ gcc repro.c -o repro -lvirt -ggdb3 && LD_LIBRARY_PATH="/home/mprivozn/work/libvirt/libvirt.git/src/.libs/" ./repro qemu:///system
> myDomainEventCallback2 EVENT: Domain f16(-1) Defined Updated o:cb1
> myDomainEventCallback2 EVENT: Domain f17(-1) Defined Updated o:cb1
> myDomainEventCallback2 EVENT: Domain f17(-1) Defined Updated o:cb2
> 
> So we can see cb1 and cb2 firing up at once (cb1 is registered against NULL, cb2 against f17 domain).

Ah, I see what's going on. The remote client is only registering a
single handler with the server, always with domain==NULL. The
filtering of events to individual domains & dispatch of multiple
handlers is then done completely client side.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|