Re: [Xen-devel] [PATCH 07/10] libxl: New API for providing OS events to libxl

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 07/10] libxl: New API for 
providing OS events to libxl"):
> On Fri, 2012-01-06 at 20:35 +0000, Ian Jackson wrote:
> > +int libxl_osevent_beforepoll(libxl_ctx *ctx, int *nfds_io,
> > +                             struct pollfd *fds, int *timeout_upd,
> > +                             struct timeval now);
> > +  /* The caller should provide beforepoll with some space for libxl's
> > +   * fds, and tell libxl how much space is available by setting *nfds_io.
> > +   * fds points to the start of this space (and fds may be a pointer into
> > +   * a larger array, for example, if the application has some fds of
> > +   * its own that it is interested in).
> 
> I think you were going to move the comments (sorry).

*sigh*  OK, I will do that.

> > + * Most code in libxlshould not need to initialise their own egc.
> 
> Missing space.         ^

Fixed.

> Will any function even need to initialise their own egc and/or is it
> normally provided by infrastructure code? Otherwise when should a
> function initialise their own egc? Is it simply that you know if you
> need to initialise one because you are calling a function which takes
> one?

You don't need to because you should never end up calling a function
that takes an egc unless you've got one already from the event
machinery.

> > + * Even functions which generate specific kinds of events don't need
> > + * to - rather, they will be passed an egc into their own callback
> > + * function and should just use the one they're given.
> > + *
> > + * A handy idiom for functions taking an egc is:
> > + *     libxl__gc *gc = &egc->gc;
> 
> You've provided lots of handy macros for other things like this, perhaps
> EGC by comparison with CTX. Using GC seems better on the face of it but
> since it will only work in a context with an egc that is actually
> confusing.

How about:
  #define EGC_GC  libxl__gc *gc = &egc->gc
?

> > + * Functions using LIBXL__INIT_EGC may *not* generally be called from
> > + * within libxl, because libxl__egc_cleanup may call back into the
> > + * application.  This should be documented near the function
> > + * prototype(s) for callers of LIBXL__INIT_EGC and EGC_INIT.
> 
> Can we also enforce/check this with a flag in the ctx? I guess not since
> multiple threads might call into libxl with the same ctx and each may
> legitimately setup an egc.?

Exactly.  This is the same disagreement I'm having with Stefano.

But this restriction is not difficult in practice because functions
that want to generate events are almost[1] invariably functions which
were themselves called in response to an OS event, and they get an egc
provided by the event machinery.

See for example disk_eject_xswatch_callback, which is the thing which
generates events of type DISK_EJECT.  It's a xenstore watch callback
function, so it gets an egc provided by the event machinery and uses
that for NEW_EVENT etc.

The code which sets up the callback, libxl_evenable_disk_eject,
doesn't need an egc because setting up a callback only needs a gc.  So
you never need to construct an egc yourself.

The functions which make their own egc, and which are therefore
forbidden for other callers inside libxl, are things like
   libxl_osevent_beforepoll
   libxl_event_wait
and if you're calling those from within libxl you are definitely doing
the whole thing entirely wrong.

Ian.

[1] libxl__ao_inprogress is a counterexample, but it's part of the
event machinery, not a normal function implementing some actual
domain-related functionality.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel