From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 2/2] libxl: fix stale timeout event callback race Date: Thu, 13 Dec 2012 10:29:36 +0000 Message-ID: <1355394576.10554.62.camel@zakaz.uk.xensource.com> References: <20678.5159.946248.90947@mariner.uk.xensource.com> <1355158624-11163-2-git-send-email-ian.jackson@eu.citrix.com> <50C7B974.4050706@suse.com> <20680.47971.962603.851882@mariner.uk.xensource.com> <50C8BE3F.4040402@suse.com> <20680.49391.646654.814456@mariner.uk.xensource.com> <50C8C665.2030202@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <50C8C665.2030202@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jim Fehlig Cc: Bamvor Jian Zhang , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org 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.