Re: [PATCH] libxl: Always set ao for qmp_proxy_spawn_outcome

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Mar 30, 2022 at 02:16:58PM -0400, Jason Andryuk wrote:
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 9a8ddbe188..59a8dcf3a9 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -2468,6 +2468,9 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
>      uint32_t dm_domid = sdss->pvqemu.guest_domid;
>      int need_qemu;
>  
> +    /* Set for out label through qmp_proxy_spawn_outcome(). */
> +    sdss->qmp_proxy_spawn.ao = ao;

I don't think that's correct. I feels like `sdss->qmp_proxy_spawn`
doesn't need to be initialised before calling spawn_qmp_proxy().

Also `qmp_proxy_spawn.ao` only need to be set before calling
libxl__spawn_spawn(), so at the same time as the rest of the
initialisation of `qmp_proxy_spawn` in spawn_qmp_proxy().


Next, about the uninitialized `ao` field:
- qmp_proxy_spawn_outcome() shouldn't use `qmp_proxy_spawn.ao`, but
  instead it should use the one available in `sdss`, that is
  `sdss->dm.spawn.ao` (The one that libxl__spawn_stub_dm() or
  spawn_stub_launch_dm() is using).
- And spawn_qmp_proxy() should also use `sdss->dm.spawn.ao` for
  STATE_AO_GC() as I don't think spawn_qmp_proxy() can expect
  `qmp_proxy_spawn` to be initialised as that's the function that
  initialise it.


Thanks,

-- 
Anthony PERARD