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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-01-13 at 15:47 +0000, Ian Jackson wrote:
> 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:
> > 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.

I think it is worth stating explicitly. e.g. "You should never need to
initialise an egc unless you are part of the event machinery itself,
Otherwise you will always be given an egc if you need one". I might even
go as far as s/should never/will never/.

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

Works for me.

> 
> > > + * 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.

Makes sense, I think the wording I proposed above describes this?

Ian.



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