On 03/03/2011 07:21 AM, Daniel P. Berrange wrote:
> Not all applications have an existing event loop they need
> to integrate with. Forcing them to implement the libvirt
> event loop integration APIs is an undue burden. This just
> exposes our simple poll() based implementation for apps
> to use. So instead of calling
>
> virEventRegister(....callbacks...)
>
> The app would call
>
> virEventRegisterDefaultImpl()
>
> And then have a thread somewhere calling
>
> static bool quit = false;
> ....
> while (!quit)
> virEventRunDefaultImpl()
>
> * daemon/libvirtd.c, tools/console.c,
> tools/virsh.c: Convert to public event loop APIs
> * include/libvirt/libvirt.h.in, src/libvirt_private.syms: Add
> virEventRegisterDefaultImpl and virEventRunDefaultImpl
> * src/util/event.c: Implement virEventRegisterDefaultImpl
> and virEventRunDefaultImpl using poll() event loop
> * src/util/event_poll.c: Add full error reporting
> * src/util/virterror.c, include/libvirt/virterror.h: Add
> VIR_FROM_EVENTS
> ---
> daemon/libvirtd.c | 12 +----
> include/libvirt/libvirt.h.in | 3 +
> include/libvirt/virterror.h | 1 +
> src/libvirt_private.syms | 8 ----
> src/libvirt_public.syms | 6 +++
> src/util/event.c | 94 +++++++++++++++++++++++++++++++++++++++++-
> src/util/event_poll.c | 24 ++++++++++-
> src/util/virterror.c | 3 +
> tools/console.c | 3 +-
> tools/virsh.c | 9 +---
> 10 files changed, 133 insertions(+), 30 deletions(-)
You need to squash this in to keep 'make syntax-check' happy:
diff --git i/po/POTFILES.in w/po/POTFILES.in
index 9852f97..1ed2765 100644
--- i/po/POTFILES.in
+++ w/po/POTFILES.in
@@ -88,6 +88,7 @@ src/util/cgroup.c
src/util/command.c
src/util/conf.c
src/util/dnsmasq.c
+src/util/event_poll.c
src/util/hooks.c
src/util/hostusb.c
src/util/interface.c
> +++ b/include/libvirt/virterror.h
> @@ -79,6 +79,7 @@ typedef enum {
> VIR_FROM_SYSINFO = 37, /* Error from sysinfo/SMBIOS */
> VIR_FROM_STREAMS = 38, /* Error from I/O streams */
> VIR_FROM_VMWARE = 39, /* Error from VMware driver */
> + VIR_FROM_EVENT = 40, /* Error from event loop impl */
> } virErrorDomain;
Hmm, the line before had TAB, but your line has spaces, which makes it
render odd in my reply. Preferences on which whitespace we should be
using there? But any cleanup should be a separate patch.
> +++ b/src/libvirt_private.syms
> @@ -396,14 +396,6 @@ virEventUpdateTimeout;
> # event_poll.h
> virEventPollToNativeEvents;
> virEventPollFromNativeEvents;
> -virEventPollRunOnce;
> -virEventPollInit;
> -virEventPollRemoveTimeout;
> -virEventPollUpdateTimeout;
> -virEventPollAddTimeout;
> -virEventPollRemoveHandle;
> -virEventPollUpdateHandle;
> -virEventPollAddHandle;
Nice - added in patch 2, and made private again in patch 3.
> +++ b/src/libvirt_public.syms
> @@ -424,4 +424,10 @@ LIBVIRT_0.8.8 {
> virConnectGetSysinfo;
> } LIBVIRT_0.8.6;
>
> +LIBVIRT_0.9.0 {
> + global:
> + virEventRegisterDefaultImpl;
> + virEventRunDefaultImpl;
> +} LIBVIRT_0.8.8;
So we're all in agreement that there's enough refactoring and other
goodness going in to call the next version 0.9.0 :)
> +
> +/**
> + * virEventRegisterDefaultImpl:
> + *
> + * Registers a default event implementation based on the
> + * poll() system call. This is a generic implementation
> + * that can be used by any client application which does
> + * not have a need to integrate with an external event
> + * loop impl.
s/impl/implementation/
Abbreviations are fine in code and even method names, but in the
documentation, it's nice to spell it out.
> +
> +/**
> + * virEventRunDefaultImpl:
> + *
> + * Run one iteration of the event loop. Applications
> + * will generally want to have a thread which invokes
> + * this method in an infinite loop
Maybe add a sentence:
s/loop/loop. Each iteration of the loop blocks without consuming CPU
until the next timeout or poll-based activity is detected./
> +int virEventRunDefaultImpl(void)
> +{
> + VIR_DEBUG0("");
Why ""? A timestamp in the log without contents looks suspicious;
should we add some contents, such as "event loop started"?
ACK with those nits addressed.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
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