[libvirt] [PATCH 7/7] qemu: snapshot: Correctly revert snapshots in PMSUSPENDED state

[Eric Blake]

On 07/18/2014 10:11 AM, Peter Krempa wrote:
> PMSUSPENDED state implies the qemu process running, so we should treat
> it that way. This increases the possible state space of transitions that
> may happen during the snapshot revert so this patch documents them as
> well.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1079162
> ---
>  src/qemu/qemu_driver.c | 67 ++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 48 insertions(+), 19 deletions(-)

I think this needs some rework - current qemu treats it as reverting to
a pm wakeup event, and the guest will start running again (unless the
user requested to revert as paused).

> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 1f98f4a..f93e0fd 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14052,22 +14052,29 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
>      virDomainDefPtr config = NULL;
>      virQEMUDriverConfigPtr cfg = NULL;
>      virCapsPtr caps = NULL;
> -    int oldState = vm->state.state;
> +    int oldState;
> 
>      virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
>                    VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
>                    VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);
> 
>      /* We have the following transitions, which create the following events:
> -     * 1. inactive -> inactive: none
> -     * 2. inactive -> running:  EVENT_STARTED
> -     * 3. inactive -> paused:   EVENT_STARTED, EVENT_PAUSED
> -     * 4. running  -> inactive: EVENT_STOPPED
> -     * 5. running  -> running:  none
> -     * 6. running  -> paused:   EVENT_PAUSED
> -     * 7. paused   -> inactive: EVENT_STOPPED
> -     * 8. paused   -> running:  EVENT_RESUMED
> -     * 9. paused   -> paused:   none
> +     * 1.  inactive    -> inactive:    none
> +     * 2.  inactive    -> running:     EVENT_STARTED
> +     * 3.  inactive    -> paused:      EVENT_STARTED, EVENT_PAUSED
> +     * 4.  running     -> inactive:    EVENT_STOPPED
> +     * 5.  running     -> running:     none
> +     * 6.  running     -> paused:      EVENT_PAUSED
> +     * 7.  paused      -> inactive:    EVENT_STOPPED
> +     * 8.  paused      -> running:     EVENT_RESUMED
> +     * 9.  paused      -> paused:      none
> +     * 10. pmsuspended -> pmsuspended: none

this state is not possible with current qemu

> +     * 11. pmsuspended -> inactive:    EVENT_STOPPED
> +     * 12. pmsuspended -> running:     EVENT_RESUMED
> +     * 13. pmsuspended -> paused:      EVENT_PAUSED
> +     * 14. inactive    -> pmsuspended: EVENT_STARTED, EVENT_PMSUSPENDED
> +     * 15. paused      -> pmsuspended: EVENT_PMSUSPENDED
> +     * 16. running     -> pmsuspended: EVENT_PMSUSPENDED

and these three states are not possible

>       * Also, several transitions occur even if we fail partway through,
>       * and use of FORCE can cause multiple transitions.
>       */
> @@ -14075,6 +14082,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
>      if (!(vm = qemuDomObjFromSnapshot(snapshot)))
>          return -1;
> 
> +    oldState = vm->state.state;
> +
>      cfg = virQEMUDriverGetConfig(driver);
> 
>      if (virDomainRevertToSnapshotEnsureACL(snapshot->domain->conn, vm->def) < 0)
> @@ -14127,6 +14136,14 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
>          }
>      }
> 
> +    if (snap->def->state == VIR_DOMAIN_PMSUSPENDED &&
> +        (flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING ||
> +         flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("snapshot of a VM in PMSUSPENDED state cannot be "
> +                         "reverted to a running or paused state"));

actually, I think that we need to tackle it differently - we can't
CREATE a snapshot in the pmsuspended state (just as we can't migrate in
that state - the mere act of migration is a wakeup event).  So, if you
eliminate all snapshots in the pmsuspended state as invalid, the set of
state transitions is smaller.


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140718/b9f2f288/attachment-0001.sig>