| Message ID | f75cb6f1f65dc9fb91e63db3fbe49711dea35060.1399892389.git.amit.shah@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/12/2014 05:16 AM, Amit Shah wrote: > This commit adds a new command, '-dump-vmstate', that takes a filename > as a parameter. When executed, QEMU will dump the vmstate information > for the machine type it's invoked with to the file, and quit. > > The JSON-format output can then be used to compare the vmstate info for > different QEMU versions, specifically to test whether live migration > would break due to changes in the vmstate data. Are we going to document that JSON format anywhere? Is it worth making it part of qapi-schema.json, whether to also expose the dump via a QMP command, or at a bare minimum to take advantage of some code generation from the qapi engine rather than doing it all by hand?
Hi, On (Mon) 12 May 2014 [06:51:54], Eric Blake wrote: > On 05/12/2014 05:16 AM, Amit Shah wrote: > > This commit adds a new command, '-dump-vmstate', that takes a filename > > as a parameter. When executed, QEMU will dump the vmstate information > > for the machine type it's invoked with to the file, and quit. > > > > The JSON-format output can then be used to compare the vmstate info for > > different QEMU versions, specifically to test whether live migration > > would break due to changes in the vmstate data. > > Are we going to document that JSON format anywhere? I suppose we should, I thought I should wait for comments here on any extra fields that people want. I suppose documenting would just be coming up with a schema, though? ... > Is it worth making > it part of qapi-schema.json, but documenting the entire schema is difficult, as each device will have its own schema text (due to the differing fields). At least that's how I understand it; please correct me if I'm wrong. > whether to also expose the dump via a QMP > command, or at a bare minimum to take advantage of some code generation > from the qapi engine rather than doing it all by hand? Surely -- I did give it some thought at the initial stages, but since I've not had to modify this much since I first wrote it, and it does work well, I'm just thinking it's alright if we do that later. Focus right now is on the tool itself. Amit
* Amit Shah (amit.shah@redhat.com) wrote: > This commit adds a new command, '-dump-vmstate', that takes a filename > as a parameter. When executed, QEMU will dump the vmstate information > for the machine type it's invoked with to the file, and quit. > > The JSON-format output can then be used to compare the vmstate info for > different QEMU versions, specifically to test whether live migration > would break due to changes in the vmstate data. > > This is based on a version from Andreas Färber posted here: > https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg03095.html > > A Python script that compares the output of such JSON dumps is included > in the following commit. > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > --- > include/migration/vmstate.h | 2 + > qemu-options.hx | 9 +++ > savevm.c | 134 ++++++++++++++++++++++++++++++++++++++++++++ > vl.c | 14 +++++ > 4 files changed, 159 insertions(+) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 7e45048..9829c0e 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -778,4 +778,6 @@ void vmstate_register_ram(struct MemoryRegion *memory, DeviceState *dev); > void vmstate_unregister_ram(struct MemoryRegion *memory, DeviceState *dev); > void vmstate_register_ram_global(struct MemoryRegion *memory); > > +void dump_vmstate_json_to_file(FILE *out_fp); > + > #endif > diff --git a/qemu-options.hx b/qemu-options.hx > index 781af14..d376227 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -3146,6 +3146,15 @@ STEXI > prepend a timestamp to each log message.(default:on) > ETEXI > > +DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate, > + "-dump-vmstate <file>\n" "", QEMU_ARCH_ALL) > +STEXI > +@item -dump-vmstate @var{file} > +@findex -dump-vmstate > +Dump json-encoded vmstate information for current machine type to file > +in @var{file} > +ETEXI > + > HXCOMM This is the last statement. Insert new options before this line! > STEXI > @end table > diff --git a/savevm.c b/savevm.c > index da8aa24..a4ce279 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -24,6 +24,7 @@ > > #include "config-host.h" > #include "qemu-common.h" > +#include "hw/boards.h" > #include "hw/hw.h" > #include "hw/qdev.h" > #include "net/net.h" > @@ -241,6 +242,139 @@ static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers = > QTAILQ_HEAD_INITIALIZER(savevm_handlers); > static int global_section_id; > > +static void dump_vmstate_vmsd(FILE *out_file, > + const VMStateDescription *vmsd, int indent, > + bool is_subsection); > + > +static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field, > + int indent) checkpatch points out that some tabs managed to get into that indent line. Generally I think this patch is OK and quite useful; two thoughts: 1) I was surprised it dumped every object type, rather than just those that are instantiated; I think the latter would be more use in some circumstances, since there's a load of weird and wonderful objects that exist and are very rarely used. 2) 'fields_exists' is a weird naming to put in the json file - it's a function pointer for determining if the field is going to be present; maybe renaming as 'conditional' would make sense. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On (Wed) 21 May 2014 [10:44:07], Dr. David Alan Gilbert wrote: > * Amit Shah (amit.shah@redhat.com) wrote: > > This commit adds a new command, '-dump-vmstate', that takes a filename > > as a parameter. When executed, QEMU will dump the vmstate information > > for the machine type it's invoked with to the file, and quit. > > > > The JSON-format output can then be used to compare the vmstate info for > > different QEMU versions, specifically to test whether live migration > > would break due to changes in the vmstate data. > > > > This is based on a version from Andreas Färber posted here: > > https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg03095.html > > > > A Python script that compares the output of such JSON dumps is included > > in the following commit. > > > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > --- > > include/migration/vmstate.h | 2 + > > qemu-options.hx | 9 +++ > > savevm.c | 134 ++++++++++++++++++++++++++++++++++++++++++++ > > vl.c | 14 +++++ > > 4 files changed, 159 insertions(+) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 7e45048..9829c0e 100644 > > --- a/include/migration/vmstate.h > > +++ b/include/migration/vmstate.h > > @@ -778,4 +778,6 @@ void vmstate_register_ram(struct MemoryRegion *memory, DeviceState *dev); > > void vmstate_unregister_ram(struct MemoryRegion *memory, DeviceState *dev); > > void vmstate_register_ram_global(struct MemoryRegion *memory); > > > > +void dump_vmstate_json_to_file(FILE *out_fp); > > + > > #endif > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 781af14..d376227 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -3146,6 +3146,15 @@ STEXI > > prepend a timestamp to each log message.(default:on) > > ETEXI > > > > +DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate, > > + "-dump-vmstate <file>\n" "", QEMU_ARCH_ALL) > > +STEXI > > +@item -dump-vmstate @var{file} > > +@findex -dump-vmstate > > +Dump json-encoded vmstate information for current machine type to file > > +in @var{file} > > +ETEXI > > + > > HXCOMM This is the last statement. Insert new options before this line! > > STEXI > > @end table > > diff --git a/savevm.c b/savevm.c > > index da8aa24..a4ce279 100644 > > --- a/savevm.c > > +++ b/savevm.c > > @@ -24,6 +24,7 @@ > > > > #include "config-host.h" > > #include "qemu-common.h" > > +#include "hw/boards.h" > > #include "hw/hw.h" > > #include "hw/qdev.h" > > #include "net/net.h" > > @@ -241,6 +242,139 @@ static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers = > > QTAILQ_HEAD_INITIALIZER(savevm_handlers); > > static int global_section_id; > > > > +static void dump_vmstate_vmsd(FILE *out_file, > > + const VMStateDescription *vmsd, int indent, > > + bool is_subsection); > > + > > +static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field, > > + int indent) > > checkpatch points out that some tabs managed to get into that indent line. Will fix. > Generally I think this patch is OK and quite useful; two thoughts: > 1) I was surprised it dumped every object type, rather than just those > that are instantiated; I think the latter would be more use in some > circumstances, since there's a load of weird and wonderful objects > that exist and are very rarely used. The idea is to be able to take a qemu binary and compare with another binary; if only fields that are instantiated are used, various invocations will have to be tried to find devices that may have broken. An alternative way of checking only devices which have been added to the running machine can be done via a monitor command (or a parameter to the existing cmdline option). But I'm not sure if that'll be more useful than the current one. > 2) 'fields_exists' is a weird naming to put in the json file - it's > a function pointer for determining if the field is going to be present; > maybe renaming as 'conditional' would make sense. Yea; I don't know if field_exists is going to be useful anyway. It's runtime info rather than static, so perhaps can just be dropped. Right now, anyway, the checker doesn't make use of this field at all. Thanks for taking a look! Amit
* Amit Shah (amit.shah@redhat.com) wrote: > On (Wed) 21 May 2014 [10:44:07], Dr. David Alan Gilbert wrote: > > * Amit Shah (amit.shah@redhat.com) wrote: > > > This commit adds a new command, '-dump-vmstate', that takes a filename > > > as a parameter. When executed, QEMU will dump the vmstate information > > > for the machine type it's invoked with to the file, and quit. > > > > > > The JSON-format output can then be used to compare the vmstate info for > > > different QEMU versions, specifically to test whether live migration > > > would break due to changes in the vmstate data. > > > > > > This is based on a version from Andreas Färber posted here: > > > https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg03095.html > > > > > > A Python script that compares the output of such JSON dumps is included > > > in the following commit. > > > > > > Signed-off-by: Amit Shah <amit.shah@redhat.com> > > > --- > > > include/migration/vmstate.h | 2 + > > > qemu-options.hx | 9 +++ > > > savevm.c | 134 ++++++++++++++++++++++++++++++++++++++++++++ > > > vl.c | 14 +++++ > > > 4 files changed, 159 insertions(+) > > > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > > index 7e45048..9829c0e 100644 > > > --- a/include/migration/vmstate.h > > > +++ b/include/migration/vmstate.h > > > @@ -778,4 +778,6 @@ void vmstate_register_ram(struct MemoryRegion *memory, DeviceState *dev); > > > void vmstate_unregister_ram(struct MemoryRegion *memory, DeviceState *dev); > > > void vmstate_register_ram_global(struct MemoryRegion *memory); > > > > > > +void dump_vmstate_json_to_file(FILE *out_fp); > > > + > > > #endif > > > diff --git a/qemu-options.hx b/qemu-options.hx > > > index 781af14..d376227 100644 > > > --- a/qemu-options.hx > > > +++ b/qemu-options.hx > > > @@ -3146,6 +3146,15 @@ STEXI > > > prepend a timestamp to each log message.(default:on) > > > ETEXI > > > > > > +DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate, > > > + "-dump-vmstate <file>\n" "", QEMU_ARCH_ALL) > > > +STEXI > > > +@item -dump-vmstate @var{file} > > > +@findex -dump-vmstate > > > +Dump json-encoded vmstate information for current machine type to file > > > +in @var{file} > > > +ETEXI > > > + > > > HXCOMM This is the last statement. Insert new options before this line! > > > STEXI > > > @end table > > > diff --git a/savevm.c b/savevm.c > > > index da8aa24..a4ce279 100644 > > > --- a/savevm.c > > > +++ b/savevm.c > > > @@ -24,6 +24,7 @@ > > > > > > #include "config-host.h" > > > #include "qemu-common.h" > > > +#include "hw/boards.h" > > > #include "hw/hw.h" > > > #include "hw/qdev.h" > > > #include "net/net.h" > > > @@ -241,6 +242,139 @@ static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers = > > > QTAILQ_HEAD_INITIALIZER(savevm_handlers); > > > static int global_section_id; > > > > > > +static void dump_vmstate_vmsd(FILE *out_file, > > > + const VMStateDescription *vmsd, int indent, > > > + bool is_subsection); > > > + > > > +static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field, > > > + int indent) > > > > checkpatch points out that some tabs managed to get into that indent line. > > Will fix. > > > Generally I think this patch is OK and quite useful; two thoughts: > > 1) I was surprised it dumped every object type, rather than just those > > that are instantiated; I think the latter would be more use in some > > circumstances, since there's a load of weird and wonderful objects > > that exist and are very rarely used. > > The idea is to be able to take a qemu binary and compare with another > binary; if only fields that are instantiated are used, various > invocations will have to be tried to find devices that may have > broken. > > An alternative way of checking only devices which have been added to > the running machine can be done via a monitor command (or a parameter > to the existing cmdline option). But I'm not sure if that'll be more > useful than the current one. Or perhaps a way to dump that info and mask your checker with it if wanted? > > 2) 'fields_exists' is a weird naming to put in the json file - it's > > a function pointer for determining if the field is going to be present; > > maybe renaming as 'conditional' would make sense. > > Yea; I don't know if field_exists is going to be useful anyway. It's > runtime info rather than static, so perhaps can just be dropped. > Right now, anyway, the checker doesn't make use of this field at all. I think it's useful to have field_exists because it lets you know that it's conditional, I just think it's weird naming it like that in the json, since an entry in the json that says 'fields_exists: true' sounds like the field always exists, which is the opposite of what it means. It's just a naming thing here. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On (Wed) 21 May 2014 [11:03:04], Dr. David Alan Gilbert wrote: > * Amit Shah (amit.shah@redhat.com) wrote: > > The idea is to be able to take a qemu binary and compare with another > > binary; if only fields that are instantiated are used, various > > invocations will have to be tried to find devices that may have > > broken. > > > > An alternative way of checking only devices which have been added to > > the running machine can be done via a monitor command (or a parameter > > to the existing cmdline option). But I'm not sure if that'll be more > > useful than the current one. > > Or perhaps a way to dump that info and mask your checker with it if wanted? A 'blacklist' file, which stores names of sections that you're not interested in? > > > 2) 'fields_exists' is a weird naming to put in the json file - it's > > > a function pointer for determining if the field is going to be present; > > > maybe renaming as 'conditional' would make sense. > > > > Yea; I don't know if field_exists is going to be useful anyway. It's > > runtime info rather than static, so perhaps can just be dropped. > > Right now, anyway, the checker doesn't make use of this field at all. > > I think it's useful to have field_exists because it lets you know that it's > conditional, I just think it's weird naming it like that in the json, since > an entry in the json that says 'fields_exists: true' sounds like the field > always exists, which is the opposite of what it means. It's just a naming > thing here. On the name of the field, I doubt anyone will read the json file itself to get confused by it. Also, it stays true to what the field is called in the actual vmstate structs in qemu. On the usability of the field: it's like subsections: they may exist or not, but we should check them nevertheless on src and dest, and any difference should be flagged. Amit
On 05/12/2014 10:12 PM, Amit Shah wrote: > Hi, > > On (Mon) 12 May 2014 [06:51:54], Eric Blake wrote: >> On 05/12/2014 05:16 AM, Amit Shah wrote: >>> This commit adds a new command, '-dump-vmstate', that takes a filename >>> as a parameter. When executed, QEMU will dump the vmstate information >>> for the machine type it's invoked with to the file, and quit. >>> >>> The JSON-format output can then be used to compare the vmstate info for >>> different QEMU versions, specifically to test whether live migration >>> would break due to changes in the vmstate data. >> >> Are we going to document that JSON format anywhere? > > I suppose we should, I thought I should wait for comments here on any > extra fields that people want. > > I suppose documenting would just be coming up with a schema, though? ... > >> Is it worth making >> it part of qapi-schema.json, > > but documenting the entire schema is difficult, as each device will > have its own schema text (due to the differing fields). At least > that's how I understand it; please correct me if I'm wrong. If field names form JSON keys, then yes, documenting each device will be its own schema. But if you write things generic enough, you can have a single schema that covers all devices, by ensuring that device-specific field names are supplied only as values, not keys. That is, 'device': { 'name': 'foo', 'fields': { 'field1': 'int32', 'field2': 'int64' } } requires a schema for each device, while 'device': { 'name': 'foo', 'fields': [ { 'name': 'field1', 'type': 'int32' }, { 'name': 'field2', 'type': 'int64' } ] } is generic. It is more verbose and requires more structure, but by isolating device details into values rather than keys, you get rid of the need for per-device schemas.
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes: > * Amit Shah (amit.shah@redhat.com) wrote: >> This commit adds a new command, '-dump-vmstate', that takes a filename >> as a parameter. When executed, QEMU will dump the vmstate information >> for the machine type it's invoked with to the file, and quit. >> >> The JSON-format output can then be used to compare the vmstate info for >> different QEMU versions, specifically to test whether live migration >> would break due to changes in the vmstate data. >> >> This is based on a version from Andreas Färber posted here: >> https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg03095.html >> >> A Python script that compares the output of such JSON dumps is included >> in the following commit. >> >> Signed-off-by: Amit Shah <amit.shah@redhat.com> >> --- >> include/migration/vmstate.h | 2 + >> qemu-options.hx | 9 +++ >> savevm.c | 134 ++++++++++++++++++++++++++++++++++++++++++++ >> vl.c | 14 +++++ >> 4 files changed, 159 insertions(+) >> >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >> index 7e45048..9829c0e 100644 >> --- a/include/migration/vmstate.h >> +++ b/include/migration/vmstate.h >> @@ -778,4 +778,6 @@ void vmstate_register_ram(struct MemoryRegion *memory, DeviceState *dev); >> void vmstate_unregister_ram(struct MemoryRegion *memory, DeviceState *dev); >> void vmstate_register_ram_global(struct MemoryRegion *memory); >> >> +void dump_vmstate_json_to_file(FILE *out_fp); >> + >> #endif >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 781af14..d376227 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -3146,6 +3146,15 @@ STEXI >> prepend a timestamp to each log message.(default:on) >> ETEXI >> >> +DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate, >> + "-dump-vmstate <file>\n" "", QEMU_ARCH_ALL) >> +STEXI >> +@item -dump-vmstate @var{file} >> +@findex -dump-vmstate >> +Dump json-encoded vmstate information for current machine type to file >> +in @var{file} >> +ETEXI >> + >> HXCOMM This is the last statement. Insert new options before this line! >> STEXI >> @end table >> diff --git a/savevm.c b/savevm.c >> index da8aa24..a4ce279 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -24,6 +24,7 @@ >> >> #include "config-host.h" >> #include "qemu-common.h" >> +#include "hw/boards.h" >> #include "hw/hw.h" >> #include "hw/qdev.h" >> #include "net/net.h" >> @@ -241,6 +242,139 @@ static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers = >> QTAILQ_HEAD_INITIALIZER(savevm_handlers); >> static int global_section_id; >> >> +static void dump_vmstate_vmsd(FILE *out_file, >> + const VMStateDescription *vmsd, int indent, >> + bool is_subsection); >> + >> +static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field, >> + int indent) > > checkpatch points out that some tabs managed to get into that indent line. > > > Generally I think this patch is OK and quite useful; two thoughts: > 1) I was surprised it dumped every object type, rather than just those > that are instantiated; I think the latter would be more use in some > circumstances, since there's a load of weird and wonderful objects > that exist and are very rarely used. Dumping everything lets you reason about what *could* happen at runtime. Which is the point of static checking, isn't it? Optionally dumping only instantiated stuff shouldn't be hard, if it turns out to be useful. [...]
Amit Shah <amit.shah@redhat.com> writes: > On (Wed) 21 May 2014 [11:03:04], Dr. David Alan Gilbert wrote: >> * Amit Shah (amit.shah@redhat.com) wrote: > >> > The idea is to be able to take a qemu binary and compare with another >> > binary; if only fields that are instantiated are used, various >> > invocations will have to be tried to find devices that may have >> > broken. >> > >> > An alternative way of checking only devices which have been added to >> > the running machine can be done via a monitor command (or a parameter >> > to the existing cmdline option). But I'm not sure if that'll be more >> > useful than the current one. >> >> Or perhaps a way to dump that info and mask your checker with it if wanted? > > A 'blacklist' file, which stores names of sections that you're not > interested in? An error message format that lets me grep -v for sections I'm not interested in? Stupidest solution that could possibly work...
On (Wed) 21 May 2014 [05:45:25], Eric Blake wrote: > On 05/12/2014 10:12 PM, Amit Shah wrote: > > Hi, > > > > On (Mon) 12 May 2014 [06:51:54], Eric Blake wrote: > >> On 05/12/2014 05:16 AM, Amit Shah wrote: > >>> This commit adds a new command, '-dump-vmstate', that takes a filename > >>> as a parameter. When executed, QEMU will dump the vmstate information > >>> for the machine type it's invoked with to the file, and quit. > >>> > >>> The JSON-format output can then be used to compare the vmstate info for > >>> different QEMU versions, specifically to test whether live migration > >>> would break due to changes in the vmstate data. > >> > >> Are we going to document that JSON format anywhere? > > > > I suppose we should, I thought I should wait for comments here on any > > extra fields that people want. > > > > I suppose documenting would just be coming up with a schema, though? ... > > > >> Is it worth making > >> it part of qapi-schema.json, > > > > but documenting the entire schema is difficult, as each device will > > have its own schema text (due to the differing fields). At least > > that's how I understand it; please correct me if I'm wrong. > > If field names form JSON keys, then yes, documenting each device will be > its own schema. But if you write things generic enough, you can have a > single schema that covers all devices, by ensuring that device-specific > field names are supplied only as values, not keys. > > That is, > 'device': { 'name': 'foo', > 'fields': { 'field1': 'int32', 'field2': 'int64' } > } > > requires a schema for each device, while > 'device': { 'name': 'foo', > 'fields': [ > { 'name': 'field1', 'type': 'int32' }, > { 'name': 'field2', 'type': 'int64' } > ] > } > > is generic. It is more verbose and requires more structure, but by > isolating device details into values rather than keys, you get rid of > the need for per-device schemas. Sure, that gives a nice schema, but complicates the python script.. at least more complex, from what I know of Python so far. Since no one should have the need to look at the json dumps, it's fair to say the simpler the script, the better for reviewers. Amit
On (Wed) 21 May 2014 [13:47:44], Markus Armbruster wrote: > Amit Shah <amit.shah@redhat.com> writes: > > > On (Wed) 21 May 2014 [11:03:04], Dr. David Alan Gilbert wrote: > >> * Amit Shah (amit.shah@redhat.com) wrote: > > > >> > The idea is to be able to take a qemu binary and compare with another > >> > binary; if only fields that are instantiated are used, various > >> > invocations will have to be tried to find devices that may have > >> > broken. > >> > > >> > An alternative way of checking only devices which have been added to > >> > the running machine can be done via a monitor command (or a parameter > >> > to the existing cmdline option). But I'm not sure if that'll be more > >> > useful than the current one. > >> > >> Or perhaps a way to dump that info and mask your checker with it if wanted? > > > > A 'blacklist' file, which stores names of sections that you're not > > interested in? > > An error message format that lets me grep -v for sections I'm not > interested in? Stupidest solution that could possibly work... The discusison had overfown onto IRC, and I think what David was looking for was to weed out certain sections altogether, like 'fdc', if the machines he's interested in do not have floppy drives at all. The error message format is quite consistent in that regard, I feel, but opinions on improving it are welcome. Currently, errors/warnings are reported like this: Section "usb-kbd" Description "usb-kbd" Field "kbd.keycodes" size mismatch: 4 , 2 Section "PIIX3-xen" Description "PIIX3": minimum version error: 1 < 2 Section "PIIX3-xen" Description "PIIX3": Entry "Subsections" missing ... and so on. Amit
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 7e45048..9829c0e 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -778,4 +778,6 @@ void vmstate_register_ram(struct MemoryRegion *memory, DeviceState *dev); void vmstate_unregister_ram(struct MemoryRegion *memory, DeviceState *dev); void vmstate_register_ram_global(struct MemoryRegion *memory); +void dump_vmstate_json_to_file(FILE *out_fp); + #endif diff --git a/qemu-options.hx b/qemu-options.hx index 781af14..d376227 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3146,6 +3146,15 @@ STEXI prepend a timestamp to each log message.(default:on) ETEXI +DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate, + "-dump-vmstate <file>\n" "", QEMU_ARCH_ALL) +STEXI +@item -dump-vmstate @var{file} +@findex -dump-vmstate +Dump json-encoded vmstate information for current machine type to file +in @var{file} +ETEXI + HXCOMM This is the last statement. Insert new options before this line! STEXI @end table diff --git a/savevm.c b/savevm.c index da8aa24..a4ce279 100644 --- a/savevm.c +++ b/savevm.c @@ -24,6 +24,7 @@ #include "config-host.h" #include "qemu-common.h" +#include "hw/boards.h" #include "hw/hw.h" #include "hw/qdev.h" #include "net/net.h" @@ -241,6 +242,139 @@ static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers = QTAILQ_HEAD_INITIALIZER(savevm_handlers); static int global_section_id; +static void dump_vmstate_vmsd(FILE *out_file, + const VMStateDescription *vmsd, int indent, + bool is_subsection); + +static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field, + int indent) +{ + fprintf(out_file, "%*s{\n", indent, ""); + indent += 2; + fprintf(out_file, "%*s\"field\": \"%s\",\n", indent, "", field->name); + fprintf(out_file, "%*s\"version_id\": %d,\n", indent, "", + field->version_id); + fprintf(out_file, "%*s\"field_exists\": %s,\n", indent, "", + field->field_exists ? "true" : "false"); + fprintf(out_file, "%*s\"size\": %zu", indent, "", field->size); + if (field->vmsd != NULL) { + fprintf(out_file, ",\n"); + dump_vmstate_vmsd(out_file, field->vmsd, indent, false); + } + fprintf(out_file, "\n%*s}", indent - 2, ""); +} + +static void dump_vmstate_vmss(FILE *out_file, + const VMStateSubsection *subsection, + int indent) +{ + if (subsection->vmsd != NULL) { + dump_vmstate_vmsd(out_file, subsection->vmsd, indent, true); + } +} + +static void dump_vmstate_vmsd(FILE *out_file, + const VMStateDescription *vmsd, int indent, + bool is_subsection) +{ + if (is_subsection) { + fprintf(out_file, "%*s{\n", indent, ""); + } else { + fprintf(out_file, "%*s\"%s\": {\n", indent, "", "Description"); + } + indent += 2; + fprintf(out_file, "%*s\"name\": \"%s\",\n", indent, "", vmsd->name); + fprintf(out_file, "%*s\"version_id\": %d,\n", indent, "", + vmsd->version_id); + fprintf(out_file, "%*s\"minimum_version_id\": %d", indent, "", + vmsd->minimum_version_id); + if (vmsd->fields != NULL) { + const VMStateField *field = vmsd->fields; + bool first; + + fprintf(out_file, ",\n%*s\"Fields\": [\n", indent, ""); + first = true; + while (field->name != NULL) { + if (!first) { + fprintf(out_file, ",\n"); + } + dump_vmstate_vmsf(out_file, field, indent + 2); + field++; + first = false; + } + fprintf(out_file, "\n%*s]", indent, ""); + } + if (vmsd->subsections != NULL) { + const VMStateSubsection *subsection = vmsd->subsections; + bool first; + + fprintf(out_file, ",\n%*s\"Subsections\": [\n", indent, ""); + first = true; + while (subsection->vmsd != NULL) { + if (!first) { + fprintf(out_file, ",\n"); + } + dump_vmstate_vmss(out_file, subsection, indent + 2); + subsection++; + first = false; + } + fprintf(out_file, "\n%*s]", indent, ""); + } + fprintf(out_file, "\n%*s}", indent - 2, ""); +} + +static void dump_machine_type(FILE *out_file) +{ + MachineClass *mc; + + mc = MACHINE_GET_CLASS(current_machine); + + fprintf(out_file, " \"vmschkmachine\": {\n"); + fprintf(out_file, " \"Name\": \"%s\"\n", mc->name); + fprintf(out_file, " },\n"); +} + +void dump_vmstate_json_to_file(FILE *out_file) +{ + GSList *list, *elt; + bool first; + + fprintf(out_file, "{\n"); + dump_machine_type(out_file); + + first = true; + list = object_class_get_list(TYPE_DEVICE, true); + for (elt = list; elt; elt = elt->next) { + DeviceClass *dc = OBJECT_CLASS_CHECK(DeviceClass, elt->data, + TYPE_DEVICE); + const char *name; + int indent = 2; + + if (!dc->vmsd) { + continue; + } + + if (!first) { + fprintf(out_file, ",\n"); + } + name = object_class_get_name(OBJECT_CLASS(dc)); + fprintf(out_file, "%*s\"%s\": {\n", indent, "", name); + indent += 2; + fprintf(out_file, "%*s\"Name\": \"%s\",\n", indent, "", name); + fprintf(out_file, "%*s\"version_id\": %d,\n", indent, "", + dc->vmsd->version_id); + fprintf(out_file, "%*s\"minimum_version_id\": %d,\n", indent, "", + dc->vmsd->minimum_version_id); + + dump_vmstate_vmsd(out_file, dc->vmsd, indent, false); + + fprintf(out_file, "\n%*s}", indent - 2, ""); + first = false; + } + fprintf(out_file, "\n}\n"); + fclose(out_file); +} + static int calculate_new_instance_id(const char *idstr) { SaveStateEntry *se; diff --git a/vl.c b/vl.c index 73e0661..165ec3e 100644 --- a/vl.c +++ b/vl.c @@ -2988,6 +2988,7 @@ int main(int argc, char **argv, char **envp) const char *trace_file = NULL; const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE * 1024 * 1024; + FILE *vmstate_dump_file = NULL; atexit(qemu_run_exit_notifiers); error_set_progname(argv[0]); @@ -3956,6 +3957,13 @@ int main(int argc, char **argv, char **envp) } configure_msg(opts); break; + case QEMU_OPTION_dump_vmstate: + vmstate_dump_file = fopen(optarg, "w"); + if (vmstate_dump_file == NULL) { + fprintf(stderr, "open %s: %s\n", optarg, strerror(errno)); + exit(1); + } + break; default: os_parse_cmd_args(popt->index, optarg); } @@ -4533,6 +4541,12 @@ int main(int argc, char **argv, char **envp) } } + if (vmstate_dump_file) { + /* dump and exit */ + dump_vmstate_json_to_file(vmstate_dump_file); + return 0; + } + if (incoming) { Error *local_err = NULL; qemu_start_incoming_migration(incoming, &local_err);
This commit adds a new command, '-dump-vmstate', that takes a filename as a parameter. When executed, QEMU will dump the vmstate information for the machine type it's invoked with to the file, and quit. The JSON-format output can then be used to compare the vmstate info for different QEMU versions, specifically to test whether live migration would break due to changes in the vmstate data. This is based on a version from Andreas Färber posted here: https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg03095.html A Python script that compares the output of such JSON dumps is included in the following commit. Signed-off-by: Amit Shah <amit.shah@redhat.com> --- include/migration/vmstate.h | 2 + qemu-options.hx | 9 +++ savevm.c | 134 ++++++++++++++++++++++++++++++++++++++++++++ vl.c | 14 +++++ 4 files changed, 159 insertions(+)