On Thu, Mar 03, 2011 at 04:13:08PM -0700, Eric Blake wrote:
> 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.
Odd, I thought our make syntax-check blocked all use of TAB
in our files.
> > +++ 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 :)
There's far more to come from me too :-)
> > +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"?
It isn't just the timestamp. The log will contain the function name,
which is what I really want to see.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list