[libvirt] [PATCHv4 3/4] snapshot: Add flag to allow hypervisor restart when reverting snapshots

[Peter Krempa]

Some hypervisors require a respawn of the hypervisor to allow reverting
to some snapshot states. This patch adds flag to remove the default
safe approach to not allow this. When this flag is specified the
hypervisor driver should re-emit events to allow management apps to
reconnect.

This flag is meant as a lesser way to enforce the restart of the
hypervisor, that is a fairly common possibility compared to other
meanings that the existing force flag has.
---
Force now selects this flag and this flag can be used with internal snapshots too.
---
 include/libvirt/libvirt.h.in | 2 ++
 src/libvirt.c                | 5 +++++
 src/qemu/qemu_driver.c       | 8 ++++++--
 tools/virsh-snapshot.c       | 4 ++++
 tools/virsh.pod              | 6 +++++-
 5 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index d3ee588..ac24ed8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3876,6 +3876,8 @@ typedef enum {
     VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED  = 1 << 1, /* Pause after revert */
     VIR_DOMAIN_SNAPSHOT_REVERT_FORCE   = 1 << 2, /* Allow risky reverts */
     VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED = 1 << 3, /* Revert into stopped state */
+    VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN = 1 << 4, /* Allow restarting of the
+                                                    hypervisor */
 } virDomainSnapshotRevertFlags;

 /* Revert the domain to a point-in-time snapshot.  The
diff --git a/src/libvirt.c b/src/libvirt.c
index 1e04d27..8b7de9f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18650,6 +18650,11 @@ error:
  * it is not possible to revert a transient domain into an inactive state,
  * so transient domains require the use of either the running or paused flag.
  *
+ * Some snapshot operations may require a restart of the hypervisor to complete
+ * successfuly. This is normally not allowed. To override this behavior add
+ * VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN to @flags. The hypervisor driver should
+ * re-emit the appropriate events to allow reconnect of management applications.
+ *
  * Reverting to any snapshot discards all configuration changes made since
  * the last snapshot.  Additionally, reverting to a snapshot from a running
  * domain is a form of data loss, since it discards whatever is in the
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 34aef0c..2178798 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12129,7 +12129,11 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,

     virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
                   VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
-                  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);
+                  VIR_DOMAIN_SNAPSHOT_REVERT_FORCE |
+                  VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN, -1);
+
+    if (flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)
+        flags |= VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN;

     /* We have the following transitions, which create the following events:
      * 1. inactive -> inactive: none
@@ -12249,7 +12253,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             if (config && !virDomainDefCheckABIStability(vm->def, config)) {
                 virErrorPtr err = virGetLastError();

-                if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
+                if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN)) {
                     /* Re-spawn error using correct category. */
                     if (err->code == VIR_ERR_CONFIG_UNSUPPORTED)
                         virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 57d2baf..8897b7e 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1519,6 +1519,8 @@ static const vshCmdOptDef opts_snapshot_revert[] = {
     {"paused", VSH_OT_BOOL, 0, N_("after reverting, change state to paused")},
     {"stopped", VSH_OT_BOOL, 0, N_("after reverting, change state to stopped")},
     {"force", VSH_OT_BOOL, 0, N_("try harder on risky reverts")},
+    {"allow-respawn", VSH_OT_BOOL, 0,
+      N_("allow respawn of hypervisor on certain operations")},
     {NULL, 0, 0, NULL}
 };

@@ -1539,6 +1541,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED;
     if (vshCommandOptBool(cmd, "stopped"))
         flags |= VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED;
+    if (vshCommandOptBool(cmd, "allow-respawn"))
+        flags |= VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN;
     /* We want virsh snapshot-revert --force to work even when talking
      * to older servers that did the unsafe revert by default but
      * reject the flag, so we probe without the flag, and only use it
diff --git a/tools/virsh.pod b/tools/virsh.pod
index e940c5f..0269709 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2810,7 +2810,7 @@ Output the name of the parent snapshot, if any, for the given
 I<snapshot>, or for the current snapshot with I<--current>.

 =item B<snapshot-revert> I<domain> {I<snapshot> | I<--current>}
-[{I<--running> | I<--paused> | I<--stopped>}] [I<--force>]
+[{I<--running> | I<--paused> | I<--stopped>}] [I<--force>] [I<--respawn>]

 Revert the given domain to the snapshot specified by I<snapshot>, or to
 the current snapshot with I<--current>.  Be aware
@@ -2830,6 +2830,10 @@ I<--running> or I<--paused> flags when reverting to a disk snapshot of a
 transient domain. The I<--stopped> flag cannot be used on snapshots
 of transient domains.

+Some snapshot revert approaches may require a respawn of the hypervisor
+process. This is not allowed by default. You may specify I<--allow-respawn>
+to override this limit.
+
 There are two cases where a snapshot revert involves extra risk, which
 requires the use of I<--force> to proceed.  One is the case of a
 snapshot that lacks full domain information for reverting
-- 
1.8.0