From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH for-4.5 v2] libxl: Initialise CTX->xce in domain suspend Date: Mon, 5 Jan 2015 14:41:04 +0000 Message-ID: <1420468864.28863.22.camel@citrix.com> References: <1419233604-859-1-git-send-email-yanghy@cn.fujitsu.com> <21674.41273.269354.954668@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21674.41273.269354.954668@mariner.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 Jackson Cc: Yang Hongyang , Wei Liu , xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org On Mon, 2015-01-05 at 14:35 +0000, Ian Jackson wrote: > Yang Hongyang writes ("[PATCH] xl/libxl: fix migrate/Remus regression (core dumped)"): > > When excuting xl migrate/Remus, the following error occurd: > > [root@master xen]# xl migrate 5 slaver > > migration target: Ready to receive domain. > > Saving to migration stream new xl format (info 0x1/0x0/1225) > > Loading new save file (new xl fmt info 0x1/0x0/1225) > > Savefile contains xl domain config in JSON format > > Parsing config from > > Segmentation fault (core dumped) > > > > This is because CTX->xce is used without been initialized. > > The bug was introduced by commit 2ffeb5d7f5d8 > > libxl: events: Deregister evtchn fd when not needed > > which remove the initialization of xce from libxl__ctx_alloc. > > > > This patch initialze the CTX->xce before use it. > > Thanks. This patch goes in the right direction, but isn't quite > correct because it doesn't check the return value from > libxl__ctx_evtchn_init. > > Looking at this it is clear that following the on-demand > initialisation of CTX->xce, it is normally necessary for any evtchn > user in libxl to call libxl__ctx_evtchn_init, since they will need the > xce for finding the right port number to pass to > libxl__ev_evtchn_wait. > > Sorry for not noticing this when I made my earlier change. > > I have therefore: > * In the patch below, added changes to the comments to document this. > * Done git grep '\bxce\b' tools/libxl and checked the other uses. > * Consequently, verified that the rest of the code in libxl_dom.c > avoids using xce unless guest_evtchn.port>=0, and properly > initialises .port to -1, so that there is no need for further calls > to libxl__ctx_evtchn_init. > > I have compiled but not executed this patch. Yang Hongyang: can you > please test that it fixes the bug for you ? > > Konrad: this should go in 4.5 because it is a bugfix without which > libxl may dereference NULL. > > (I have also somewhat improved the English grammar in the commit > message.) > > Thanks, > Ian. > > commit 9d1cb27f5e961fd9db1c7d8381af18e33510f924 > Author: Ian Jackson > Date: Mon Jan 5 14:31:00 2015 +0000 > > libxl: Initialise CTX->xce in domain suspend, as needed > > When excuting xl migrate/Remus, the following error can occur: > [root@master xen]# xl migrate 5 slaver > migration target: Ready to receive domain. > Saving to migration stream new xl format (info 0x1/0x0/1225) > Loading new save file (new xl fmt info 0x1/0x0/12\ > ) > Savefile contains xl domain config in JSON format > Parsing config from > Segmentation fault (core dumped) > > This is because CTX->xce is used without been initialized. > The bug was introduced by commit 2ffeb5d7f5d8 > libxl: events: Deregister evtchn fd when not needed > which removed the initialization of xce from libxl__ctx_alloc. > > In this patch we initialise the CTX->xce before using it. Also, we > adjust the doc comment for libxl__ev_evtchn_* to mention the need to > do so. > > Signed-off-by: Yang Hongyang > Signed-off-by: Ian Jackson Acked-by: Ian Campbell