On Wed, 21 Apr 2010 15:28:16 +0200 Kevin Wolf <kw...@redhat.com> wrote:
> 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. Yeah, it's an RFC. I decided to send it as is because I needed feedback as this series is a snowball. > 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. Maybe 'info status' should have a specific status for this too. (Assuming we don't want to radically call exit(1)). > > 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. That's exactly what I want to do. > 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). Defining the right behavior and deciding what to expose have been proven very difficult tasks in the QMP world.
