Re: [Xen-devel] [PATCH] fix race condition between libvirtd event handling and libxl fd deregister

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-01-11 at 17:51 +0000, Jim Fehlig wrote:
> Ian Campbell wrote:
> > On Mon, 2012-12-10 at 16:56 +0000, Ian Jackson wrote:
> >   
> >> Ian Jackson writes ("Re: [PATCH] fix race condition between libvirtd event 
> >> handling and libxl fd deregister"):
> >>     
> >>> I'm not surprised that the original patch makes Bamvor's symptoms go
> >>> away.  Bamvor had one of the possible races (the fd-related one) but
> >>> not the other.
> >>>       
> >> Here (followups to this message, shortly) is v3 of my two-patch series
> >> which after conversation with Ian C I think fully fixes the race, and
> >> which I have tested now.
> >>     
> >
> > Is this version now tested and ready to be applied?
> >   
> 
> Hi Ian,
> 
> I have been doing quite a bit of testing with this version, but have one
> remaining issue wrt races between the libvirt libxl driver and libxl. 
> Earlier in this thread you mentioned this potential solution
> 
> "The other scheme which springs to mind is to do reference counting, with
> the application holding a reference whenever the event is present in its
> event loop (such that there is any chance of the event being generated)
> and libxl holding a reference while it considers the event to be active"
> 
> I thought this was a good approach, particularly since libvirt has
> excellent support for it.  When libxl registers an fd/timer, I create an
> object containing the details with an initial reference count of 1.  If
> the fd/timer is successfully injected into libvirt's event loop, I take
> another reference on the object.  The object is only destroyed after
> libxl has deregistered the fd/timer *and* it has been removed from
> libvirt's event loop.  For each fd/timer object, I also increment the
> reference count on my libxl_ctx object.  This approach works well IMO. 
> It ensures the libxl_ctx exists for the life of all fd/timer objects.

Is taking a reference count on the ctx for each fd/timer strictly
necessary?

You can guarantee that the ctx lifetime is greater than the fd/timer
lifetime because if you were to destroy the ctx then it would teardown
the fd/timer as part of ctx_free (I think? More of an Ian J question).

Without those extra references I think the problem you describe below
doesn't happen.

> The only wrench in this machinery is that watch_efd is not deregistered
> until calling libxl_ctx_free().  But I never get to that point since
> that fd registration holds a reference on my libxl_ctx :(.  My first
> thought was to cleanup/deregister that fd on domain death, but I didn't
> have much success creating a patch.  Perhaps I should look at that again...

I'd be worried about libxl internal uses of this watch which you cannot
easily control preventing you from doing this.

> Some other thoughts included: 1) an API to remove fd/timers from libxl,
> 2) ensure no callbacks are invoked from libxl_ctx_free().
> 
> Thanks!
> Jim
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel