Re: [Qemu-devel] Re: [PATCH 11/11] test-vmstate: add test case to verify we don't change VMState

[Anthony Liguori]

On 03/23/2011 09:17 AM, Juan Quintela wrote:
> Anthony Liguori<address@hidden>  wrote:
> > On 03/23/2011 05:22 AM, Peter Maydell wrote:
> > > On 23 March 2011 00:16, Anthony Liguori<address@hidden>   wrote:
> > > > +    if (old_version != new_version) {
> > > > +        g_error("Version %d of device `%s' is available in QEMU, but schema still 
> > > > reports %d, please update schema.\n",
> > > > +                new_version, device, old_version);
> > > > +    }
> > > Might be nice for these "please update" error messages to
> > > include a pointer to a docs file explaining in more detail
> > > how to do that?
> > > (also>80 char line ;-))
> > Ack.
> >
> > > > diff --git a/vmstate/schema.json b/vmstate/schema.json
> > > > new file mode 100644
> > > > index 0000000..23483ab
> > > > --- /dev/null
> > > > +++ b/vmstate/schema.json
> > > > @@ -0,0 +1,1176 @@
> > > > +{
> > > > +    "cpu": {
> > > > +        "mcg_cap": "uint64",
> > > > +        "a20_mask": "int32",
> > > > +        "tsc_offset": "uint64",
> > > This schema file appears to be board-specific (or at least
> > > x86-specific) -- shouldn't the cpu/board/whatever name
> > > be in the filename, so we have scope to expand the test
> > > to checking migration issues for other platforms too?
> > It's not really.  Every VMStateDescription that is builtin into the
> > tree is in the file.
> >
> > That said, the only target where the CPU is currently described by
> > VMStateDescription is target-i386.
> >
> > Right now the file is generated via i386-softmmu.  There may be a few
> > devices left out because they are either not compiled into
> > i386-softmmu or are target specific.
> >
> > We could complicate things further by trying to run against every
> > target and then building a union of all target outputs but I'm not
> > sure it's worth the effort at this stage.
> >
> > > (I don't care much about ARM migration breakages just at the
> > > moment but I suspect that it will be becoming more important
> > > by this time next year...)
> > >
> > > Also since this looks like an autogenerated file that's going
> > > to be going into version control maybe it should have a
> > > comment header at the top of the "autogenerated, do not edit
> > > by hand!" type.
> > JSON doesn't support comments..  I can add comment parsing to our
> > parser though.
> We need to fix the ordering problem.

Dunno what you mean by ordering.

> Whatever schema we have should be good enough to allow:
> - describe me this blob that contains the state for this device.

Schema for VMState is different than what's used for this test case 
here.  I agree, it's a harder problem than just what's being spit out 
here :-)

> eepro100 at least is missing.  Althought I would vote to just change the
> eepro100 "naming" to always use eepro100 or similar, and remove the
> current hack of having to change the vmstate->name for each different
> device.

I just ran into eepro100 and my head nearly exploded.

I set the name to be eepro100-base and then just added that once.  A 
better solution would be to separate out the fields such that we can 
have a bunch of VMStateDescriptions that all use the same fields.

I think we ought to merge VMStateDescription into DeviceInfo.  For 
compatibility, we probably need a vmstate_alias name since the device 
names don't always map 1-1 with the qdev names.  But this should 
eliminate the problem of reusing VMStateDescriptions for multiple devices.

Regards,

Anthony Liguori

> Later, Juan.