[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 11/19/2012 03:53 PM, Eric Blake wrote:
> On 11/19/2012 09:06 AM, Peter Krempa wrote:
>> 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.
>>

>> @@ -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.
> 
> It might be worth mentioning that --force implies --allow-respawn (that
> is, --force is a supserset of --allow-respawn, in that it can force
> situations that --allow-respawn will not).

One more possible problem - the existing virsh code fakes '_FORCE' even
for older servers, by first trying without _FORCE, and then only adding
it if it would make a difference.  Should we also fake '_RESPAWN' in the
same manner?  Then again, if --allow-respawn fails because the server is
too old, you can still use --force; so I think we're okay.

Here's what I'm changing before my repost:

diff --git i/src/libvirt.c w/src/libvirt.c
index a8ce47b..762d829 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -18712,8 +18712,9 @@ error:
  *
  * 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.
+ * VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN to @flags. The hypervisor driver will
+ * re-emit the appropriate events to allow reconnect of management
+ * applications.  This flag is implied by VIR_DOMAIN_SNAPSHOT_REVERT_FORCE.
  *
  * Reverting to any snapshot discards all configuration changes made since
  * the last snapshot.  Additionally, reverting to a snapshot from a running
@@ -18735,7 +18736,8 @@ error:
  * such as VNC or Spice graphics) - this condition arises from active
  * snapshots that are provably ABI incomaptible, as well as from
  * inactive snapshots with a @flags request to start the domain after
- * the revert.
+ * the revert (this latter situation is also covered via the weaker
+ * VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN, as of libvirt 1.0.1).
  *
  * Returns 0 if the creation is successful, -1 on error.
  */
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 3de8197..b873604 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -12182,24 +12182,23 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                          "yet"));
         goto cleanup;
     }
-    if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
-        if (!snap->def->dom) {
-            virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
-                           _("snapshot '%s' lacks domain '%s' rollback
info"),
-                           snap->def->name, vm->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))) {
-            virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
-                           _("must respawn qemu to start inactive
snapshot"));
-            goto cleanup;
-        }
+    if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE) &&
+        !snap->def->dom) {
+        virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
+                       _("snapshot '%s' lacks domain '%s' rollback info"),
+                       snap->def->name, vm->def->name);
+        goto cleanup;
+    }
+    if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN) &&
+        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))) {
+        virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
+                       _("must respawn qemu to start inactive snapshot"));
+        goto cleanup;
     }
-

     if (vm->current_snapshot) {
         vm->current_snapshot->def->current = false;
diff --git i/tools/virsh.pod w/tools/virsh.pod
index 01b3625..cb41352 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -2845,25 +2845,18 @@ 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
+There are some cases where a snapshot revert involves extra risk, and
+where the revert does not succeed by default but can happen with extra
+flags.  The first case is any time that reverting from a running domain
+would require respawning the hypervisor, rather than reusing the existing
+one; this action can impact SPICE and VNC connections, and requires the
+I<--allow-respawn> or I<--force> flag.  Another problem is the case of a
 snapshot that lacks full domain information for reverting
 configuration (such as snapshots created prior to libvirt 0.9.5);
 since libvirt cannot prove that the current configuration matches what
 was in use at the time of the snapshot, supplying I<--force> assures
 libvirt that the snapshot is compatible with the current configuration
-(and if it is not, the domain will likely fail to run).  The other is
-the case of reverting from a running domain to an active state where a
-new hypervisor has to be created rather than reusing the existing
-hypervisor, because it implies drawbacks such as breaking any existing
-VNC or Spice connections; this condition happens with an active
-snapshot that uses a provably incompatible configuration, as well as
-with an inactive snapshot that is combined with the I<--start> or
-I<--pause> flag.
+(and if it is not, the domain will likely fail to run).

 =item B<snapshot-delete> I<domain> {I<snapshot> | I<--current>}
[I<--metadata>]
 [{I<--children> | I<--children-only>}]


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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]