| Message ID | 1300839376-22520-12-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 03/22/2011 07:16 PM, Anthony Liguori wrote: > We've had lots of issues surrounding live migration breaking. This is because > we haven't had a good way to validate that what we're migrating isn't changing > underneath of us. > > This test works by first converting the in-tree schema to C source as a string. > This is built into the test case. The test case will then query QEMU for the > current schema. > > It converts both strings to QObjects via the JSON parser and then does a > recursive diff. Unlike a simple diff command, this lets us say exactly what you > did to break migration. For instance, you get error messages like: > > You broke migration by changing the type of field 'par' in device 'cpu', > version 1 from 'uint32' to 'uint64'. > > It only covers devices that support VMState and it currently doesn't look at > subsections yet. > > The in-tree schema needs to be updated whenever migration changes but this ends > up being a very nice way to ensure that we're reviewing migration protocol > changes. > > The test case is built in the default build, and can be run by `make check'. This test takes a disturbingly long time to run considering what it's doing (~3 seconds). Turns out, the JSON parser is horribly slow because it allocates and frees memory faster than a hummingbird on steroids. I have a local patch that removes this and afterwards the test runs in just 70ms. I'll send that fix out tomorrow. Regards, Anthony Liguori
On Wed, Mar 23, 2011 at 12:16 AM, Anthony Liguori <aliguori@us.ibm.com> wrote: > +static QObject *read_current_schema(void) > +{ > + char buffer[65536]; > + int fd; > + int ret; > + size_t offset = 0; > + ssize_t len; > + > + ret = system("i386-softmmu/qemu -vmstate-dump > /tmp/schema.json"); Please don't hardcode i386-softmmu, there should at least be a way to override it. For example, I tend to build x86_64-softmmu only. Using a temporary file is not ideal because as soon as this program runs as part of an automated build system we could be clobbering the file if multiple runs are going in parallel. How about popen(3)? > 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", > + "idt": { > + "flags": "uint32", > + "limit": "uint32", > + "selector": "uint32", > + "base": "uint32", > + "__version__": 1 > + }, Is field ordering important and did we lose that information as soon as we started using dicts to represent vmstate dumps? Stefan
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 ;-)) > 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? (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. -- PMM
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. Regards, Anthony Liguori > -- PMM >
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. Whatever schema we have should be good enough to allow: - describe me this blob that contains the state for this device. 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. Later, Juan.
Peter Maydell <peter.maydell@linaro.org> 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 ;-)) > >> 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? > > (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. I agree with you. Just passing another argument to all programs telling what we are talking about would be much better for this. And we need (at least) x86_64 & i386 (this ones are supposed to work). ARM people are sending lots of vmstate changes, I guess/hope that somebody is trying to get it working. /me looks at Peter O:-), hint, hint, ... Any idea if there are images for testing ARM? Later, Juan.
On 23 March 2011 14:19, Juan Quintela <quintela@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> wrote: > ARM people are sending lots of vmstate changes, I guess/hope that > somebody is trying to get it working. > > /me looks at Peter O:-), hint, hint, ... Well, the main thing I care about currently (or did back before Christmas which is when I sent a patchset to add save/restore to a pile of ARM devices) is simple save-and-restore for debugging use. The rest is just that patches don't get through code review unless they get the vmstate stuff right, and I care about not being rejected :-) I think it's still the case that there are devices in some of the ARM devboards with no save/restore support at all. I would really prefer it if the default for a device was "I do not support this" with the things like USB where somebody has audited them as genuinely needing no save/restore code explicitly marked as "this is OK"; then we could easily determine what needed fixing and not offer a broken facility to users. > Any idea if there are images for testing ARM? There are prebuilt images on Aurelien's website for ARM and others, which is the simplest thing: http://www.aurel32.net/info/debian_arm_qemu.php -- PMM
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. >
On 23 March 2011 14:52, Anthony Liguori <anthony@codemonkey.ws> wrote: > 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. That's a feature, not a bug. Consider eg hw/pl110.c -- there are two different DeviceInfo devices but since the underlying implementation is the same you definitely don't want to have two separate VMStateDescription structures to get out of sync. -- PMM
On 2011-03-23 16:00, Peter Maydell wrote: > On 23 March 2011 14:52, Anthony Liguori <anthony@codemonkey.ws> wrote: >> 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. > > That's a feature, not a bug. Consider eg hw/pl110.c -- there > are two different DeviceInfo devices but since the underlying > implementation is the same you definitely don't want to have > two separate VMStateDescription structures to get out of sync. Yep. i8254/apic/ioapic vs. (upcoming) i8254-kvm/apic-kvm/ioapic-kvm will provide further use cases. Jan
Peter Maydell <peter.maydell@linaro.org> wrote: > On 23 March 2011 14:19, Juan Quintela <quintela@redhat.com> wrote: >> Peter Maydell <peter.maydell@linaro.org> wrote: >> ARM people are sending lots of vmstate changes, I guess/hope that >> somebody is trying to get it working. >> >> /me looks at Peter O:-), hint, hint, ... > > Well, the main thing I care about currently (or did back before > Christmas which is when I sent a patchset to add save/restore > to a pile of ARM devices) is simple save-and-restore for debugging > use. The rest is just that patches don't get through code review > unless they get the vmstate stuff right, and I care about not > being rejected :-) > > I think it's still the case that there are devices in > some of the ARM devboards with no save/restore support > at all. I would really prefer it if the default for a > device was "I do not support this" with the things like > USB where somebody has audited them as genuinely needing > no save/restore code explicitly marked as "this is OK"; > then we could easily determine what needed fixing and > not offer a broken facility to users. I agree, but that means (again), review of all devices to change the defaults. It is on my ToDo list (but my ToDo list is huge :-( >> Any idea if there are images for testing ARM? > > There are prebuilt images on Aurelien's website for > ARM and others, which is the simplest thing: > http://www.aurel32.net/info/debian_arm_qemu.php That images don't migrate for me at all. Guest got hung after migration, at least some state (probably irq's) are not passed correctly. Later, Juan.
On 23 March 2011 15:13, Juan Quintela <quintela@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> wrote: >> There are prebuilt images on Aurelien's website for >> ARM and others, which is the simplest thing: >> http://www.aurel32.net/info/debian_arm_qemu.php > > That images don't migrate for me at all. Guest got hung after > migration, at least some state (probably irq's) are not passed > correctly. Yeah, I said they were probably buggy. The only thing I've ever tested is vmsave/restore for versatilepb, and that was a few months ago now. -- PMM
Anthony Liguori <anthony@codemonkey.ws> wrote: > On 03/23/2011 09:17 AM, Juan Quintela wrote: >> Anthony Liguori<anthony@codemonkey.ws> 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. 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". >> 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. >> 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. Later, Juan.
On 03/23/2011 10:26 AM, Juan Quintela wrote: > Anthony Liguori<anthony@codemonkey.ws> wrote: >> On 03/23/2011 09:17 AM, Juan Quintela wrote: >>> Anthony Liguori<anthony@codemonkey.ws> 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.
On 03/23/2011 10:00 AM, Peter Maydell wrote: > On 23 March 2011 14:52, Anthony Liguori<anthony@codemonkey.ws> wrote: >> 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. > That's a feature, not a bug. Consider eg hw/pl110.c -- there > are two different DeviceInfo devices but since the underlying > implementation is the same you definitely don't want to have > two separate VMStateDescription structures to get out of sync. No, it's a bug. Migration uses the VMStateDescription name as a section identifier. The section identifiers MUST be unique for a given device. Otherwise, if both devices are present, migration fails miserably. It also means that if the wrong devices are created on the destination, instead of predictable failure, you get unpredictable guest corruption. The Right Way to support what you're describing above is to have a single VMStateField array and two VMStateDescriptions. IOW: VMStateField pl110_fields[] = { VMSTATE_INT32(versatile, pl110_state), VMSTATE_UINT32_ARRAY(timing, pl110_state, 4), VMSTATE_UINT32(cr, pl110_state), VMSTATE_UINT32(upbase, pl110_state), VMSTATE_UINT32(lpbase, pl110_state), VMSTATE_UINT32(int_status, pl110_state), .... }; VMStateDescription vmstate_pl110 = { .name = "pl110", ... .fields = pl110_fields, }; VMStateDescription vmstate_pl110_versatile = { .name = "pl110-versatile", ... .fields = pl110_fields, }; Merging VMStateDescription with DeviceInfo will force this on people making it difficult to make this mistake again. Regards, Anthony Liguori > -- PMM >
On 2011-03-23 17:27, Anthony Liguori wrote: > On 03/23/2011 10:00 AM, Peter Maydell wrote: >> On 23 March 2011 14:52, Anthony Liguori<anthony@codemonkey.ws> wrote: >>> 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. >> That's a feature, not a bug. Consider eg hw/pl110.c -- there >> are two different DeviceInfo devices but since the underlying >> implementation is the same you definitely don't want to have >> two separate VMStateDescription structures to get out of sync. > > No, it's a bug. > > Migration uses the VMStateDescription name as a section identifier. The > section identifiers MUST be unique for a given device. Otherwise, if > both devices are present, migration fails miserably. It also means that > if the wrong devices are created on the destination, instead of > predictable failure, you get unpredictable guest corruption. This is true for incompatible devices, but not for those that are perfectly exchangeable (like in-kernel vs. user-space irqchip). Jan
Anthony Liguori <anthony@codemonkey.ws> wrote: > On 03/23/2011 10:26 AM, Juan Quintela wrote: >> Anthony Liguori<anthony@codemonkey.ws> wrote: >>> On 03/23/2011 09:17 AM, Juan Quintela wrote: >>>> Anthony Liguori<anthony@codemonkey.ws> 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.
On 23 March 2011 16:27, Anthony Liguori <anthony@codemonkey.ws> wrote: > Migration uses the VMStateDescription name as a section identifier. The > section identifiers MUST be unique for a given device. Otherwise, if both > devices are present, migration fails miserably. So how does it work if you have two devices of the same type in the system? I'd assumed that the unique identifier would be one based on the actually instantiated devices. > It also means that if the > wrong devices are created on the destination, instead of predictable > failure, you get unpredictable guest corruption. > > The Right Way to support what you're describing above is to have a single > VMStateField array and two VMStateDescriptions. IOW: This doesn't make sense because it's decoupling the information about minimum version ID etc (in the VMStateDescription) from the information about which fields are in which version (which is in the VMStateField array when it's initialised via VMSTATE_*_V() macros). So although you get to avoid duplicating all the fields in the arrangement you suggest you still end up with version_id, minimum_version_id etc being duplicated and needing to stay in sync. -- PMM
On 03/23/2011 11:45 AM, Peter Maydell wrote: > On 23 March 2011 16:27, Anthony Liguori<anthony@codemonkey.ws> wrote: >> Migration uses the VMStateDescription name as a section identifier. The >> section identifiers MUST be unique for a given device. Otherwise, if both >> devices are present, migration fails miserably. > So how does it work if you have two devices of the same type > in the system? I'd assumed that the unique identifier would > be one based on the actually instantiated devices. It's a combination of the section name + a device instance ID that has some magic behind making it unique and stable. But instance ID is not unique on it's own. >> It also means that if the >> wrong devices are created on the destination, instead of predictable >> failure, you get unpredictable guest corruption. >> >> The Right Way to support what you're describing above is to have a single >> VMStateField array and two VMStateDescriptions. IOW: > This doesn't make sense because it's decoupling the information > about minimum version ID etc (in the VMStateDescription) from > the information about which fields are in which version (which > is in the VMStateField array when it's initialised via > VMSTATE_*_V() macros). So although you get to avoid duplicating > all the fields in the arrangement you suggest you still end > up with version_id, minimum_version_id etc being duplicated > and needing to stay in sync. So maybe we just need to remove name from VMStateDescription and force it to come from DeviceInfo. Regards, Anthony Liguori > -- PMM >
On 03/23/2011 11:36 AM, Jan Kiszka wrote: > > This is true for incompatible devices, but not for those that are > perfectly exchangeable (like in-kernel vs. user-space irqchip). I view this as the exception that proves the rule. We should handle cases like this as very special cases. I've found almost close to a dozen cases where we've got devices with conflicting names that aren't compatible and so far, none that truly are. Regards, Anthony Liguori > Jan >
diff --git a/Makefile b/Makefile index 5f9b120..f813208 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,8 @@ Makefile: ; configure: ; .PHONY: all clean cscope distclean dvi html info install install-doc \ - pdf recurse-all speed tar tarbin test build-all + pdf recurse-all speed tar tarbin test build-all check-vmstate check \ + check-help $(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw) @@ -71,7 +72,9 @@ defconfig: -include config-all-devices.mak -build-all: $(DOCS) $(TOOLS) recurse-all +TESTS=test-vmstate + +build-all: $(DOCS) $(TOOLS) $(TESTS) recurse-all config-host.h: config-host.h-timestamp config-host.h-timestamp: config-host.mak @@ -146,6 +149,33 @@ trace-dtrace.o: trace-dtrace.dtrace $(GENERATED_HEADERS) simpletrace.o: simpletrace.c $(GENERATED_HEADERS) +vmstate-schema.c: $(SRC_PATH)/vmstate/schema.json + $(call quiet-command,$(SRC_PATH)/scripts/json2c.sh < $^ > $@," GEN $@") + +test-vmstate-obj-y = qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o +test-vmstate-obj-y += json-streamer.o json-lexer.o json-parser.o +test-vmstate-obj-y += vmstate-schema.o qemu-malloc.o $(oslib-obj-y) + +test-vmstate: test-vmstate.o $(test-vmstate-obj-y) + +check-vmstate: test-vmstate + @./test-vmstate + +check: check-vmstate + +check-help: + @echo " make check run all check tests against build" + @echo " make check-vmstate run VMState checks" + @echo " make test-report.html make an HTML report of make check" + +test-report.log: $(TESTS) + @gtester -k -o $@ $^ + +test-report.html: test-report.log + @gtester-report $^ > $@ + +-include $(SUBDIR_DEVICES_MAK_DEP) + version.o: $(SRC_PATH)/version.rc config-host.mak $(call quiet-command,$(WINDRES) -I. -o $@ $<," RC $(TARGET_DIR)$@") diff --git a/scripts/json2c.sh b/scripts/json2c.sh new file mode 100755 index 0000000..0947211 --- /dev/null +++ b/scripts/json2c.sh @@ -0,0 +1,5 @@ +#!/bin/sh + +echo 'const char *vmstate_schema_json = ' +sed -e "s:\":\':g;s:^\(.*\)$: \"\1\":g" +echo ' ;' diff --git a/test-vmstate.c b/test-vmstate.c new file mode 100644 index 0000000..e2ac711 --- /dev/null +++ b/test-vmstate.c @@ -0,0 +1,142 @@ +/* + * VMState test case + * + * Copyright IBM, Corp. 2011 + * + * Authors: + * Anthony Liguori <aliguori@us.ibm.com> + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + */ + +#include <glib.h> +#include "qemu-objects.h" + +/* TODO + * 1) verify subsection semantics + */ + +extern const char *vmstate_schema_json; + +static void compare_section(const char *device, QDict *old, QDict *new) +{ + const QDictEntry *e; + int old_version, new_version; + + old_version = qdict_get_int(old, "__version__"); + new_version = qdict_get_int(new, "__version__"); + + 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); + } + + for (e = qdict_first(new); e; e = qdict_next(new, e)) { + QObject *old_field, *new_field; + + if (strcmp(e->key, "__version__") == 0) { + continue; + } + + if (!qdict_haskey(old, e->key)) { + g_error("You broke migration by adding a new field, `%s', to device `%s', version %d, without changing version.\n", + e->key, device, new_version); + } + + old_field = qdict_get(old, e->key); + new_field = qdict_get(new, e->key); + + if (qobject_type(old_field) != qobject_type(new_field)) { + g_error("You broke migration by changing the type of field, `%s', in device `%s', version %d, without changing version.\n", + e->key, device, new_version); + } + + if (qobject_type(old_field) == QTYPE_QSTRING) { + const char *old_field_type, *new_field_type; + + old_field_type = qdict_get_str(old, e->key); + new_field_type = qdict_get_str(new, e->key); + + if (strcmp(old_field_type, new_field_type) != 0) { + g_error("You broke migration by changing the type of field, `%s' in device `%s', version %d from `%s' to `%s'\n", + e->key, device, new_version, old_field_type, new_field_type); + } + } else { + char buffer[1024]; + + snprintf(buffer, sizeof(buffer), "%s.%s", device, e->key); + compare_section(buffer, qobject_to_qdict(old_field), qobject_to_qdict(new_field)); + } + } + + for (e = qdict_first(old); e; e = qdict_next(old, e)) { + if (!qdict_haskey(new, e->key)) { + g_error("You broke migration by removing the field, `%s', from device `%s', version %d, without changing version.\n", + e->key, device, new_version); + } + } +} + +static void compare_schema(QDict *old, QDict *new) +{ + const QDictEntry *e; + + for (e = qdict_first(new); e; e = qdict_next(new, e)) { + if (!qdict_haskey(old, e->key)) { + g_error("New device introduced, '%s', that's not in current schema. Please update.\n", e->key); + } + + compare_section(e->key, qobject_to_qdict(qdict_get(old, e->key)), + qobject_to_qdict(qdict_get(new, e->key))); + } +} + +static QObject *read_current_schema(void) +{ + char buffer[65536]; + int fd; + int ret; + size_t offset = 0; + ssize_t len; + + ret = system("i386-softmmu/qemu -vmstate-dump > /tmp/schema.json"); + g_assert_cmpint(ret, ==, 0); + + fd = open("/tmp/schema.json", O_RDONLY); + g_assert_cmpint(fd, !=, -1); + + do { + len = read(fd, buffer + offset, sizeof(buffer) - 1 - offset); + offset += len; + } while (len > 0); + + buffer[offset] = 0; + + close(fd); + + return qobject_from_json(buffer); +} + +static void test_missing_fields(void) +{ + QObject *schema = qobject_from_json(vmstate_schema_json); + QObject *cur_schema = read_current_schema(); + + compare_schema(qobject_to_qdict(schema), qobject_to_qdict(cur_schema)); + + qobject_decref(schema); + qobject_decref(cur_schema); +} + +int main(int argc, char **argv) +{ + g_test_init(&argc, &argv, NULL); + + g_test_add_func("/latest/schema-change", test_missing_fields); + + g_test_run(); + + return 0; +} 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", + "idt": { + "flags": "uint32", + "limit": "uint32", + "selector": "uint32", + "base": "uint32", + "__version__": 1 + }, + "intercept_cr_write": "uint16", + "nmi_injected": "uint8", + "vm_hsave": "uint64", + "fpregs-fpregs_is_0": "fpreg", + "fpus_vmstate": "uint16", + "interrupt_injected": "int32", + "intercept_exceptions": "uint32", + "system_time_msr": "uint64", + "hflags2": "uint32", + "sysenter_eip": "uint32", + "vm_vmcb": "uint64", + "mtrr_fixed": "uint64", + "mce_banks": "uint64", + "nmi_pending": "uint8", + "fptag_vmstate": "uint16", + "pat": "uint64", + "intercept_dr_read": "uint16", + "mcg_ctl": "uint64", + "v_tpr": "uint8", + "hflags": "uint32", + "ymmh_regs": { + "_q[1]": "uint64", + "_q[0]": "uint64", + "__version__": 1 + }, + "mp_state": "uint32", + "dr": "uint32", + "mcg_status": "uint64", + "cr[4]": "uint32", + "cr[3]": "uint32", + "cr[2]": "uint32", + "cr[0]": "uint32", + "xcr0": "uint64", + "xmm_regs": { + "_q[1]": "uint64", + "_q[0]": "uint64", + "__version__": 1 + }, + "mtrr_deftype": "uint64", + "intercept": "uint64", + "exception_injected": "int32", + "intercept_cr_read": "uint16", + "gdt": { + "flags": "uint32", + "limit": "uint32", + "selector": "uint32", + "base": "uint32", + "__version__": 1 + }, + "sipi_vector": "uint32", + "sysenter_cs": "uint32", + "eip": "uint32", + "tsc": "uint64", + "ldt": { + "flags": "uint32", + "limit": "uint32", + "selector": "uint32", + "base": "uint32", + "__version__": 1 + }, + "soft_interrupt": "uint8", + "regs": "uint32", + "sysenter_esp": "uint32", + "fpuc": "uint16", + "wall_clock_msr": "uint64", + "smbase": "uint32", + "xstate_bv": "uint64", + "mxcsr": "uint32", + "halted": "uint32", + "tsc_aux": "uint64", + "eflags": "uint32", + "fpregs-fpregs_is_1_no_mmx": "fpreg_1_no_mmx", + "segs": { + "flags": "uint32", + "limit": "uint32", + "selector": "uint32", + "base": "uint32", + "__version__": 1 + }, + "tr": { + "flags": "uint32", + "limit": "uint32", + "selector": "uint32", + "base": "uint32", + "__version__": 1 + }, + "fpregs-fpregs_is_1_mmx": "fpreg_1_mmx", + "intercept_dr_write": "uint16", + "mtrr_var": { + "base": "uint64", + "mask": "uint64", + "__version__": 1 + }, + "__version__": 12, + "has_error_code": "uint8", + "fpregs_format_vmstate": "uint16" + }, + "PCIBUS": { + "irq_count": "int32", + "nirq": "int32 equal", + "__version__": 1 + }, + "ne2000": { + "ne2000": { + "stop": "uint32", + "imr": "uint8", + "rsr": "uint8", + "start": "uint32", + "rsar": "uint32", + "rcnt": "uint16", + "phys": "buffer", + "isr": "uint8", + "dcfg": "uint8", + "unused": "unused_buffer", + "mem": "buffer", + "mult": "buffer", + "tsr": "uint8", + "tpsr": "uint8", + "tcnt": "uint16", + "curpag": "uint8", + "rxcr": "uint8", + "boundary": "uint8", + "__version__": 2, + "cmd": "uint8" + }, + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "__version__": 3 + }, + "isa-vga": { + "state": { + "ar_index": "uint8", + "dac_cache": "buffer", + "palette": "buffer", + "dac_write_index": "uint8", + "vbe_start_addr": "uint32", + "vbe_line_offset": "uint32", + "gr": "buffer", + "msr": "uint8", + "st01": "uint8", + "st00": "uint8", + "dac_state": "uint8", + "vbe_bank_mask": "uint32", + "dac_read_index": "uint8", + "gr_index": "uint8", + "vbe_index": "uint16", + "ar_flip_flop": "int32", + "cr": "buffer", + "cr_index": "uint8", + "vbe_regs": "uint16", + "dac_sub_index": "uint8", + "sr": "buffer", + "is_vbe_vmstate": "uint8 equal", + "bank_offset": "int32", + "fcr": "uint8", + "latch": "uint32", + "sr_index": "uint8", + "ar": "buffer", + "__version__": 2 + }, + "__version__": 1 + }, + "hpet": { + "num_timers": "uint8", + "isr": "uint64", + "config": "uint64", + "hpet_counter": "uint64", + "timer": { + "tn": "uint8", + "wrap_flag": "uint8", + "period": "uint64", + "config": "uint64", + "fsb": "uint64", + "qemu_timer": "timer", + "__version__": 1, + "cmp": "uint64" + }, + "__version__": 2 + }, + "intel-hda": { + "st": { + "lvi": "uint32", + "cbl": "uint32", + "fmt": "uint32", + "bdlp_ubase": "uint32", + "ctl": "uint32", + "__version__": 1, + "bdlp_lbase": "uint32", + "lpib": "uint32" + }, + "wall_base_ns": "int64", + "wall_clk": "uint32", + "int_ctl": "uint32", + "corb_size": "uint32", + "rirb_size": "uint32", + "dp_ubase": "uint32", + "dp_lbase": "uint32", + "corb_sts": "uint32", + "rirb_sts": "uint32", + "corb_rp": "uint32", + "g_ctl": "uint32", + "rirb_cnt": "uint32", + "ics": "uint32", + "icw": "uint32", + "corb_wp": "uint32", + "rirb_wp": "uint32", + "state_sts": "uint32", + "corb_ubase": "uint32", + "rirb_ubase": "uint32", + "corb_ctl": "uint32", + "rirb_ctl": "uint32", + "wake_en": "uint32", + "irr": "uint32", + "corb_lbase": "uint32", + "rirb_lbase": "uint32", + "int_sts": "uint32", + "pci": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "rirb_count": "uint32", + "__version__": 1 + }, + "serial": { + "state": { + "msr": "uint8", + "mcr": "uint8", + "fcr_vmstate": "uint8", + "scr": "uint8", + "iir": "uint8", + "rbr": "uint8", + "divider": "uint16", + "ier": "uint8", + "__version__": 3, + "lsr": "uint8", + "lcr": "uint8" + }, + "__version__": 3 + }, + "vmmouse": { + "nb_queue": "uint16", + "status": "uint16", + "queue": "uint32", + "queue_size": "int32 equal", + "absolute": "uint8", + "__version__": 0 + }, + "port92": { + "outport": "uint8", + "__version__": 1 + }, + "ioh-3240-express-root-port": { + "port.br.dev.exp.aer_log": { + "log_max": "uint16", + "log": { + "flags": "uint16", + "header": "uint32", + "status": "uint32", + "prefix": "uint32", + "__version__": 1, + "source_id": "uint16" + }, + "log_num": "uint16", + "__version__": 1 + }, + "__version__": 1, + "port.br.dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + } + }, + "ioapic": { + "ioredtbl": "uint64", + "unused290": "unused_buffer", + "irr": "uint32", + "ioregsel": "uint8", + "__version__": 3, + "id": "uint8" + }, + "xio3130-express-downstream-port": { + "port.br.dev.exp.aer_log": { + "log_max": "uint16", + "log": { + "flags": "uint16", + "header": "uint32", + "status": "uint32", + "prefix": "uint32", + "__version__": 1, + "source_id": "uint16" + }, + "log_num": "uint16", + "__version__": 1 + }, + "__version__": 1, + "port.br.dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + } + }, + "ps2mouse": { + "mouse_type": "uint8", + "mouse_wrap": "uint8", + "common": { + "queue.count": "int32", + "queue.data": "buffer", + "queue.wptr": "int32", + "write_cmd": "int32", + "queue.rptr": "int32", + "__version__": 3 + }, + "mouse_detect_state": "uint8", + "mouse_sample_rate": "uint8", + "mouse_resolution": "uint8", + "mouse_buttons": "uint8", + "__version__": 2, + "mouse_status": "uint8", + "mouse_dz": "int32", + "mouse_dy": "int32", + "mouse_dx": "int32" + }, + "xio3130-express-upstream-port": { + "br.dev.exp.aer_log": { + "log_max": "uint16", + "log": { + "flags": "uint16", + "header": "uint32", + "status": "uint32", + "prefix": "uint32", + "__version__": 1, + "source_id": "uint16" + }, + "log_num": "uint16", + "__version__": 1 + }, + "br.dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "__version__": 1 + }, + "e1000": { + "unused965": "unused_buffer", + "unused964": "unused_buffer", + "eecd_state.reading": "uint16", + "mac_reg[MPC]": "uint32", + "mac_reg[IMC]": "uint32", + "tx.tcp": "int8", + "mac_reg[WUFC]": "uint32", + "mac_reg[GPRC]": "uint32", + "tx.ip": "int8", + "tx.data": "buffer", + "mac_reg[TDH]": "uint32", + "mac_reg[RDH]": "uint32", + "tx.size": "uint16", + "rxbuf_min_shift": "uint32", + "mac_reg[VET]": "uint32", + "mac_reg[TPT]": "uint32", + "mac_reg[TDT]": "uint32", + "mac_reg[RDT]": "uint32", + "tx.sum_needed": "uint8", + "mac_reg[VFTA]": "uint32", + "mac_reg[TDBAL]": "uint32", + "mac_reg[TDBAH]": "uint32", + "mac_reg[RDBAL]": "uint32", + "mac_reg[RDBAH]": "uint32", + "tx.mss": "uint16", + "phy_reg": "uint16", + "mac_reg[EECD]": "uint32", + "mac_reg[MTA]": "uint32", + "mac_reg[PBA]": "uint32", + "mac_reg[IMS]": "uint32", + "mac_reg[ICS]": "uint32", + "mac_reg[SWSM]": "uint32", + "tx.header": "buffer", + "mac_reg[TOTL]": "uint32", + "mac_reg[TOTH]": "uint32", + "mac_reg[TCTL]": "uint32", + "mac_reg[RCTL]": "uint32", + "tx.paylen": "uint32", + "mac_reg[TPR]": "uint32", + "mac_reg[ICR]": "uint32", + "mac_reg[TXDCTL]": "uint32", + "mac_reg[LEDCTL]": "uint32", + "mac_reg[MDIC]": "uint32", + "eecd_state.old_eecd": "uint32", + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "eecd_state.bitnum_out": "uint16", + "tx.tso_frames": "uint16", + "mac_reg[TDLEN]": "uint32", + "mac_reg[RDLEN]": "uint32", + "eeprom_data": "uint16", + "eecd_state.bitnum_in": "uint16", + "mac_reg[MANC]": "uint32", + "mac_reg[GPTC]": "uint32", + "mac_reg[STATUS]": "uint32", + "tx.hdr_len": "uint8", + "mac_reg[TORL]": "uint32", + "mac_reg[TORH]": "uint32", + "mac_reg[EERD]": "uint32", + "mac_reg[CTRL]": "uint32", + "__version__": 2, + "tx.tucse": "uint16", + "tx.tucso": "uint8", + "tx.tucss": "uint8", + "tx.ipcse": "uint16", + "tx.ipcso": "uint8", + "tx.ipcss": "uint8", + "eecd_state.val_in": "uint32", + "mac_reg[RA]": "uint32", + "rxbuf_size": "uint32" + }, + "vmware_vga": { + "chip": { + "index": "int32", + "scratch": "uint32", + "fb_size": "int32", + "guest": "uint32", + "syncing": "int32", + "enable": "int32", + "svgaid": "uint32", + "new_width": "int32", + "config": "int32", + "cursor.y": "int32", + "cursor.x": "int32", + "cursor.on": "int32", + "cursor.id": "int32", + "depth": "int32 equal", + "new_height": "int32", + "__version__": 0 + }, + "card": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "__version__": 0 + }, + "rtl8139/hotplug_ready": { + "__version__": 1 + }, + "mc146818rtc": { + "current_tm.tm_mday": "int32", + "current_tm.tm_wday": "int32", + "period": "uint32", + "next_second_time": "int64", + "current_tm.tm_hour": "int32", + "cmos_data": "buffer", + "cmos_index": "uint8", + "irq_coalesced": "uint32", + "periodic_timer": "timer", + "current_tm.tm_sec": "int32", + "second_timer": "timer", + "current_tm.tm_year": "int32", + "second_timer2": "timer", + "next_periodic_time": "int64", + "__version__": 2, + "current_tm.tm_min": "int32", + "current_tm.tm_mon": "int32" + }, + "uhci": { + "frame_timer": "timer", + "frnum": "uint16", + "ports": { + "ctrl": "uint16", + "__version__": 1 + }, + "status2": "uint8", + "status": "uint16", + "fl_base_addr": "uint32", + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "num_ports_vmstate": "uint8 equal", + "expire_time": "int64", + "__version__": 2, + "sof_timing": "uint8", + "cmd": "uint16", + "intr": "uint16" + }, + "PIIX3": { + "pci_irq_levels": "int32", + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "__version__": 3 + }, + "fdc": { + "state": { + "timer0": "uint8", + "msr": "uint8", + "status2": "uint8", + "dor_vmstate": "uint8", + "lock": "uint8", + "dsr": "uint8", + "data_len": "uint32", + "tdr": "uint8", + "data_dir": "uint8", + "timer1": "uint8", + "status0": "uint8", + "config": "uint8", + "pwrd": "uint8", + "precomp_trk": "uint8", + "data_pos": "uint32", + "srb": "uint8", + "sra": "uint8", + "eot": "uint8", + "status1": "uint8", + "num_floppies": "uint8 equal", + "drives": { + "track": "uint8", + "head": "uint8", + "sect": "uint8", + "__version__": 1 + }, + "fifo": "uint8", + "__version__": 2, + "data_state": "uint8" + }, + "__version__": 2 + }, + "cirrus_vga": { + "cirrus_vga": { + "cirrus_hidden_dac_lockindex": "uint8", + "vga.ar_flip_flop": "int32", + "vga.cr_index": "uint8", + "vga.ar_index": "uint8", + "vga.gr_index": "uint8", + "vga.sr_index": "uint8", + "cirrus_hidden_dac_data": "uint8", + "vga.fcr": "uint8", + "vga.bank_offset": "int32", + "vga.dac_state": "uint8", + "vga.latch": "uint32", + "vga.dac_write_index": "uint8", + "vga.dac_read_index": "uint8", + "cirrus_shadow_gr0": "uint8", + "hw_cursor_y": "uint32", + "vga.dac_sub_index": "uint8", + "hw_cursor_x": "uint32", + "vga.st01": "uint8", + "vga.st00": "uint8", + "vga.cr": "buffer", + "vga.ar": "buffer", + "vga.gr": "buffer", + "vga.sr": "buffer", + "cirrus_shadow_gr1": "uint8", + "vga.palette": "buffer", + "vga.msr": "uint8", + "__version__": 2, + "vga.dac_cache": "buffer" + }, + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "__version__": 2 + }, + "ib700_wdt": { + "timer": "timer", + "__version__": 0 + }, + "usb-kbd": { + "kbd.keys": "int32", + "idle": "uint8", + "kbd.leds": "uint8", + "n": "uint32", + "protocol": "int32", + "head": "uint32", + "kbd.modifiers": "uint16", + "dev": { + "state": "int32", + "remote_wakeup": "int32", + "addr": "uint8", + "setup_len": "int32", + "setup_state": "int32", + "setup_buf": "uint8", + "setup_index": "int32", + "__version__": 1 + }, + "__version__": 1, + "kbd.key": "uint8", + "kbd.keycodes": "uint32" + }, + "eeprom": { + "unused141": "unused_buffer", + "command": "uint8", + "addrbits": "uint8", + "contents": "uint16", + "writeable": "uint8", + "size": "uint16 equal", + "size-hack": "uint16_from_uint8", + "address": "uint8", + "tick": "uint8", + "__version__": 20061113, + "eedo": "uint8", + "eesk": "uint8", + "eecs": "uint8", + "data": "uint16" + }, + "hda-audio": { + "st": { + "gain_right": "uint32", + "mute_right": "bool", + "buf": "buffer", + "channel": "uint32", + "format": "uint32", + "bpos": "uint32", + "stream": "uint32", + "__version__": 1, + "gain_left": "uint32", + "mute_left": "bool" + }, + "running": "bool", + "__version__": 1 + }, + "apic": { + "lvt": "uint32", + "next_time": "int64", + "divide_conf": "uint32", + "dest_mode": "uint8", + "icr": "uint32", + "isr": "uint32", + "esr": "uint32", + "tmr": "uint32", + "tpr": "uint8", + "apicbase": "uint32", + "irr": "uint32", + "count_shift": "int32", + "initial_count_load_time": "int64", + "timer": "timer", + "log_dest": "uint8", + "__version__": 3, + "id": "uint8", + "spurious_vec": "uint32", + "initial_count": "uint32", + "arb_id": "uint8" + }, + "lsiscsi": { + "sxfer": "uint8", + "istat1": "uint8", + "script_ram": "buffer", + "respid1": "uint8", + "temp": "uint32", + "scratch": "buffer", + "mmws": "uint32", + "mmrs": "uint32", + "sstat1": "uint8", + "scntl1": "uint8", + "ctest5": "uint8", + "csbc": "uint32", + "msg": "buffer", + "sdid": "uint8", + "stest2": "uint8", + "dsp": "uint32", + "dsa": "uint32", + "ctest3": "uint8", + "dmode": "uint8", + "ccntl0": "uint8", + "dsps": "uint32", + "dcmd": "uint8", + "pmjad1": "uint32", + "stime0": "uint8", + "ua": "uint32", + "dcntl": "uint8", + "dstat": "uint8", + "sfs": "uint32", + "dien": "uint8", + "ssid": "uint8", + "scid": "uint8", + "dfifo": "uint8", + "rbc": "uint32", + "scntl2": "uint8", + "msg_action": "int32", + "sfbr": "uint8", + "drs": "uint32", + "dbc": "uint32", + "istat0": "uint8", + "sidl": "uint8", + "mbox1": "uint8", + "mbox0": "uint8", + "stest3": "uint8", + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "dbms": "uint32", + "respid0": "uint8", + "sien1": "uint8", + "sien0": "uint8", + "sist1": "uint8", + "sist0": "uint8", + "sstat0": "uint8", + "scntl0": "uint8", + "ctest4": "uint8", + "sbr": "uint8", + "sbc": "uint32", + "socl": "uint8", + "waiting": "int32", + "ccntl1": "uint8", + "pmjad2": "uint32", + "dnad64": "uint32", + "sbms": "uint32", + "stest1": "uint8", + "carry": "int32", + "ctest2": "uint8", + "ia": "uint32", + "dnad": "uint32", + "msg_len": "int32", + "__version__": 0, + "sense": "int32", + "scntl3": "uint8" + }, + "kvmclock": { + "clock": "uint64", + "__version__": 1 + }, + "fw_cfg": { + "cur_entry": "uint16", + "cur_offset-hack": "int32_as_uint16", + "cur_offset": "uint32", + "__version__": 2 + }, + "ac97": { + "mixer_data": "buffer", + "glob_sta": "uint32", + "unused1232": "unused_buffer", + "glob_cnt": "uint32", + "bm_regs": { + "lvi": "uint8", + "civ": "uint8", + "cr": "uint8", + "bd.addr": "uint32", + "bd_valid": "uint32", + "sr": "uint16", + "bdbar": "uint32", + "bd.ctl_len": "uint32", + "piv": "uint8", + "__version__": 1, + "picb": "uint16" + }, + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "cas": "uint32", + "__version__": 3 + }, + "es1370": { + "status": "uint32", + "codec": "uint32", + "sctl": "uint32", + "chan": { + "shift": "uint32", + "scount": "uint32", + "frame_cnt": "uint32", + "frame_addr": "uint32", + "leftover": "uint32", + "__version__": 2 + }, + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "mempage": "uint32", + "ctl": "uint32", + "__version__": 2 + }, + "usb-hub": { + "ports": { + "wPortStatus": "uint16", + "wPortChange": "uint16", + "__version__": 1 + }, + "dev": { + "state": "int32", + "remote_wakeup": "int32", + "addr": "uint8", + "setup_len": "int32", + "setup_state": "int32", + "setup_buf": "uint8", + "setup_index": "int32", + "__version__": 1 + }, + "__version__": 1 + }, + "vga": { + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "vga": { + "ar_index": "uint8", + "dac_cache": "buffer", + "palette": "buffer", + "dac_write_index": "uint8", + "vbe_start_addr": "uint32", + "vbe_line_offset": "uint32", + "gr": "buffer", + "msr": "uint8", + "st01": "uint8", + "st00": "uint8", + "dac_state": "uint8", + "vbe_bank_mask": "uint32", + "dac_read_index": "uint8", + "gr_index": "uint8", + "vbe_index": "uint16", + "ar_flip_flop": "int32", + "cr": "buffer", + "cr_index": "uint8", + "vbe_regs": "uint16", + "dac_sub_index": "uint8", + "sr": "buffer", + "is_vbe_vmstate": "uint8 equal", + "bank_offset": "int32", + "fcr": "uint8", + "latch": "uint32", + "sr_index": "uint8", + "ar": "buffer", + "__version__": 2 + }, + "__version__": 2 + }, + "sb16": { + "freq": "int32", + "port": "uint32", + "can_write": "int32", + "dma": "uint32", + "left_till_irq": "int32", + "mixer_regs": "buffer", + "time_const": "int32", + "csp_regs": "buffer", + "in2_data": "buffer", + "fmt_bits": "int32", + "nzero": "int32", + "csp_mode": "uint8", + "csp_reg83": "buffer", + "out_data": "buffer", + "highspeed": "int32", + "block_size": "int32", + "csp_reg83w": "int32", + "csp_reg83r": "int32", + "ver": "uint32", + "v2x6": "int32", + "fmt_stereo": "int32", + "needed_bytes": "int32", + "csp_value": "uint8", + "last_read_byte": "uint8", + "fmt": "uint32", + "csp_param_dummy": "uint8", + "out_data_len": "int32", + "fmt_signed": "int32", + "speaker": "int32", + "irq": "uint32", + "bytes_per_second": "int32", + "mixer_nreg": "int32", + "csp_param": "uint8", + "use_hdma": "int32", + "hdma": "uint32", + "test_reg": "uint8", + "fifo": "int32", + "dma_auto": "int32", + "__version__": 1, + "in_index": "int32", + "cmd": "int32", + "csp_index": "uint8", + "align": "int32", + "dma_running": "int32" + }, + "ps2kbd": { + "scancode_set": "int32", + "common": { + "queue.count": "int32", + "queue.data": "buffer", + "queue.wptr": "int32", + "write_cmd": "int32", + "queue.rptr": "int32", + "__version__": 3 + }, + "translate": "int32", + "__version__": 3, + "scan_enabled": "int32" + }, + "mm-pckbd": { + "pending": "uint8", + "status": "uint8", + "mode": "uint8", + "write_cmd": "uint8", + "__version__": 3 + }, + "rtl8139": { + "conf.macaddr": "buffer", + "TCTR": "uint32", + "eeprom.eedi": "uint8", + "eeprom.eedo": "uint8", + "MultiIntr": "uint16", + "RxBufAddr": "uint32", + "TxThresh": "uint8", + "TxStatus": "uint32", + "Config3": "uint8", + "RxBuf": "uint32", + "eeprom.input": "uint16", + "TimerInt": "uint32", + "BasicModeStatus": "uint16", + "rtl8139_mmio_io_addr_dummy": "int32", + "IntrMask": "uint16", + "TxAddr": "uint32", + "RxBufPtr": "uint32", + "eeprom.contents": "uint16", + "RxMissed": "uint32", + "phys": "buffer", + "RxConfig": "uint32", + "NWayExpansion": "uint16", + "cplus_enabled": "uint32", + "Config1": "uint8", + "eeprom.eecs": "uint8", + "CSCR": "uint16", + "CpCmd": "uint16", + "Config4": "uint8", + "eeprom.address": "uint8", + "bChipCmdState": "uint8", + "currCPlusTxDesc": "uint32", + "currCPlusRxDesc": "uint32", + "IntrStatus": "uint16", + "tally_counters": { + "RxOk": "uint64", + "RxOkPhy": "uint64", + "RxERR": "uint32", + "MissPkt": "uint16", + "RxOkBrd": "uint64", + "TxOk": "uint64", + "TxAbt": "uint16", + "TxERR": "uint64", + "TxUndrn": "uint16", + "FAE": "uint16", + "TxMCol": "uint32", + "Tx1Col": "uint32", + "__version__": 1 + }, + "clock_enabled": "uint8", + "NWayLPAR": "uint16", + "currTxDesc": "uint32", + "mult": "buffer", + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "eeprom.mode": "int32", + "eeprom.eesk": "uint8", + "eeprom.tick": "uint32", + "TCTR_base": "int64", + "Cfg9346": "uint8", + "TxConfig": "uint32", + "eeprom.output": "uint16", + "BasicModeCtrl": "uint16", + "RxRingAddrLO": "uint32", + "unused3230": "unused_buffer", + "Config5": "uint8", + "RxBufferSize": "uint32", + "__version__": 4, + "RxRingAddrHI": "uint32", + "Config0": "uint8", + "NWayAdvert": "uint16" + }, + "i8254": { + "channels": { + "write_latch": "uint8", + "latched_count": "uint16", + "status_latched": "uint8", + "status": "uint8", + "write_state": "uint8", + "count": "int32", + "mode": "uint8", + "gate": "uint8", + "read_state": "uint8", + "next_transition_time": "int64", + "count_load_time": "int64", + "bcd": "uint8", + "rw_mode": "uint8", + "__version__": 2, + "count_latched": "uint8" + }, + "channels[0].irq_timer": "timer", + "__version__": 2 + }, + "i8259": { + "init4": "uint8", + "poll": "uint8", + "imr": "uint8", + "auto_eoi": "uint8", + "isr": "uint8", + "single_mode": "uint8", + "special_fully_nested_mode": "uint8", + "read_reg_select": "uint8", + "special_mask": "uint8", + "rotate_on_auto_eoi": "uint8", + "last_irr": "uint8", + "irr": "uint8", + "irq_base": "uint8", + "priority_add": "uint8", + "elcr": "uint8", + "__version__": 1, + "init_state": "uint8" + }, + "pckbd": { + "kbd": { + "pending": "uint8", + "status": "uint8", + "mode": "uint8", + "write_cmd": "uint8", + "__version__": 3 + }, + "__version__": 3 + }, + "pcnet": { + "state": { + "csr": "uint16", + "unused1712": "unused_buffer", + "buffer": "buffer", + "xmit_pos": "int32", + "prom": "buffer", + "isr": "int32", + "tdra": "uint32", + "poll_timer": "timer", + "lnkst": "int32", + "tx_busy": "int32", + "rap": "int32", + "bcr": "uint16", + "timer": "uint64", + "__version__": 3, + "rdra": "uint32" + }, + "pci_dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "__version__": 3 + }, + "i2c_bus": { + "saved_address": "uint8", + "__version__": 1 + }, + "i6300esb_wdt": { + "enabled": "int32", + "clock_scale": "int32", + "unlock_state": "int32", + "timer1_preload": "uint32", + "locked": "int32", + "stage": "int32", + "previous_reboot_flag": "int32", + "reboot_enabled": "int32", + "timer2_preload": "uint32", + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "int_type": "int32", + "timer": "timer", + "free_run": "int32", + "__version__": 720 + }, + "piix4_pm": { + "pci0_status": { + "down": "uint32", + "up": "uint32", + "__version__": 1 + }, + "pmcntrl": "uint16", + "tmr_overflow_time": "int64", + "apm": { + "apms": "uint8", + "apmc": "uint8", + "__version__": 1 + }, + "dev": { + "version_id": "int32 equal", + "config": "pci config", + "__version__": 2, + "irq_state": "pci irq state" + }, + "tmr_timer": "timer", + "pmen": "uint16", + "pmsts": "uint16", + "gpe": { + "sts": "uint16", + "__version__": 1, + "en": "uint16" + }, + "__version__": 2 + }, + "sysbus-fdc": { + "state": { + "timer0": "uint8", + "msr": "uint8", + "status2": "uint8", + "dor_vmstate": "uint8", + "lock": "uint8", + "dsr": "uint8", + "data_len": "uint32", + "tdr": "uint8", + "data_dir": "uint8", + "timer1": "uint8", + "status0": "uint8", + "config": "uint8", + "pwrd": "uint8", + "precomp_trk": "uint8", + "data_pos": "uint32", + "srb": "uint8", + "sra": "uint8", + "eot": "uint8", + "status1": "uint8", + "num_floppies": "uint8 equal", + "drives": { + "track": "uint8", + "head": "uint8", + "sect": "uint8", + "__version__": 1 + }, + "fifo": "uint8", + "__version__": 2, + "data_state": "uint8" + }, + "__version__": 2 + } +}
We've had lots of issues surrounding live migration breaking. This is because we haven't had a good way to validate that what we're migrating isn't changing underneath of us. This test works by first converting the in-tree schema to C source as a string. This is built into the test case. The test case will then query QEMU for the current schema. It converts both strings to QObjects via the JSON parser and then does a recursive diff. Unlike a simple diff command, this lets us say exactly what you did to break migration. For instance, you get error messages like: You broke migration by changing the type of field 'par' in device 'cpu', version 1 from 'uint32' to 'uint64'. It only covers devices that support VMState and it currently doesn't look at subsections yet. The in-tree schema needs to be updated whenever migration changes but this ends up being a very nice way to ensure that we're reviewing migration protocol changes. The test case is built in the default build, and can be run by `make check'. I've also added a `make check-help' to encourage people to use more specific check commands. Finally, there's a make test-report which generates a nice HTML version of the test output. The current test case would catch the couple recent migration regressions. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- Makefile | 34 ++- scripts/json2c.sh | 5 + test-vmstate.c | 142 +++++++ vmstate/schema.json | 1176 +++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1355 insertions(+), 2 deletions(-) create mode 100755 scripts/json2c.sh create mode 100644 test-vmstate.c create mode 100644 vmstate/schema.json