From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=57274 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q2Qhn-00025G-R2 for qemu-devel@nongnu.org; Wed, 23 Mar 2011 12:14:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q2Qhm-000733-Q5 for qemu-devel@nongnu.org; Wed, 23 Mar 2011 12:14:43 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:58038) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q2Qhm-00072r-ME for qemu-devel@nongnu.org; Wed, 23 Mar 2011 12:14:42 -0400 Received: by gxk26 with SMTP id 26so4208999gxk.4 for ; Wed, 23 Mar 2011 09:14:42 -0700 (PDT) Message-ID: <4D8A1C68.6050506@codemonkey.ws> Date: Wed, 23 Mar 2011 11:14:32 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1300839376-22520-1-git-send-email-aliguori@us.ibm.com> <1300839376-22520-12-git-send-email-aliguori@us.ibm.com> <4D89EABE.50204@codemonkey.ws> <4D8A0947.5080809@codemonkey.ws> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 11/11] test-vmstate: add test case to verify we don't change VMState List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com Cc: Peter Maydell , qemu-devel@nongnu.org, Jan Kiszka On 03/23/2011 10:26 AM, Juan Quintela wrote: > Anthony Liguori wrote: >> On 03/23/2011 09:17 AM, Juan Quintela wrote: >>> Anthony Liguori 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.