On 12/13/2011 05:38 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list