Re: [PATCH for-4.5 v2] libxl: Initialise CTX->xce in domain suspend

[Ian Campbell <Ian.Campbell@citrix.com>]

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 <incoming migration stream> (new xl fmt info 0x1/0x0/1225)
> >  Savefile contains xl domain config in JSON format
> > Parsing config from <saved>
> > 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 <ian.jackson@eu.citrix.com>
> 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 <incoming migration stream> (new xl fmt info 0x1/0x0/12\
>     )
>        Savefile contains xl domain config in JSON format
>       Parsing config from <saved>
>       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 <yanghy@cn.fujitsu.com>
>     Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>