[Qemu-devel] Re: [PATCH 04/22] savevm: do_loadvm(): Always resume the VM

[Kevin Wolf]

Am 20.04.2010 23:09, schrieb Luiz Capitulino:
> do_loadvm(), which implements the 'loadvm' Monitor command, pauses
> the emulation to load the saved VM, however it will only resume
> it if the loading succeeds.
> 
> In other words, if the user issues 'loadvm' and it fails, the
> end result will be an error message and a paused VM.
> 
> This seems an undesirable side effect to me because, most of the
> time, if a Monitor command fails the best thing we can do is to
> leave the VM as it were before the command was executed.

I completely agree with Juan here, this is broken.

If you could leave the VM as it was before, just like you describe
above, everything would be okay. But in fact you can't tell where the
error occurred. You may still have the old state; or you may have loaded
the snapshot on one disk, but not on the other one; or you may have
loaded snapshots for all disks, but only half of the devices.

We must not run a machine in such an unknown state. I'd even go further
and say that after a failed loadvm we must even prevent that a user uses
the cont command to resume manually.

> FIXME: This will try to run a potentially corrupted image, the
>        solution is to split load_vmstate() in two and only keep
>        the VM paused if qemu_loadvm_state() fails.

Your suggestion of having a prepare function that doesn't change any
state looks fine to me. It could just check if the snapshot is there and
contains VM state. This should cover all of the trivial cases where
recovery is really as easy as resuming the old state.

Another problem that I see is that it's really hard to define what an
error is. Current code print "Warning: ..." for all non-primary images
and continues with loading the snapshot. I'm really unsure what the
right behaviour would be if a snapshot doesn't exist on a secondary
image (note that this is not the CD-ROM case, these drives are already
excluded by bdrv_has_snapshot as they are read-only).

Kevin