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

[Juan Quintela]

Anthony Liguori <address@hidden> wrote:
> On 03/23/2011 10:26 AM, Juan Quintela wrote:
>> Anthony Liguori<address@hidden>  wrote:
>>> On 03/23/2011 09:17 AM, Juan Quintela wrote:
>>>> Anthony Liguori<address@hidden>   wrote:

>> Once told that, I think that doing a big schema is just wrong, we should
>> do an schema for device (or at least for architecture).  And no
>> hardcoded names (as they are today).  It is just trivial to run it for
>> x86_64-softmmu/i386-softmmu (the things that should work nowadays).
>>
>> That way, downstreams can use it for its own "minimal machines".
>
> I agree, we ought to try to make this schema more consumable.  But
> some of that is that the schema needs to describe types in a more
> meaningful way as there's a lot of weird types right now.

I have no clue about qdicts, but it is as easy as:
- add a new field that gives the "order", if this is not possible
- add a new qdict from order -> name

it is not rockets science.  I am looking at your patch right now, but my
experience with glib, q* and json is inexistant, so I go slowwwwww.

>>>> 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 :-)
>> It should be the same IMHO, it will not complicate anything here, and
>> just make it more useful.
>
> Yeah, it will tremendously complicate things because QDict's don't
> preserve order.  It's all fixable but why wait to have something
> that's incredibly useful in tree?

see above.

>>>> 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.
>> Being there, know the feeling.
>>
>>> 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.
>> Agreed with that.
>
> Once we settle on the unit tests, I can look at writing some scripts
> to do this.

I think that we need to improve the code that you sent, but I agree that
we can go by parts.

Later, Juan.