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