Re: [Xen-devel] [xen-unstable bisection] complete test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm

[Anthony PERARD <anthony.perard@citrix.com>]

On Tue, Jan 21, 2020 at 10:21:09AM +0000, Roger Pau Monné wrote:
> The issue is that this change is passing the guest domain_create_state
> to libxl__domain_build in libxl__spawn_stub_dm, and hence the
> stubdomain doesn't get created. I have the following patch that fixes
> it, but it's kind of dirty.
> 
> ---8<---
> From 688fde95992d07bb1123d324a68006dd08bc6512 Mon Sep 17 00:00:00 2001
> From: Roger Pau Monne <roger.pau@citrix.com>
> Date: Tue, 21 Jan 2020 10:14:09 +0000
> Subject: [PATCH] libxl: fix stubdomain creation after aacc143006429de
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8

:-(, this is a lie. The email I've received has a different charset. git
complained about it. Maybe next time the patch could be attached, or it
could be a proper patch with some note (after ---) (git send-email can
do --in-reply-to), or it could be two separated emails with the first
one replying to the report and the second the patch (all in the same
thread).

> Content-Transfer-Encoding: 8bit
> 
> aacc143006429de broke stubdomain creation by passing the guest
> domain_create_state to libxl__domain_build in libxl__spawn_stub_dm,
> when it should instead be crafting a new domain_create_state for the
> stubdomain.
> 
> Fixes: aacc143006429de ('tools/libxl: Plumb domain_create_state down into libxl__build_pre()')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  tools/libxl/libxl_dm.c       | 22 +++++++++++++---------
>  tools/libxl/libxl_internal.h |  3 +--
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 3f08ccad1b..b1ddde77e8 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -2110,17 +2110,21 @@ void libxl__spawn_stub_dm(libxl__egc *egc, libxl__domain_create_state *dcs)
>      xs_transaction_t t;
>  
>      /* convenience aliases */
> -    libxl_domain_config *const dm_config = &sdss->dm_config;
>      libxl_domain_config *const guest_config = sdss->dm.guest_config;
>      const int guest_domid = sdss->dm.guest_domid;
>      libxl__domain_build_state *const d_state = sdss->dm.build_state;
> -    libxl__domain_build_state *const stubdom_state = &sdss->dm_state;
> +    libxl__domain_build_state *stubdom_state;
> +    libxl_domain_config *dm_config;
>  
>      /* Initialise private part of sdss */
> -    libxl__domain_build_state_init(stubdom_state);
>      dmss_init(&sdss->dm);
>      dmss_init(&sdss->pvqemu);
>      libxl__xswait_init(&sdss->xswait);
> +    GCNEW(sdss->dcs);
> +    stubdom_state = &sdss->dcs->build_state;
> +    libxl__domain_build_state_init(stubdom_state);
> +    GCNEW(sdss->dcs->guest_config);
> +    dm_config = sdss->dcs->guest_config;

I don't think that's enough, we need to initialize the dcs properly.
Otherwise, libxl__domain_build() might start using thing that aren't set
properly. Maybe we would need a new struct which could be pass to
libxl__domain_build*, or that might be more complicated than needed.

>  
>      if (guest_config->b_info.device_model_version !=
>          LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL) {
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index d919f91882..abf88dfd76 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -4102,8 +4102,7 @@ typedef struct {
>      /* filled in by user, must remain valid: */
>      libxl__dm_spawn_cb *callback; /* called as callback(,&sdss->dm,) */
>      /* private to libxl__spawn_stub_dm: */
> -    libxl_domain_config dm_config;
> -    libxl__domain_build_state dm_state;
> +    libxl__domain_create_state *dcs;

This should be named dm_dcs, I think, to follow the same pattern as
before.


Thanks for tracking this down.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel