From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42939)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WxD2n-0002ob-7q
	for qemu-devel@nongnu.org; Wed, 18 Jun 2014 06:24:48 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WxD2b-0003rr-4t
	for qemu-devel@nongnu.org; Wed, 18 Jun 2014 06:24:41 -0400
Received: from mx1.redhat.com ([209.132.183.28]:48793)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1WxD2a-0003rn-Qn
	for qemu-devel@nongnu.org; Wed, 18 Jun 2014 06:24:29 -0400
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <4ab8ba6bf5de4943288c1ddc565179c807f263e3.1403079139.git.amit.shah@redhat.com>
	(Amit Shah's message of "Wed, 18 Jun 2014 13:43:35 +0530")
References: <cover.1403079139.git.amit.shah@redhat.com>
	<cover.1403079139.git.amit.shah@redhat.com>
	<4ab8ba6bf5de4943288c1ddc565179c807f263e3.1403079139.git.amit.shah@redhat.com>
Date: Wed, 18 Jun 2014 12:24:25 +0200
Message-ID: <87mwdaldly.fsf@troll.troll>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v4 01/18] migration: dump vmstate info as a
	json file for static analysis
Reply-To: quintela@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Amit Shah <amit.shah@redhat.com>
Cc: qemu list <qemu-devel@nongnu.org>, Markus Armbruster <armbru@redhat.com>, Alexander Graf <agraf@suse.de>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Andreas =?utf-8?Q?F=C3=A4rber?= <afaerber@suse.de>

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.
>
> 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>



> +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, "");

Remove last "\n" (*)


> +        first = true;

first can go now

> +        while (field->name != NULL) {
> +            if (field->flags & VMS_MUST_EXIST) {
> +                /* Ignore VMSTATE_VALIDATE bits; these don't get migrated */
> +                field++;
> +                continue;
> +            }
> +            if (!first) {
> +                fprintf(out_file, ",\n");

You can print always \n now, right?

Same for the other places?  Or I am missing something.

I will even go that itwould be better to just left the \n on the (*),
and just add this \n at the end of writing a subsection.


> +    fprintf(out_file, "\n%*s}", indent - 2, "");

And you remove it from here.

<full disclosure>
Yes, I have always hated pretty-printers
</full-disclosure>

Rest of this, I fully agree.

Later, Juan.