* Amit Shah (amit.s...@redhat.com) wrote: > On (Wed) 21 May 2014 [10:44:07], Dr. David Alan Gilbert wrote: > > * Amit Shah (amit.s...@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.s...@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 / dgilb...@redhat.com / Manchester, UK
