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

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 2/2] libxl: fix stale timeout event callback race"):
> On Wed, 2012-12-12 at 18:01 +0000, Jim Fehlig wrote:
> > 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.

Yes, but if the application has bugs which are exposed by the new
approach I think it is probably simplest to fix those bugs.

The way I'm proposing at the moment means that there are two sets of
relevant changes to libxl and libvirt:

 - libxl always use timeout_modify and never _deregister.  Officially
   speaking this is a backwards-compatible change: libxl now promises
   to use only a strict subset of the documented interface provided by
   the application.  Any correct application will still work.

 - libvirt implement both _modify and _deregister correctly.  With old
   libxl this leaves the timeout deregistration race.  With new libxl
   there is no problem at all.

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

I don't know what other users there might be who would be hurt by an
API change.

But I think from a deployment/testing/etc. point of view two separate
patches to libxl and libvirt which each make matters strictly better,
and which can be applied independently, does seem like the best
approach.

Ian.