Re: [PATCH] hw/i386/microvm: Fix crash that occurs when introspecting the microvm machine

[Sergio Lopez Pascual]

Thomas Huth <thuth@redhat.com> writes:

> QEMU currently crashes when you try to inspect the properties of the
> microvm machine:
>
>  $ echo '{ "execute": "qmp_capabilities" }
>          { "execute": "qom-list-properties","arguments":
>            { "typename": "microvm-machine"}}' | \
>    ./qemu-system-x86_64 -qmp stdio
>  {"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 9},
>   "package": "v9.2.0-1072-g60af367187-dirty"}, "capabilities": ["oob"]}}
>  {"return": {}}
>  qemu-system-x86_64: ../qemu/hw/i386/acpi-microvm.c:250:
>   void acpi_setup_microvm(MicrovmMachineState *):
>    Assertion `x86ms->fw_cfg' failed.
>  Aborted (core dumped)
>
> This happens because the microvm machine adds a machine_done (and a
> powerdown_req) notifier in their instance_init function - however, the
> instance_init of machines are not only called for machines that are
> realized, but also for machines that are introspected, so in this case
> the listener is added for a microvm machine that is never realized. And
> since there is already a running machine, the listener function is
> triggered immediately, causing a crash since it was not for the right
> machine it was meant for.
>
> Such listener functions must never be installed from an instance_init
> function. Let's do it from microvm_machine_state_init() instead - this
> function is the MachineClass->init() function instead, i.e. guaranteed
> to be only called once in the lifetime of a QEMU process.
>
> Since the microvm_machine_done() and microvm_powerdown_req() were
> defined quite late in the microvm.c file, we have to move them now
> also earlier, so that we can get their function pointers from
> microvm_machine_state_init() without having to introduce a separate
> prototype for those functions earlier.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/i386/microvm.c | 66 +++++++++++++++++++++++------------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)

Reviewed-by: Sergio Lopez <slp@redhat.com>

Thanks, Thomas!

Sergio.