diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index bd7a0f7925..07617be489 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2723,6 +2723,7 @@ virDomainSnapshotPtr virDomainSnapshotGetParent(virDomainSnapshotPtr snapshot, typedef enum { VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 << 0, /* Run after revert */ VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED = 1 << 1, /* Pause after revert */ + VIR_DOMAIN_SNAPSHOT_REVERT_FORCE = 1 << 2, /* Allow risky reverts */ } virDomainSnapshotRevertFlags; /* Revert the domain to a point-in-time snapshot. The diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 0aae6229d1..0c98014b1e 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -237,6 +237,8 @@ typedef enum { the given driver */ VIR_ERR_STORAGE_PROBE_FAILED = 75, /* storage pool proble failed */ VIR_ERR_STORAGE_POOL_BUILT = 76, /* storage pool already built */ + VIR_ERR_SNAPSHOT_REVERT_RISKY = 77, /* force was not requested for a + risky domain snapshot revert */ } virErrorNumber; /** diff --git a/src/libvirt.c b/src/libvirt.c index 9080b2f7bf..182e031f9c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -16262,6 +16262,28 @@ error: * into an inactive state, so transient domains require the use of one * of these two flags. * + * 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 + * guest's RAM at the time. Since the very nature of keeping snapshots + * implies the intent to roll back state, no additional confirmation is + * normally required for these lossy effects. + * + * However, there are two particular situations where reverting will + * be refused by default, and where @flags must include + * VIR_DOMAIN_SNAPSHOT_REVERT_FORCE to acknowledge the risks. 1) Any + * attempt to revert to a snapshot that lacks the metadata to perform + * ABI compatibility checks (generally the case for snapshots that + * lack a full when listed by virDomainSnapshotGetXMLDesc(), + * such as those created prior to libvirt 0.9.5). 2) Any attempt to + * revert a running domain to an active state that requires starting a + * new hypervisor instance rather than reusing the existing hypervisor + * (since this would terminate all connections to the domain, such as + * 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. + * * Returns 0 if the creation is successful, -1 on error. */ int diff --git a/src/util/virterror.c b/src/util/virterror.c index 26c4981b06..5006fa27ed 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1204,6 +1204,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("argument unsupported: %s"); break; + case VIR_ERR_SNAPSHOT_REVERT_RISKY: + if (info == NULL) + errmsg = _("revert requires force"); + else + errmsg = _("revert requires force: %s"); + break; } return (errmsg); } diff --git a/tools/virsh.c b/tools/virsh.c index 3a17971948..48f2b8af7d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13446,6 +13446,7 @@ static const vshCmdOptDef opts_snapshot_revert[] = { {"snapshotname", VSH_OT_DATA, VSH_OFLAG_REQ, N_("snapshot name")}, {"running", VSH_OT_BOOL, 0, N_("after reverting, change state to running")}, {"paused", VSH_OT_BOOL, 0, N_("after reverting, change state to paused")}, + {"force", VSH_OT_BOOL, 0, N_("try harder on risky reverts")}, {NULL, 0, 0, NULL} }; @@ -13457,11 +13458,19 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; virDomainSnapshotPtr snapshot = NULL; unsigned int flags = 0; + bool force = false; + int result; if (vshCommandOptBool(cmd, "running")) flags |= VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING; if (vshCommandOptBool(cmd, "paused")) flags |= VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED; + /* 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 + * when the error says it will make a difference. */ + if (vshCommandOptBool(cmd, "force")) + force = true; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; @@ -13477,7 +13486,15 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) if (snapshot == NULL) goto cleanup; - if (virDomainRevertToSnapshot(snapshot, flags) < 0) + result = virDomainRevertToSnapshot(snapshot, flags); + if (result < 0 && force && + last_error->code == VIR_ERR_SNAPSHOT_REVERT_RISKY) { + flags |= VIR_DOMAIN_SNAPSHOT_REVERT_FORCE; + virFreeError(last_error); + last_error = NULL; + result = virDomainRevertToSnapshot(snapshot, flags); + } + if (result < 0) goto cleanup; ret = true; diff --git a/tools/virsh.pod b/tools/virsh.pod index be81afc470..1f7c76f3d2 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1995,6 +1995,7 @@ Using I<--security-info> will also include security sensitive information. Output the name of the parent snapshot for the given I, if any. =item B I I [{I<--running> | I<--paused>}] +[I<--force>] Revert the given domain to the snapshot specified by I. Be aware that this is a destructive action; any changes in the domain since the last @@ -2010,6 +2011,22 @@ I<--running> or I<--paused> flag will perform additional state changes transient domains cannot be inactive, it is required to use one of these flags when reverting to a disk snapshot of a transient domain. +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 +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. + =item B I I [I<--metadata>] [{I<--children> | I<--children-only>}]