[libvirt] [PATCH 7/8] Hide use of timers for domain event dispatch

[Eric Blake]

On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange at redhat.com>
> 
> Currently all drivers using domain events need to provide a callback
> for handling a timer to dispatch events in a clean stack. There is
> no technical reason for dispatch to go via driver specific code. It
> could trivially be dispatched directly from the domain event code,
> thus removing tedious boilerplate code from all drivers

I think the couple hunks I mentioned in 4/8 remote_driver.c belong in here.

> 
> * src/conf/domain_event.c, src/conf/domain_event.h: Internalize
>   dispatch of events from timer callback
> * src/libxl/libxl_driver.c, src/lxc/lxc_driver.c,
>   src/qemu/qemu_domain.c, src/qemu/qemu_driver.c,
>   src/remote/remote_driver.c, src/test/test_driver.c,
>   src/uml/uml_driver.c, src/vbox/vbox_tmpl.c,
>   src/xen/xen_driver.c: Remove all timer dispatch functions
> ---
>  src/conf/domain_event.c    |   86 +++++++++++++++++++++++++++++---------------
>  src/conf/domain_event.h    |   32 +----------------
>  src/libxl/libxl_driver.c   |   30 +---------------
>  src/lxc/lxc_driver.c       |   33 +----------------
>  src/qemu/qemu_domain.c     |   25 -------------
>  src/qemu/qemu_driver.c     |    5 +--
>  src/remote/remote_driver.c |   37 +------------------
>  src/test/test_driver.c     |   32 +----------------
>  src/uml/uml_driver.c       |   33 +----------------
>  src/vbox/vbox_tmpl.c       |   45 +----------------------
>  src/xen/xen_driver.c       |   40 +--------------------
>  11 files changed, 66 insertions(+), 332 deletions(-)

More impressive deletion numbers.

> @@ -1230,10 +1244,11 @@ void virDomainEventDispatch(virDomainEventPtr event,
>  }
>  
>  
> -void virDomainEventQueueDispatch(virDomainEventQueuePtr queue,
> -                                 virDomainEventCallbackListPtr callbacks,
> -                                 virDomainEventDispatchFunc dispatch,
> -                                 void *opaque)
> +static void
> +virDomainEventQueueDispatch(virDomainEventQueuePtr queue,

In most cases, you changed things to start the function name in column 1
(with the return type in previous line) - I actually like this style.

> @@ -1266,10 +1281,23 @@ virDomainEventStateQueue(virDomainEventStatePtr state,
>      virDomainEventStateUnlock(state);
>  }
>  
> -void
> -virDomainEventStateFlush(virDomainEventStatePtr state,
> -                         virDomainEventDispatchFunc dispatchFunc,
> -                         void *opaque)
> +
> +static void virDomainEventStateDispatchFunc(virConnectPtr conn,

...but here's a case where the diff makes it look at first glance like
you converted in the opposite direction.  A closer look shows that you
kept the layout of virDomainEventStateFlush intact, but added a new
function, with the new function not using consistent layout, but it
would still be worth consistent layout here.

> @@ -952,11 +928,7 @@ libxlStartup(int privileged) {
>      }
>      VIR_FREE(log_file);
>  
> -    libxl_driver->domainEventState = virDomainEventStateNew(
> -                                                        libxlDomainEventFlush,
> -                                                        libxl_driver,
> -                                                        NULL,
> -                                                        false);
> +    libxl_driver->domainEventState = virDomainEventStateNew(true);

Why the change from false to true?

> @@ -326,10 +325,7 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr auth, unsigned int flags)
>          return VIR_DRV_OPEN_ERROR;
>      }
>  
> -    if (!(priv->domainEvents = virDomainEventStateNew(xenDomainEventFlush,
> -                                                      priv,
> -                                                      NULL,
> -                                                      NULL))) {
> +    if (!(priv->domainEvents = virDomainEventStateNew(true))) {

Why the change from false (once you fix my comment in 1/8) to true?

ACK; the above are nits, and the bool parameter goes away in 8/8 (but it
would be worth having this one be correct in the meantime).

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111214/5c8b257e/attachment-0001.sig>