Re: [Qemu-devel] [PATCH 01/18] migration: dump vmstate info as a json file for static analysis

[Markus Armbruster]

"Dr. David Alan Gilbert" <address@hidden> writes:

> * Amit Shah (address@hidden) 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 <address@hidden>
>> ---
>>  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
>> address@hidden -dump-vmstate @var{file}
>> address@hidden -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.

[...]