Re: [PATCH 2/2] libxl: fix stale timeout event callback race

[Ian Campbell <Ian.Campbell@citrix.com>]

On Wed, 2012-12-12 at 18:01 +0000, Jim Fehlig wrote:
> Ian Jackson wrote:

> >   
> >>  After again reading
> >> libxl_event.h, I'm considering the below patch in the libvirt libxl
> >> driver.  The change is primarily inspired by this comment for
> >> libxl_osevent_occurred_timeout:
> >>     
> > ...
> >   
> >> /* Implicitly, on entry to this function the timeout has been
> >>  * deregistered.  If _occurred_timeout is called, libxl will not
> >>  * call timeout_deregister; if it wants to requeue the timeout it
> >>  * will call timeout_register again.
> >>  */
> >>     
> >
> > Well your patch is only correct when used with the new libxl, with my
> > patches.
> >   
> 
> Hmm, it is not clear to me how to make the libxl driver work correctly
> with libxl pre and post your patches :-/.

Ideally we will find a way to make this work without changes on the
application side.

But if that turns out to be impossible and applications are going to
need patching anyway then I think we should consider just fixing the API
rather than playing tricks like the "modify to 0" thing to try and keep
it compatible.

One option is to add new hooks which libxl can call to take/release the
application's event loop lock + a LIBXL_HAVE_EVENT_LOOP_LOCK define so
the application can conditionally provide them. The upside is that I
would expect this to result in much simpler code in both libxl and
libvirt. The downside is that doing this kind of sucks from an API
stability point of view, but if the application has to change anyway
then we might as well do it cleanly instead of bending over backwards to
keep the API the same.

Ian.