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
