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

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Thu, Mar 31, 2022 at 9:24 AM Anthony PERARD
<anthony.perard@xxxxxxxxxx> wrote:
>
> 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.

That all sounds good.  Thanks, Anthony.

-Jason