From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:43006)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1TjYzq-0005Ft-HY
	for qemu-devel@nongnu.org; Fri, 14 Dec 2012 12:24:28 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1TjYzp-000547-9C
	for qemu-devel@nongnu.org; Fri, 14 Dec 2012 12:24:26 -0500
Received: from mx1.redhat.com ([209.132.183.28]:13717)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <lcapitulino@redhat.com>) id 1TjYzp-00053x-1v
	for qemu-devel@nongnu.org; Fri, 14 Dec 2012 12:24:25 -0500
Received: from int-mx09.intmail.prod.int.phx2.redhat.com
	(int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBEHOO6M000793
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <qemu-devel@nongnu.org>; Fri, 14 Dec 2012 12:24:24 -0500
Date: Fri, 14 Dec 2012 15:24:24 -0200
From: Luiz Capitulino <lcapitulino@redhat.com>
Message-ID: <20121214152424.1e6b1ab8@doriath.home>
In-Reply-To: <cover.1355404685.git.phrdina@redhat.com>
References: <cover.1355404685.git.phrdina@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH v2 00/17] qapi: Convert savevm, loadvm,
 delvm and info snapshots
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: Pavel Hrdina <phrdina@redhat.com>
Cc: qemu-devel@nongnu.org

On Thu, 13 Dec 2012 16:40:34 +0100
Pavel Hrdina <phrdina@redhat.com> wrote:

> This patch series convert these commands into qapi and introduce QMP comm=
ands
> vm-snapshot-save, vm-snapshot-load, vm-snapshot-delete and query-vm-snaps=
hots.
> It also rewrite error report for functions used by these commands.

I've reviewed most of this series and identified several problems, but I
think that the major problem here is that this series is doing too much at
once.

A properly done and split series would easily reach twice as many patches
this series currently has.

So, my main advice here is to start simple. You could take one of savevm,
delvm or loadvm and convert it to propagate errors internally.

Also note that this series doesn't build:

  CC    x86_64-softmmu/savevm.o
/home/lcapitulino/work/src/qmp-unstable/savevm.c: In function =E2=80=98qmp_=
vm_snapshot_load=E2=80=99:
/home/lcapitulino/work/src/qmp-unstable/savevm.c:2274:9: error: variable =
=E2=80=98ret=E2=80=99 set but not used [-Werror=3Dunused-but-set-variable]
cc1: all warnings being treated as errors
make[1]: *** [savevm.o] Error 1
make: *** [subdir-x86_64-softmmu] Error 2