[Qemu-devel] Re: [RFC 00/22]: QMP: Convert savevm/loadvm/delvm

[Juan Quintela <quintela@redhat.com>]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
>  Hi there,
>
>  Libvirt has a new snapshot API which uses savevm, loadvm and delvm, so we
> need to convert them to QMP.
>
>  I thought this wouldn't be difficult, but while doing and testing this work
> I hit a number of problems and had to made not so easy decisions.
>
>  Most of the time, the problem is that the handler implementation is not
> as consistent as it should be. Usually, this doesn't affect users very much
> but the headache begins when you have to make this available under QMP.
>
>  Here goes a list of macro issues/decisions. Details (and other small problems)
> in the patches.
>
>    1. Multiple failures: do_delvm() and do_savevm() report errors in a
>       QTAILQ_FOREACH() loop and they don't return when an error happens.
>
>       Although QMP will end up reporting only the first error, this is
>       considered a bug because handlers are not expected to continue
>       executing when an error happens.

I haven't looked at all the error cases, but I think that part of the
problem are devices like cdrom, that are by definition _non_
snapshotable  (what an ugly word).

>       There are two solutions:
>
>             1) Return right away in the first error
>             2) Identify fatal errors and only report those
>
>       I don't know the implications of doing 1) and don't know how to
>       to do 2) (although do_loadvm() works this way).
>
>       Can someone from the block layer help here?

Use case:  We want to save snapshot foo.  It can be:
- snapshot foo already exist in some device. What to do:
   - just fail and ask the user for other name
   - overwrite the name on the devices that already have it
   - any other good idea?

if we just fail the save, then we tell the user: devices bar1 and bar2
already have that snapshot name.  What to do?
- force the user to use a different name (not such a bad idea)
- ask the user to stop the vm and remove the snaphost from the ofended
  devices with qemu-img.  High avalavility people don't like to be
  forced to stop machines.
- delvm just ignore the devices that don't have the snapshot name
  (basically what we have now)
- we add a delvm_force or del_device_vm <device> <name> operations.\
- any other great idea?

Except where commented, I like the rest of the patches.  The loadvm is a
change that I don't agree with.  The StateVM name is something that I
can live with.

Thanks, Juan.