On 20/10/2023 11.07, Juan Quintela wrote:
> Otherwise device-introspection-test fails.
>
> $ ./tests/qtest/device-introspect-test
> ...
> Broken pipe
> ../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
> Aborted (core dumped)
>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/ipmi/ipmi_bmc_extern.c | 2 +-
> hw/ipmi/ipmi_bmc_sim.c | 2 +-
> hw/ipmi/isa_ipmi_bt.c | 2 +-
> hw/ipmi/isa_ipmi_kcs.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
> index e232d35ba2..324a2c8835 100644
> --- a/hw/ipmi/ipmi_bmc_extern.c
> +++ b/hw/ipmi/ipmi_bmc_extern.c
> @@ -504,7 +504,7 @@ static void ipmi_bmc_extern_init(Object *obj)
> IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj);
>
> ibe->extern_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, extern_timeout, ibe);
> - vmstate_register(NULL, 0, &vmstate_ipmi_bmc_extern, ibe);
> + vmstate_register_any(NULL, &vmstate_ipmi_bmc_extern, ibe);
> }
This is an instance_init() function. We shouldn't register vmstate (and
timer) here, but do it in the realize() function instead.
>
> static void ipmi_bmc_extern_finalize(Object *obj)
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 905e091094..404db5d5bc 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -2188,7 +2188,7 @@ static void ipmi_sim_realize(DeviceState *dev, Error **errp)
>
> ibs->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ipmi_timeout, ibs);
>
> - vmstate_register(NULL, 0, &vmstate_ipmi_sim, ibs);
> + vmstate_register_any(NULL, &vmstate_ipmi_sim, ibs);
> }
Now here it's been done in the realize() function already ... did this
really cause trouble for you?
Anyway, I wonder why the first parameter is NULL here ... shouldn't this
point to dev instead?
> static Property ipmi_sim_properties[] = {
> diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
> index a83e7243d6..afb76b548a 100644
> --- a/hw/ipmi/isa_ipmi_bt.c
> +++ b/hw/ipmi/isa_ipmi_bt.c
> @@ -125,7 +125,7 @@ static void isa_ipmi_bt_init(Object *obj)
>
> ipmi_bmc_find_and_link(obj, (Object **) &iib->bt.bmc);
>
> - vmstate_register(NULL, 0, &vmstate_ISAIPMIBTDevice, iib);
> + vmstate_register_any(NULL, &vmstate_ISAIPMIBTDevice, iib);
> }
It's an instance_init() function again. This should be done in the realize()
instead.
> static void *isa_ipmi_bt_get_backend_data(IPMIInterface *ii)
> diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
> index b2ed70b9da..5ab63b2fcf 100644
> --- a/hw/ipmi/isa_ipmi_kcs.c
> +++ b/hw/ipmi/isa_ipmi_kcs.c
> @@ -132,7 +132,7 @@ static void isa_ipmi_kcs_init(Object *obj)
> * IPMI device, so receive it, but transmit a different
> * version.
> */
> - vmstate_register(NULL, 0, &vmstate_ISAIPMIKCSDevice, iik);
> + vmstate_register_any(NULL, &vmstate_ISAIPMIKCSDevice, iik);
> }
Dito, this should be moved to realize().
Thomas