From f0aa7a8b2d518c54430e4382309281b93e51981a Mon Sep 17 00:00:00 2001 From: Miguel Di Ciurcio Filho Date: Mon, 19 Jul 2010 15:25:01 -0300 Subject: [PATCH] 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 Signed-off-by: Kevin Wolf --- monitor.c | 3 ++- savevm.c | 71 ++++++++++++++++++++++++++----------------------------- 2 files changed, 36 insertions(+), 38 deletions(-) diff --git a/monitor.c b/monitor.c index 5366c36525..c313d5ac14 100644 --- a/monitor.c +++ b/monitor.c @@ -2274,8 +2274,9 @@ static void do_loadvm(Monitor *mon, const QDict *qdict) vm_stop(0); - if (load_vmstate(name) >= 0 && saved_vm_running) + if (load_vmstate(name) == 0 && saved_vm_running) { vm_start(); + } } int monitor_get_fd(Monitor *mon, const char *fdname) diff --git a/savevm.c b/savevm.c index 4c0e5d3b10..53d47a6234 100644 --- a/savevm.c +++ b/savevm.c @@ -1894,12 +1894,27 @@ void do_savevm(Monitor *mon, const QDict *qdict) 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 @@ int load_vmstate(const char *name) 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; } -- GitLab