Re: [PATCH] hw/display/vga-isa: Fix migration of the isa-vga device

[Fabiano Rosas <farosas@suse.de>]

Thomas Huth <thuth@redhat.com> writes:

> From: Thomas Huth <thuth@redhat.com>
>
> QEMU currently crashes when migrating a guest that uses the
> isa-vga device as display. This happens because vga_isa_class_initfn()
> registers a vmsd for vmstate_vga_common that operates on VGACommonState.
> But the isa-vga device is derived from ISADevice, not from VGACommonState,
> so the migration code tries to fill in the data for VGACommonState to
> the memory that is a ISADevice instead, which is of cause causing trouble.
>
> We need an indirection here as it's also e.g. done in vga-pci.c, so
> that the migration data gets filled into the right location.
>
> While we're at it, also drop the "global_vmstate = true" here. Since
> migration was broken for this device during the last 15 years (!) anyway,
> we don't have to worry about maintaining backward compatibility with this
> switch for older versions of QEMU anymore.
>
> Fixes: 7435b791ca9 ("vga-isa: convert to qdev")
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/display/vga-isa.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
> index 95d85ff69a5..5f55c884a1b 100644
> --- a/hw/display/vga-isa.c
> +++ b/hw/display/vga-isa.c
> @@ -32,6 +32,7 @@
>  #include "qemu/timer.h"
>  #include "hw/core/loader.h"
>  #include "hw/core/qdev-properties.h"
> +#include "migration/vmstate.h"
>  #include "ui/console.h"
>  #include "qom/object.h"
>  
> @@ -62,7 +63,6 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
>      MemoryRegion *vga_io_memory;
>      const MemoryRegionPortio *vga_ports, *vbe_ports;
>  
> -    s->global_vmstate = true;
>      if (!vga_common_init(s, OBJECT(dev), errp)) {
>          return;
>      }
> @@ -88,6 +88,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
>      rom_add_vga(VGABIOS_FILENAME);
>  }
>  
> +static const VMStateDescription vmstate_vga_isa = {
> +    .name = "vga-isa",
> +    .version_id = 1,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_STRUCT(state, ISAVGAState, 0, vmstate_vga_common, VGACommonState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const Property vga_isa_properties[] = {
>      DEFINE_PROP_UINT32("vgamem_mb", ISAVGAState, state.vram_size_mb, 8),
>  };
> @@ -98,7 +107,7 @@ static void vga_isa_class_initfn(ObjectClass *klass, const void *data)
>  
>      dc->realize = vga_isa_realizefn;
>      device_class_set_legacy_reset(dc, vga_isa_reset);
> -    dc->vmsd = &vmstate_vga_common;
> +    dc->vmsd = &vmstate_vga_isa;
>      device_class_set_props(dc, vga_isa_properties);
>      set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
>  }

Reviewed-by: Fabiano Rosas <farosas@suse.de>