From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48372 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q2PQp-000842-OT for qemu-devel@nongnu.org; Wed, 23 Mar 2011 10:53:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q2PQn-0005WH-RJ for qemu-devel@nongnu.org; Wed, 23 Mar 2011 10:53:07 -0400 Received: from mail-gy0-f173.google.com ([209.85.160.173]:51267) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q2PQn-0005WA-Oi for qemu-devel@nongnu.org; Wed, 23 Mar 2011 10:53:05 -0400 Received: by gyf1 with SMTP id 1so3366378gyf.4 for ; Wed, 23 Mar 2011 07:53:05 -0700 (PDT) Message-ID: <4D8A0947.5080809@codemonkey.ws> Date: Wed, 23 Mar 2011 09:52:55 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH 11/11] test-vmstate: add test case to verify we don't change VMState 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> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 09:17 AM, Juan Quintela wrote: > Anthony Liguori wrote: >> On 03/23/2011 05:22 AM, Peter Maydell wrote: >>> On 23 March 2011 00:16, Anthony Liguori 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. >