Re: [PATCH 2/2] g364fb: add VMStateDescription for G364SysBusState

[Mark Cave-Ayland]

On 25/06/2021 09:44, Philippe Mathieu-Daudé wrote:

> On 6/25/21 9:38 AM, Mark Cave-Ayland wrote:
> > Currently when QEMU attempts to migrate the MIPS magnum machine it crashes due
> > to a mistake in the g364fb VMStateDescription configuration which expects a
> > G364SysBusState and not a G364State.
> >
> > Resolve the issue by adding a new VMStateDescription for G364SysBusState and
> > embedding the existing vmstate_g364fb VMStateDescription inside it using
> > VMSTATE_STRUCT.
>
> Broken since 97a3f6ffbba ("g364fb: convert to qdev")?

(goes and looks)

Wow that does appear to be correct - I gave up looking when I got to 8 years 
ago ;)
But yes, the bug is introduced by that commit: before you can see that 
register_savevm() was called with a G364State opaque whereas after the switch to QOM 
object the object reference for the VMStateDescription becomes G364SysBusState instead.

I'll add a "Fixes:" tag and send again.

> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/display/g364fb.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
> > index 163d7f5391..990ef3afdd 100644
> > --- a/hw/display/g364fb.c
> > +++ b/hw/display/g364fb.c
> > @@ -518,6 +518,16 @@ static Property g364fb_sysbus_properties[] = {
> >       DEFINE_PROP_END_OF_LIST(),
> >   };
> >   
> > +static const VMStateDescription vmstate_g364fb_sysbus = {
> > +    .name = "g364fb-sysbus",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_STRUCT(g364, G364SysBusState, 1, vmstate_g364fb, G364State),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >   static void g364fb_sysbus_class_init(ObjectClass *klass, void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> > @@ -526,7 +536,7 @@ static void g364fb_sysbus_class_init(ObjectClass *klass, 
> > void *data)
> >       set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
> >       dc->desc = "G364 framebuffer";
> >       dc->reset = g364fb_sysbus_reset;
> > -    dc->vmsd = &vmstate_g364fb;
> > +    dc->vmsd = &vmstate_g364fb_sysbus;
> >       device_class_set_props(dc, g364fb_sysbus_properties);
> >   }
> >   


ATB,

Mark.