From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jim Fehlig Subject: Re: [PATCH 2/2] libxl: fix stale timeout event callback race Date: Thu, 13 Dec 2012 08:53:14 -0700 Message-ID: <50C9F9EA.7020408@suse.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> <1355394576.10554.62.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1355394576.10554.62.camel@zakaz.uk.xensource.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: Ian Campbell Cc: "xen-devel@lists.xen.org" , Ian Jackson , Bamvor Jian Zhang List-Id: xen-devel@lists.xenproject.org Ian Campbell wrote: > 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. > I think Ian J. is right about applications still working, *if* they have the callbacks coded correctly. There are some bugs on the libvirt side as well :). > 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. libvirt's event loop lock is private to the event impl and not exposed to its numerous users. Regards, Jim