From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42666) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn3Mj-0001km-TN for qemu-devel@nongnu.org; Wed, 21 May 2014 06:03:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wn3Md-0004bu-06 for qemu-devel@nongnu.org; Wed, 21 May 2014 06:03:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58499) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wn3Mc-0004bd-OK for qemu-devel@nongnu.org; Wed, 21 May 2014 06:03:10 -0400 Date: Wed, 21 May 2014 11:03:04 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20140521100304.GD2589@work-vm> References: <20140521094407.GC2589@work-vm> <20140521095502.GA32726@grmbl.mre> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <20140521095502.GA32726@grmbl.mre> Subject: Re: [Qemu-devel] [PATCH 01/18] migration: dump vmstate info as a json file for static analysis List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: Juan Quintela , Markus Armbruster , Alexander Graf , qemu list , Paolo Bonzini , Andreas F?rber * 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. > > >=20 > > > The JSON-format output can then be used to compare the vmstate info f= or > > > different QEMU versions, specifically to test whether live migration > > > would break due to changes in the vmstate data. > > >=20 > > > This is based on a version from Andreas F=E4rber posted here: > > > https://lists.gnu.org/archive/html/qemu-devel/2013-10/msg03095.html > > >=20 > > > A Python script that compares the output of such JSON dumps is includ= ed > > > in the following commit. > > >=20 > > > Signed-off-by: Amit Shah > > > --- > > > include/migration/vmstate.h | 2 + > > > qemu-options.hx | 9 +++ > > > savevm.c | 134 ++++++++++++++++++++++++++++++++++= ++++++++++ > > > vl.c | 14 +++++ > > > 4 files changed, 159 insertions(+) > > >=20 > > > 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 *me= mory, DeviceState *dev); > > > void vmstate_unregister_ram(struct MemoryRegion *memory, DeviceState= *dev); > > > void vmstate_register_ram_global(struct MemoryRegion *memory); > > > =20 > > > +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 > > > =20 > > > +DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate, > > > + "-dump-vmstate \n" "", QEMU_ARCH_ALL) > > > +STEXI > > > +@item -dump-vmstate @var{file} > > > +@findex -dump-vmstate > > > +Dump json-encoded vmstate information for current machine type to fi= le > > > +in @var{file} > > > +ETEXI > > > + > > > HXCOMM This is the last statement. Insert new options before this li= ne! > > > 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 @@ > > > =20 > > > #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, SaveStateEn= try) savevm_handlers =3D > > > QTAILQ_HEAD_INITIALIZER(savevm_handlers); > > > static int global_section_id; > > > =20 > > > +static void dump_vmstate_vmsd(FILE *out_file, > > > + const VMStateDescription *vmsd, int in= dent, > > > + bool is_subsection); > > > + > > > +static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *fi= eld, > > > + int indent) > >=20 > > checkpatch points out that some tabs managed to get into that indent li= ne. >=20 > Will fix. >=20 > > Generally I think this patch is OK and quite useful; two thoughts: > > 1) I was surprised it dumped every object type, rather than just tho= se > > that are instantiated; I think the latter would be more use in so= me > > circumstances, since there's a load of weird and wonderful objects > > that exist and are very rarely used. >=20 > 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. >=20 > 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 pr= esent; > > maybe renaming as 'conditional' would make sense. >=20 > 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