[libvirt] [PATCH] events: Return the correct number of registered events

Commit d09f6ba5feb655925175dc80122ca2a1e14db2b9 introduced a regression in event registration. virDomainEventCallbackListAddID() will only return a positive integer if the type of event being registered is VIR_DOMAIN_EVENT_ID_LIFECYCLE. For other event types, 0 is always returned on success. This has the unfortunate side effect of not enabling remote event callbacks because remoteDomainEventRegisterAny() uses the return value from the local call to determine if an event callback needs to be registered on the remote end. Make sure virDomainEventCallbackListAddID() returns the callback count for the eventID being registered. Signed-off-by: Adam Litke <agl@us.ibm.com> --- src/conf/domain_event.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 3fd3ed2..0431697 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -434,7 +434,7 @@ virDomainEventCallbackListAddID(virConnectPtr conn, event->callbackID = cbList->nextID++; for (i = 0 ; i < cbList->count ; i++) { - if (cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE && + if (cbList->callbacks[i]->eventID == eventID && cbList->callbacks[i]->conn == conn && !cbList->callbacks[i]->deleted) ret++; -- 1.7.5.rc1

On 01/13/2012 01:44 PM, Adam Litke wrote:
Commit d09f6ba5feb655925175dc80122ca2a1e14db2b9 introduced a regression in event registration. virDomainEventCallbackListAddID() will only return a positive integer if the type of event being registered is VIR_DOMAIN_EVENT_ID_LIFECYCLE. For other event types, 0 is always returned on success. This has the unfortunate side effect of not enabling remote event callbacks because remoteDomainEventRegisterAny() uses the return value from the local call to determine if an event callback needs to be registered on the remote end.
Make sure virDomainEventCallbackListAddID() returns the callback count for the eventID being registered.
Signed-off-by: Adam Litke <agl@us.ibm.com> --- src/conf/domain_event.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 3fd3ed2..0431697 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -434,7 +434,7 @@ virDomainEventCallbackListAddID(virConnectPtr conn, event->callbackID = cbList->nextID++;
for (i = 0 ; i < cbList->count ; i++) { - if (cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE && + if (cbList->callbacks[i]->eventID == eventID &&
Oh my, good catch, but incomplete. There were four instances in this file of hard-coding the comparison to VIR_DOMAIN_EVENT_ID_LIFECYCLE, but the other two were in static functions only called from locations that operate solely on lifecycle events. I'm squashing this in before pushing: diff --git i/src/conf/domain_event.c w/src/conf/domain_event.c index 0431697..1d8b45d 100644 --- i/src/conf/domain_event.c +++ w/src/conf/domain_event.c @@ -397,7 +397,7 @@ virDomainEventCallbackListAddID(virConnectPtr conn, /* check if we already have this callback on our list */ for (i = 0 ; i < cbList->count ; i++) { if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) && - cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE && + cbList->callbacks[i]->eventID == eventID && cbList->callbacks[i]->conn == conn) { eventReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("event callback already tracked")); -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Fri, Jan 13, 2012 at 02:00:31PM -0700, Eric Blake wrote:
On 01/13/2012 01:44 PM, Adam Litke wrote:
Commit d09f6ba5feb655925175dc80122ca2a1e14db2b9 introduced a regression in event registration. virDomainEventCallbackListAddID() will only return a positive integer if the type of event being registered is VIR_DOMAIN_EVENT_ID_LIFECYCLE. For other event types, 0 is always returned on success. This has the unfortunate side effect of not enabling remote event callbacks because remoteDomainEventRegisterAny() uses the return value from the local call to determine if an event callback needs to be registered on the remote end.
Make sure virDomainEventCallbackListAddID() returns the callback count for the eventID being registered.
Signed-off-by: Adam Litke <agl@us.ibm.com> --- src/conf/domain_event.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 3fd3ed2..0431697 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -434,7 +434,7 @@ virDomainEventCallbackListAddID(virConnectPtr conn, event->callbackID = cbList->nextID++;
for (i = 0 ; i < cbList->count ; i++) { - if (cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE && + if (cbList->callbacks[i]->eventID == eventID &&
Oh my, good catch, but incomplete. There were four instances in this file of hard-coding the comparison to VIR_DOMAIN_EVENT_ID_LIFECYCLE, but the other two were in static functions only called from locations that operate solely on lifecycle events. I'm squashing this in before pushing:
diff --git i/src/conf/domain_event.c w/src/conf/domain_event.c index 0431697..1d8b45d 100644 --- i/src/conf/domain_event.c +++ w/src/conf/domain_event.c @@ -397,7 +397,7 @@ virDomainEventCallbackListAddID(virConnectPtr conn, /* check if we already have this callback on our list */ for (i = 0 ; i < cbList->count ; i++) { if (cbList->callbacks[i]->cb == VIR_DOMAIN_EVENT_CALLBACK(callback) && - cbList->callbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE && + cbList->callbacks[i]->eventID == eventID && cbList->callbacks[i]->conn == conn) { eventReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("event callback already tracked"));
ACK 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 :|
participants (3)
-
Adam Litke
-
Daniel P. Berrange
-
Eric Blake