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

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/23/2011 09:17 AM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 03/23/2011 05:22 AM, Peter Maydell wrote:
>>> On 23 March 2011 00:16, Anthony Liguori<aliguori@us.ibm.com>   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.
>