loadvm: improve tests before bdrv_snapshot_goto()
This patch improves the resilience of the load_vmstate() function, doing
further and better ordered tests.
In load_vmstate(), if there is any error on bdrv_snapshot_goto(), except if the
error is on VM state device, load_vmstate() will return zero and the VM will be
started with major corruption chances.
The current process:
- test if there is any writable device without snapshot support
- if exists return -error
- get the device that saves the VM state, possible return -error but unlikely
because it was tested earlier
- flush I/O
- run bdrv_snapshot_goto() on devices
- if fails, give an warning and goes to the next (not good!)
- if fails on the VM state device, return zero (not good!)
- check if the requested snapshot exists on the device that saves the VM state
and the state is not zero
- if fails return -error
- open the file with the VM state
- if fails return -error
- load the VM state
- if fails return -error
- return zero
New behavior:
- get the device that saves the VM state
- if fails return -error
- check if the requested snapshot exists on the device that saves the VM state
and the state is not zero
- if fails return -error
- test if there is any writable device without snapshot support
- if exists return -error
- test if the devices with snapshot support have the requested snapshot
- if anyone fails, return -error
- flush I/O
- run snapshot_goto() on devices
- if anyone fails, return -error
- open the file with the VM state
- if fails return -error
- load the VM state
- if fails return -error
- return zero
do_loadvm must not call vm_start if any error has occurred in load_vmstate.
Signed-off-by: Miguel Di Ciurcio Filho <miguel.filho@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/savevm.c b/savevm.c
index 4c0e5d3..53d47a6 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1894,12 +1894,27 @@
int load_vmstate(const char *name)
{
- BlockDriverState *bs, *bs1;
+ BlockDriverState *bs, *bs_vm_state;
QEMUSnapshotInfo sn;
QEMUFile *f;
int ret;
- /* Verify if there is a device that doesn't support snapshots and is writable */
+ bs_vm_state = bdrv_snapshots();
+ if (!bs_vm_state) {
+ error_report("No block device supports snapshots");
+ return -ENOTSUP;
+ }
+
+ /* Don't even try to load empty VM states */
+ ret = bdrv_snapshot_find(bs_vm_state, &sn, name);
+ if (ret < 0) {
+ return ret;
+ } else if (sn.vm_state_size == 0) {
+ return -EINVAL;
+ }
+
+ /* Verify if there is any device that doesn't support snapshots and is
+ writable and check if the requested snapshot is available too. */
bs = NULL;
while ((bs = bdrv_next(bs))) {
@@ -1912,63 +1927,45 @@
bdrv_get_device_name(bs));
return -ENOTSUP;
}
- }
- bs = bdrv_snapshots();
- if (!bs) {
- error_report("No block device supports snapshots");
- return -EINVAL;
+ ret = bdrv_snapshot_find(bs, &sn, name);
+ if (ret < 0) {
+ error_report("Device '%s' does not have the requested snapshot '%s'",
+ bdrv_get_device_name(bs), name);
+ return ret;
+ }
}
/* Flush all IO requests so they don't interfere with the new state. */
qemu_aio_flush();
- bs1 = NULL;
- while ((bs1 = bdrv_next(bs1))) {
- if (bdrv_can_snapshot(bs1)) {
- ret = bdrv_snapshot_goto(bs1, name);
+ bs = NULL;
+ while ((bs = bdrv_next(bs))) {
+ if (bdrv_can_snapshot(bs)) {
+ ret = bdrv_snapshot_goto(bs, name);
if (ret < 0) {
- switch(ret) {
- case -ENOTSUP:
- error_report("%sSnapshots not supported on device '%s'",
- bs != bs1 ? "Warning: " : "",
- bdrv_get_device_name(bs1));
- break;
- case -ENOENT:
- error_report("%sCould not find snapshot '%s' on device '%s'",
- bs != bs1 ? "Warning: " : "",
- name, bdrv_get_device_name(bs1));
- break;
- default:
- error_report("%sError %d while activating snapshot on '%s'",
- bs != bs1 ? "Warning: " : "",
- ret, bdrv_get_device_name(bs1));
- break;
- }
- /* fatal on snapshot block device */
- if (bs == bs1)
- return 0;
+ error_report("Error %d while activating snapshot '%s' on '%s'",
+ ret, name, bdrv_get_device_name(bs));
+ return ret;
}
}
}
- /* Don't even try to load empty VM states */
- ret = bdrv_snapshot_find(bs, &sn, name);
- if ((ret >= 0) && (sn.vm_state_size == 0))
- return -EINVAL;
-
/* restore the VM state */
- f = qemu_fopen_bdrv(bs, 0);
+ f = qemu_fopen_bdrv(bs_vm_state, 0);
if (!f) {
error_report("Could not open VM state file");
return -EINVAL;
}
+
ret = qemu_loadvm_state(f);
+
qemu_fclose(f);
if (ret < 0) {
error_report("Error %d while loading VM state", ret);
return ret;
}
+
return 0;
}
