Re: [libvirt] [PATCH 2/2] snapshot: enforce REVERT_FORCE on qemu

Implements the documentation for snapshot revert vs. force. Part of the patch tightens existing behavior (previously, reverting to an old snapshot without<domain> was blindly attempted, now it requires force), while part of it relaxes behavior (previously, it was not possible to revert an active domain to an ABI-incompatible active snapshot, now force allows this transition). * src/qemu/qemu_driver.c (qemuDomainRevertToSnapshot): Check for risky situations, and allow force to get past them. --- src/qemu/qemu_driver.c | 47 +++++++++++++++++++++++++++++++++++++---------- 1 files changed, 37 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5110102..efd60a7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9753,7 +9753,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, * 7. paused -> inactive: EVENT_STOPPED * 8. paused -> running: EVENT_RESUMED * 9. paused -> paused: none - * Also, several transitions occur even if we fail partway through. + * Also, several transitions occur even if we fail partway through, + * and use of FORCE can cause multiple transitions. */ qemuDriverLock(driver); @@ -9789,6 +9790,24 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, "yet")); goto cleanup; } + if (!(flags& VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { + if (!snap->def->dom) { + qemuReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, + _("snapshot lacks domain '%s' rollback details"), + snap->def->name); + goto cleanup; + } + if (virDomainObjIsActive(vm)&& + !(snap->def->state == VIR_DOMAIN_RUNNING + || snap->def->state == VIR_DOMAIN_PAUSED)&& + (flags& (VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING | + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) { + qemuReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, + _("must respawn qemu to start inactive snapshot")); + goto cleanup; + } + } + if (vm->current_snapshot) { vm->current_snapshot->def->current = false; @@ -9818,11 +9837,6 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, VIR_FREE(xml); if (!config) goto cleanup; - } else { - /* XXX Fail if VIR_DOMAIN_REVERT_FORCE is not set, rather than - * blindly hoping for the best. */ - VIR_WARN("snapshot is lacking rollback information for domain '%s'", - snap->def->name); } if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)< 0) @@ -9843,10 +9857,22 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, /* Transitions 5, 6, 8, 9 */ /* Check for ABI compatibility. */ if (config&& !virDomainDefCheckABIStability(vm->def, config)) { - /* XXX Add VIR_DOMAIN_REVERT_FORCE to permit killing - * and restarting a new qemu, since loadvm monitor - * command won't work. */ - goto endjob; + if (!(flags& VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) { + /* Alter existing error to give correct category. */ + virErrorPtr err = virGetLastError(); + err->code = VIR_ERR_SNAPSHOT_REVERT_RISKY; + goto endjob; + } + qemuProcessStop(driver, vm, 0, + VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT); + virDomainAuditStop(vm, "from-snapshot"); + detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT; + event = virDomainEventNewFromObj(vm, + VIR_DOMAIN_EVENT_STOPPED, + detail); + if (event) + qemuDomainEventQueue(driver, event); + goto load; } priv = vm->privateData; @@ -9882,6 +9908,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot, virDomainObjAssignDef(vm, config, false); } else { /* Transitions 2, 3 */ + load: was_stopped = true; if (config) virDomainObjAssignDef(vm, config, false);