[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] libxl: Don't segfault on soft-reset failure


  • To: Jason Andryuk <jandryuk@xxxxxxxxx>
  • From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
  • Date: Thu, 31 Mar 2022 15:10:00 +0100
  • Authentication-results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Juergen Gross <jgross@xxxxxxxx>
  • Delivery-date: Thu, 31 Mar 2022 14:10:10 +0000
  • Ironport-data: A9a23:J+py2KmgcmBg7tjyDec5Ob7o5gyWJkRdPkR7XQ2eYbSJt1+Wr1Gzt xIcWzyBMv2DZmP3f48nPti0p0lQv5PUnNJlTFRtrnozFSMWpZLJC+rCIxarNUt+DCFioGGLT Sk6QoOdRCzhZiaE/n9BCpC48T8kk/vgqoPUUIYoAAgoLeNfYHpn2EoLd9IR2NYy24DiWFrV4 7senuWEULOb828sWo4rw/rrRCNH5JwebxtB4zTSzdgS1LPvvyF94KA3fMldHFOhKmVgJcaoR v6r8V2M1jixEyHBqD+Suu2TnkUiGtY+NOUV45Zcc/DKbhNq/kTe3kunXRa1hIg+ZzihxrhMJ NtxWZOYSDcRYKSdpf4hQlpSNyFAA715/oTtGC3q2SCT5xWun3rExvxvCAc9PJEC+/YxCmZLn RAaAGlTNFbZ3bvwme/lDLk37iggBJCD0Ic3s3d8zTbfHLA+TIrKWani7t5ExjYgwMtJGJ4yY uJHNmI+M0WRM3WjPH83S4gVn8P5tEWjWDdEs2+y/7Eo5FHcmVkZPL/Fb4OOJ43iqd9utlmcj nLL+SL+GB5yHN6Q0z2e6Vq3m/TC2yj8Xeo6FrS++uR7nV67yWkaCRlQXly+ydGph0j7V99BJ kg8/is1sbN05EGtVsP6XRCzvDiDpBF0ZjZLO7RkskfXkPOSulvHQDhfJtJcVDA4nM4GfWIjh 2KTo8KqDxhvjoOFdnSm3I7B+FteJhMpBWMFYCYFSy4M7N/ivJw/g3rzczpzLEKmpoarQG+tm lhmuAB73uxO1pBTi81X6Hid21qRSo71ohnZD+k9dkas9UtHaYGsfOREAnCLvK8bfO51orRs1 UXoevRyDshTVflhdwTXGY3h+Y1FAd7falUwZnY1Q/EcG8yFoSLLQGypyGgWyL1VGsgFYyT1R 0TYpBlc4pReVFPzM/MnPdzsVJpwkPe9fTgAahwyRoAUCnSWXFXalByCmGbKhzy9+KTSuf9X1 WinnTaEUi9BVPUPIMueTOYBy747rh3SNkuILa0XOy+PiOLEDFbMEO9tGALXMogRsfPVyC2Io o03H5bblH1ivBjWP3C/HXg7dgtRcxDWxPne9qRqSwJ0ClE4QDt4Ua+AnOtJlk4Mt/09q9okN 0qVAidwoGcTT1WdQelWQhiPsI/SYKs=
  • Ironport-hdrordr: A9a23:Q7cbHagdoPNcZ3AE4gF5dCyaLnBQXrkji2hC6mlwRA09TyX4ra CTdZEgviMc5wx9ZJhNo7q90cq7IE80i6Qb3WB5B97LYOCMggeVxe9Zg7ff/w==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Mar 30, 2022 at 02:17:27PM -0400, Jason Andryuk wrote:
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index 15ed021f41..631caa416d 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -1970,7 +1970,8 @@ static void domcreate_complete(libxl__egc *egc,
>      libxl_domain_config *const d_config = dcs->guest_config;
>      libxl_domain_config *d_config_saved = &dcs->guest_config_saved;
>  
> -    libxl__xswait_stop(gc, &dcs->console_xswait);
> +    if (dcs->console_xswait.path)
> +        libxl__xswait_stop(gc, &dcs->console_xswait);

libxl__xswait_stop() needs to be called here. It's a function that can
be called several time without ill effect, as long as `console_xswait`
is initialised properly (by calling libxl__xswait_init() like you did
below.

>      libxl__domain_build_state_dispose(&dcs->build_state);
>  
> @@ -2176,6 +2177,10 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
>                                aop_console_how);
>      cdcs->domid_out = &domid_out;
>  
> +    /* Initialize in case we end up in domcreate_complete without calling
> +     * initiate_domain_create. */

That comment isn't needed. This is just part of the initialisation of
`cdcs->dcs`.

> +    libxl__xswait_init(&cdcs->dcs.console_xswait);
> +
>      dom_path = libxl__xs_get_dompath(gc, domid);
>      if (!dom_path) {
>          LOGD(ERROR, domid, "failed to read domain path");


As part of the patch, could you also move the libxl__xswait_init() call
in initiate_domain_create() to do_domain_create()? That would avoid
`console_xswait` been initialised twice, and do_domain_create() is the
function that initialise `dcs` before calling initiate_domain_create().

Thanks,

-- 
Anthony PERARD



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.