qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH 11/11] test-vmstate: add test case to verify we


From: Anthony Liguori
Subject: [Qemu-devel] Re: [PATCH 11/11] test-vmstate: add test case to verify we don't change VMState
Date: Wed, 23 Mar 2011 11:14:32 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.14) Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8

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:
We need to fix the ordering problem.
Dunno what you mean by ordering.
vmstate:

static const VMStateDescription vmstate_cpu = {
     .name = "cpu",
     .version_id = CPU_SAVE_VERSION,
     .minimum_version_id = 3,
     .minimum_version_id_old = 3,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_UINTTL_ARRAY(regs, CPUState, CPU_NB_REGS),
         VMSTATE_UINTTL(eip, CPUState),
         VMSTATE_UINTTL(eflags, CPUState),
         VMSTATE_UINT32(hflags, CPUState),
         /* FPU */


vs

dump

     "cpu": {
         "mcg_cap": "uint64",
         "a20_mask": "int32",
         "tsc_offset": "uint64",
         "idt": {
             "flags": "uint32",
             "limit": "uint32",
             "selector": "uint32",
             "base": "uint32",
             "__version__": 1
         },
         "intercept_cr_write": "uint16",
         "nmi_injected": "uint8",


You see that they are not in same order, then I can't use the schema to
read an arbitrary savevm image.  I think that ordering should be
preserved, makes schema much, much more useful.

This is *only* for testing right now. We can enhance it down the road if we want to but this is an opaque thing that's only there to enable test cases to be written.


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.

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?

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.

Regards,

Anthony Liguori

Later, Juan.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]