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

[Laine Stump]

On 09/30/2011 02:52 PM, Eric Blake wrote:
> 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);


ACK.