[libvirt] [PATCHv2 07/14] event: don't let old-style events clobber per-domain events

[John Ferlan]

On 01/06/2014 05:27 PM, Eric Blake wrote:
> Right now, the older virConnectDomainEventRegister (takes a
> function pointer, returns 0 on success) and the newer
> virConnectDomainEventRegisterID (takes an eventID, returns a
> callbackID) share the underlying implementation (the older
> API ends up consuming a callbackID for eventID 0 under the
> hood).  We implemented that by a lot of copy and pasted
> code between object_event.c and domain_event.c, according to
> whether we are dealing with a function pointer or an eventID.
> However, our copy and paste is not symmetric.  Consider this
> sequence:
> 
> id1 = virConnectDomainEventRegisterAny(conn, dom,
>    VIR_DOMAIN_EVENT_ID_LIFECYCLE,
>    VIR_DOMAIN_EVENT_CALLBACK(callback), NULL, NULL);
> virConnectDomainEventRegister(conn, callback, NULL, NULL);
> virConnectDomainEventDeregister(conn, callback);
> virConnectDomainEventDeregsiterAny(conn, id1);
> 
> the first three calls would succeed, but the third call ended
> up nuking the id1 callbackID (the per-domain new-style handler),
> then the fourth call failed with an error about an unknown
> callbackID, leaving us with the global handler (old-style) still
> live and receiving events.  It required another old-style
> deregister to clean up the mess.  Root cause was that
> virDomainEventCallbackList{Remove,MarkDelete} were only
> checking for function pointer match, rather than also checking
> for whether the registration was global.
> 
> Rather than playing with the guts of object_event ourselves
> in domain_event, it is nicer to add a mapping function for the
> internal callback id, then share common code for event removal.
> For now, the function-to-id mapping is used only internally;
> I thought about whether a new public API to let a user learn
> the callback would be useful, but decided exposing this to the
> user is probably a disservice, since we already publicly
> document that they should avoid the old style, and since this
> patch already demonstrates that older libvirt versions have
> weird behavior when mixing old and new styles.
> 
> * src/conf/object_event.c (virObjectEventCallbackLookup)
> (virObjectEventStateCallbackID): New functions.
> (virObjectEventCallbackLookup): Use helper function.
> * src/conf/object_event_private.h (virObjectEventStateCallbackID):
> Declare new function.
> * src/conf/domain_event.c (virDomainEventStateRegister)
> (virDomainEventStateDeregister): Let common code handle the
> complexity.
> (virDomainEventCallbackListRemove)
> (virDomainEventCallbackListMarkDelete)
> (virDomainEventCallbackListAdd): Drop unused functions.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/conf/domain_event.c         | 171 ++++------------------------------------
>  src/conf/object_event.c         |  96 +++++++++++++++++++---
>  src/conf/object_event_private.h |   9 +++
>  3 files changed, 109 insertions(+), 167 deletions(-)
> 

ACK the pair (6 & 7)... The intro comment to 6 had me do a double take
though thinking I messed up the pair!

John