[libvirt] [PATCHv2 02/14] event: make deregister return value match docs

[John Ferlan]

On 01/06/2014 05:27 PM, Eric Blake wrote:
> Ever since their introduction (commit 1509b80 in v0.5.0 for
> virConnectDomainEventRegister, commit 4445723 in v0.8.0 for
> virConnectDomainEventDeregisterAny), the event deregistration
> functions have been documented as returning 0 on success;
> likewise for older registration (only the newer RegisterAny
> must return a non-zero callbackID).  And now that we are
> adding virConnectNetworkEventDeregisterAny for v1.2.1, it
> should have the same semantics.
> 
> Fortunately, all of the stateful drivers have been obeying
> the docs and returning 0, thanks to the way the remote_driver
> tracks things (in fact, the RPC wire protocol is unable to
> send a return value for DomainEventRegisterAny, at least not
> without adding a new RPC number).  Well, except for vbox,
> which was always failing, due to failure to set the return
> value to anything besides its initial -1 value.
> 
> But for local drivers, such as test:///default, we've been
> returning non-zero numbers; worse, the non-zero numbers have
> differed over time.  For example, in Fedora 12 (libvirt 0.8.2),
> calling Register twice would return 0 and 1 [the callbackID
> generated under the hood]; while in Fedora 20 (libvirt 1.1.3),
> it returns 1 and 2 [the number of callbacks registered for
> that event type].  Since we have changed the behavior over
> time, and since it differs by local vs. remote, we can safely
> argue that no one could have been reasonably relying on any
> particular behavior, so we might as well obey the docs, as well
> as prepare callers that might deal with older clients to not be
> surprised if the docs are not strictly followed.
> 
> For consistency, this patch fixes the code for all drivers,
> even though it only makes an impact for vbox and for local
> drivers.  By fixing all drivers, future copy and paste from
> a remote driver to a local driver is less likely to
> reintroduce the bug.
> 
> * src/libvirt.c (virConnectDomainEventRegister)
> (virConnectDomainEventDeregister)
> (virConnectDomainEventDeregisterAny): Clarify docs.
> * src/libxl/libxl_driver.c (libxlConnectDomainEventRegister)
> (libxlConnectDomainEventDeregister)
> (libxlConnectDomainEventDeregisterAny): Match documentation.
> * src/lxc/lxc_driver.c (lxcConnectDomainEventRegister)
> (lxcConnectDomainEventDeregister)
> (lxcConnectDomainEventDeregisterAny): Likewise.
> * src/test/test_driver.c (testConnectDomainEventRegister)
> (testConnectDomainEventDeregister)
> (testConnectDomainEventDeregisterAny)
> (testConnectNetworkEventDeregisterAny): Likewise.
> * src/uml/uml_driver.c (umlConnectDomainEventRegister)
> (umlConnectDomainEventDeregister)
> (umlConnectDomainEventDeregisterAny): Likewise.
> * src/vbox/vbox_tmpl.c (vboxConnectDomainEventRegister)
> (vboxConnectDomainEventDeregister)
> (vboxConnectDomainEventDeregisterAny): Likewise.
> * src/xen/xen_driver.c (xenUnifiedConnectDomainEventRegister)
> (xenUnifiedConnectDomainEventDeregister)
> (xenUnifiedConnectDomainEventDeregisterAny): Likewise.
> * src/network/bridge_driver.c
> (networkConnectNetworkEventDeregisterAny): Likewise.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/libvirt.c               | 17 +++++++++++------
>  src/libxl/libxl_driver.c    | 35 ++++++++++++++++++-----------------
>  src/lxc/lxc_driver.c        | 30 +++++++++++++++---------------
>  src/network/bridge_driver.c | 11 +++++++----
>  src/test/test_driver.c      | 38 +++++++++++++++++++++-----------------
>  src/uml/uml_driver.c        | 29 ++++++++++++++++-------------
>  src/vbox/vbox_tmpl.c        | 32 ++++++++++++++++++++++----------
>  src/xen/xen_driver.c        | 25 ++++++++++++++-----------
>  8 files changed, 124 insertions(+), 93 deletions(-)
> 

Since they're a match set...  I'll ACK 1 & 2 together.


I was going to comment about vbox_tmpl.c, but some things are just
better left alone :-)


John