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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list