Re: [Qemu-devel] [PATCH v2 00/17] qapi: Convert savevm, loadvm, delvm and info snapshots

[Luiz Capitulino]

On Thu, 13 Dec 2012 16:40:34 +0100
Pavel Hrdina <address@hidden> wrote:

> This patch series convert these commands into qapi and introduce QMP commands
> vm-snapshot-save, vm-snapshot-load, vm-snapshot-delete and query-vm-snapshots.
> 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 
‘qmp_vm_snapshot_load’:
/home/lcapitulino/work/src/qmp-unstable/savevm.c:2274:9: error: variable ‘ret’ 
set but not used [-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors
make[1]: *** [savevm.o] Error 1
make: *** [subdir-x86_64-softmmu] Error 2