Re: [libvirt] [PATCH 3/3] Expose event loop implementation as a public API

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_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 :)

> +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"?