11:49 a.m.
This series adds basic support for reverting to a snapshot. For more advanced
support we need the creation of snapshot branches.
As of now, we will be able just to invalidate the snapshot tree from the point
we're reverting to.
I'll follow up with patches that add ability to delete children snapshots when
reverting
to a deeper tree.
Peter Krempa (5):
qemu: Split out guts of qemuDomainSaveImageStartVM() to allow reuse
snapshot: qemu: Fix detection of external snapshots when deleting
snapshot: Add flag VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED
snapshot: Add flag to allow hypervisor restart when reverting
snapshots
snapshot: qemu: Implement reverting of external snapshots
include/libvirt/libvirt.h.in | 3 +
src/conf/snapshot_conf.c | 17 ++
src/conf/snapshot_conf.h | 2 +
src/libvirt.c | 36 ++--
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 383 ++++++++++++++++++++++++++++++++++++++++---
tools/virsh-snapshot.c | 7 +
tools/virsh.pod | 19 ++-
8 files changed, 423 insertions(+), 45 deletions(-)
--
1.8.0
11:49 a.m.
New subject: [libvirt] [PATCHv3 1/5] qemu: Split out guts of qemuDomainSaveImageStartVM() to allow reuse
The workhorse part of qemuDomainSaveImageStartVM can be reused while
loading external snapshots. This patch splits the code out into a new
function qemuDomainSaveImageLoad that is free of setting lifecycle
events.
---
Previous version ACKed. Just a repost.
---
src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 36 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5556f1e..2ddf63a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4956,19 +4956,19 @@ error:
return -1;
}
-static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
-qemuDomainSaveImageStartVM(virConnectPtr conn,
- struct qemud_driver *driver,
- virDomainObjPtr vm,
- int *fd,
- const struct qemud_save_header *header,
- const char *path,
- bool start_paused)
+/* this helper loads the save image into a new qemu process */
+static int
+qemuDomainSaveImageLoad(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ int *fd,
+ const struct qemud_save_header *header,
+ const char *path)
+
{
- int ret = -1;
- virDomainEventPtr event;
int intermediatefd = -1;
virCommandPtr cmd = NULL;
+ int ret = -1;
if (header->version == 2) {
const char *prog = qemudSaveCompressionTypeToString(header->compressed);
@@ -4976,7 +4976,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
virReportError(VIR_ERR_OPERATION_FAILED,
_("Invalid compressed save format %d"),
header->compressed);
- goto out;
+ goto cleanup;
}
if (header->compressed != QEMUD_SAVE_FORMAT_RAW) {
@@ -4992,7 +4992,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
_("Failed to start decompression binary %s"),
prog);
*fd = intermediatefd;
- goto out;
+ goto cleanup;
}
}
}
@@ -5022,6 +5022,30 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
ret = -1;
}
+cleanup:
+ virCommandFree(cmd);
+ if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager,
+ vm->def, path) < 0)
+ VIR_WARN("failed to restore save state label on %s", path);
+
+
+ return ret;
+}
+
+static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6)
+qemuDomainSaveImageStartVM(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ int *fd,
+ const struct qemud_save_header *header,
+ const char *path,
+ bool start_paused)
+{
+ virDomainEventPtr event;
+ int ret = -1;
+
+ ret = qemuDomainSaveImageLoad(conn, driver, vm, fd, header, path);
+
if (ret < 0) {
virDomainAuditStart(vm, "restored", false);
goto out;
@@ -5062,11 +5086,6 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
ret = 0;
out:
- virCommandFree(cmd);
- if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager,
- vm->def, path) < 0)
- VIR_WARN("failed to restore save state label on %s", path);
-
return ret;
}
--
1.8.0
3:34 p.m.
New subject: [libvirt] [PATCHv3 1/5] qemu: Split out guts of qemuDomainSaveImageStartVM() to allow reuse
On 11/12/2012 10:49 AM, Peter Krempa wrote:
The workhorse part of qemuDomainSaveImageStartVM can be reused while
loading external snapshots. This patch splits the code out into a new
function qemuDomainSaveImageLoad that is free of setting lifecycle
events.
---
Previous version ACKed. Just a repost.
---
src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 36 insertions(+), 17 deletions(-)
ACK still stands.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:49 a.m.
New subject: [libvirt] [PATCHv3 2/5] snapshot: qemu: Fix detection of external snapshots when deleting
This patch adds a helper to determine if snapshots are external and uses
the helper to fix detection of those in snapshot deletion code.
Snapshots are external if they have an external memory image or if the
disk locations are external. As mixed snapshots are forbidden for now
we need to check just one disk to know.
---
- Squashed the patch adding the helper with the actual fix.
- Fixed detection of external snapshots
---
src/conf/snapshot_conf.c | 17 +++++++++++++++++
src/conf/snapshot_conf.h | 2 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 6 +++---
4 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 691950a..aa2b526 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -1023,3 +1023,20 @@ cleanup:
}
return ret;
}
+
+
+bool
+virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap)
+{
+ int i;
+
+ if (snap->def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+ return true;
+
+ for (i = 0; i < snap->def->ndisks; i++) {
+ if (snap->def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+ return true;
+ }
+
+ return false;
+}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 00775ae..b5c6e25 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -155,6 +155,8 @@ int virDomainListSnapshots(virDomainSnapshotObjListPtr snapshots,
virDomainSnapshotPtr **snaps,
unsigned int flags);
+bool virDomainSnapshotIsExternal(virDomainSnapshotObjPtr snap);
+
VIR_ENUM_DECL(virDomainSnapshotLocation)
VIR_ENUM_DECL(virDomainSnapshotState)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e94b478..5a07139 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1083,6 +1083,7 @@ virDomainSnapshotFindByName;
virDomainSnapshotForEach;
virDomainSnapshotForEachChild;
virDomainSnapshotForEachDescendant;
+virDomainSnapshotIsExternal;
virDomainSnapshotLocationTypeFromString;
virDomainSnapshotLocationTypeToString;
virDomainSnapshotObjListGetNames;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2ddf63a..c33080d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2025,7 +2025,7 @@ cleanup:
}
-/* Count how many snapshots in a set have external disk snapshots. */
+/* Count how many snapshots in a set are external snapshots or checkpoints. */
static void
qemuDomainSnapshotCountExternal(void *payload,
const void *name ATTRIBUTE_UNUSED,
@@ -2034,7 +2034,7 @@ qemuDomainSnapshotCountExternal(void *payload,
virDomainSnapshotObjPtr snap = payload;
int *count = data;
- if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
+ if (virDomainSnapshotIsExternal(snap))
(*count)++;
}
@@ -12531,7 +12531,7 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr
snapshot,
if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY)) {
if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) &&
- snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT)
+ virDomainSnapshotIsExternal(snap))
external++;
if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN)
virDomainSnapshotForEachDescendant(snap,
--
1.8.0
1:06 p.m.
New subject: [libvirt] [PATCHv3 2/5] snapshot: qemu: Fix detection of external snapshots when deleting
On 11/12/2012 10:49 AM, Peter Krempa wrote:
This patch adds a helper to determine if snapshots are external and
uses
the helper to fix detection of those in snapshot deletion code.
Snapshots are external if they have an external memory image or if the
disk locations are external. As mixed snapshots are forbidden for now
we need to check just one disk to know.
---
- Squashed the patch adding the helper with the actual fix.
- Fixed detection of external snapshots
---
src/conf/snapshot_conf.c | 17 +++++++++++++++++
src/conf/snapshot_conf.h | 2 ++
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 6 +++---
4 files changed, 23 insertions(+), 3 deletions(-)
ACK; this can be applied now, in part because I need this particular
patch for my next post. I still need to look at the rest of your
series, though.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:46 p.m.
New subject: [libvirt] [PATCHv3 2/5] snapshot: qemu: Fix detection of external snapshots when deleting
On 11/13/12 20:06, Eric Blake wrote:
On 11/12/2012 10:49 AM, Peter Krempa wrote:
> This patch adds a helper to determine if snapshots are external and uses
> the helper to fix detection of those in snapshot deletion code.
>
> Snapshots are external if they have an external memory image or if the
> disk locations are external. As mixed snapshots are forbidden for now
> we need to check just one disk to know.
> ---
> - Squashed the patch adding the helper with the actual fix.
> - Fixed detection of external snapshots
> ---
> src/conf/snapshot_conf.c | 17 +++++++++++++++++
> src/conf/snapshot_conf.h | 2 ++
> src/libvirt_private.syms | 1 +
> src/qemu/qemu_driver.c | 6 +++---
> 4 files changed, 23 insertions(+), 3 deletions(-)
ACK; this can be applied now, in part because I need this particular
patch for my next post. I still need to look at the rest of your
series, though.
Thanks; I pushed this one to ease the review and merge of your series.
Peter
11:49 a.m.
New subject: [libvirt] [PATCHv3 3/5] snapshot: Add flag VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED
The current snapshot reverting api supported changing the state of the
machine after the snapshot was reverted to either started or paused.
This patch adds the ability to revert the state but to stopped state.
---
- fixed libvirt.c and virsh.pod docs.
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 31 ++++++++++++++++---------------
tools/virsh-snapshot.c | 3 +++
tools/virsh.pod | 15 +++++++++------
4 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index bf584a0..f5bbc89 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3873,6 +3873,7 @@ 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 */
+ VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED = 1 << 3, /* Revert into stopped state */
} virDomainSnapshotRevertFlags;
/* Revert the domain to a point-in-time snapshot. The
diff --git a/src/libvirt.c b/src/libvirt.c
index bdb1dc6..b6b885c 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18638,17 +18638,17 @@ error:
*
* Revert the domain to a given snapshot.
*
- * Normally, the domain will revert to the same state the domain was
- * in while the snapshot was taken (whether inactive, running, or
- * paused), except that disk snapshots default to reverting to
- * inactive state. Including VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING in
- * @flags overrides the snapshot state to guarantee a running domain
- * after the revert; or including VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED in
- * @flags guarantees a paused domain after the revert. These two
- * flags are mutually exclusive. While a persistent domain does not
- * need either flag, it is not possible to revert a transient domain
- * into an inactive state, so transient domains require the use of one
- * of these two flags.
+ * Normally, the domain will revert to the same state the domain was in while
+ * the snapshot was taken (whether inactive, running, or paused), except that
+ * disk snapshots default to reverting to inactive state. Including
+ * VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING in @flags overrides the snapshot state to
+ * guarantee a running domain after the revert; or including
+ * VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED in @flags guarantees a paused domain after
+ * the revert. With VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED contained in the flags
+ * the domain's memory state is not restored. These three flags are mutually
+ * exclusive. While a persistent domain does not need either flag, it is not
+ * possible to revert a transient domain 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
@@ -18697,11 +18697,12 @@ virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
goto error;
}
- if ((flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
- (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
+ if ((!!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) +
+ !!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED) +
+ !!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED)) > 1) {
virReportInvalidArg(flags,
- _("running and paused flags in %s are mutually
exclusive"),
- __FUNCTION__);
+ _("running, paused and stopped flags in %s are "
+ "mutually exclusive"), __FUNCTION__);
goto error;
}
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 4281109..952dec5 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -1517,6 +1517,7 @@ static const vshCmdOptDef opts_snapshot_revert[] = {
{"current", VSH_OT_BOOL, 0, N_("revert to current snapshot")},
{"running", VSH_OT_BOOL, 0, N_("after reverting, change state to
running")},
{"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")},
{NULL, 0, 0, NULL}
};
@@ -1536,6 +1537,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd)
flags |= VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING;
if (vshCommandOptBool(cmd, "paused"))
flags |= VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED;
+ if (vshCommandOptBool(cmd, "stopped"))
+ flags |= VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED;
/* 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 0984e6e..ce5d010 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2822,7 +2822,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<--force>]
+[{I<--running> | I<--paused> | I<--stopped>}] [I<--force>]
Revert the given domain to the snapshot specified by I<snapshot>, or to
the current snapshot with I<--current>. Be aware
@@ -2833,11 +2833,14 @@ the original snapshot was taken.
Normally, reverting to a snapshot leaves the domain in the state it was
at the time the snapshot was created, except that a disk snapshot with
-no vm state leaves the domain in an inactive state. Passing either the
-I<--running> or I<--paused> flag will perform additional state changes
-(such as booting an inactive domain, or pausing a running domain). Since
-transient domains cannot be inactive, it is required to use one of these
-flags when reverting to a disk snapshot of a transient domain.
+no vm state leaves the domain in an inactive state. Passing one of the
+I<--running>, I<--paused> or I<--stopped> flag will perform additional
+state changes such as booting an inactive domain, pausing a running domain
+or shutting the domain down after the snapshot is reverted. Since
+transient domains cannot be inactive, it is required to use one of
+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.
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
--
1.8.0
5:39 p.m.
New subject: [libvirt] [PATCHv3 3/5] snapshot: Add flag VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED
On 11/12/2012 10:49 AM, Peter Krempa wrote:
The current snapshot reverting api supported changing the state of
the
machine after the snapshot was reverted to either started or paused.
This patch adds the ability to revert the state but to stopped state.
---
- fixed libvirt.c and virsh.pod docs.
---
include/libvirt/libvirt.h.in | 1 +
src/libvirt.c | 31 ++++++++++++++++---------------
tools/virsh-snapshot.c | 3 +++
tools/virsh.pod | 15 +++++++++------
4 files changed, 29 insertions(+), 21 deletions(-)
+ * Normally, the domain will revert to the same state the domain was in while
+ * the snapshot was taken (whether inactive, running, or paused), except that
+ * disk snapshots default to reverting to inactive state. Including
+ * VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING in @flags overrides the snapshot state to
+ * guarantee a running domain after the revert; or including
+ * VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED in @flags guarantees a paused domain after
+ * the revert. With VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED contained in the flags
+ * the domain's memory state is not restored. These three flags are mutually
+ * exclusive. While a persistent domain does not need either flag, it is not
+ * possible to revert a transient domain into an inactive state, so transient
+ * domains require the use of one of these two flags.
Change the last sentence:
While a persistent domain does not need any of these flags, 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.
=item B<snapshot-revert> I<domain> {I<snapshot> |
I<--current>}
-[{I<--running> | I<--paused>}] [I<--force>]
+[{I<--running> | I<--paused> | I<--stopped>}] [I<--force>]
Revert the given domain to the snapshot specified by I<snapshot>, or to
the current snapshot with I<--current>. Be aware
@@ -2833,11 +2833,14 @@ the original snapshot was taken.
Normally, reverting to a snapshot leaves the domain in the state it was
at the time the snapshot was created, except that a disk snapshot with
-no vm state leaves the domain in an inactive state. Passing either the
-I<--running> or I<--paused> flag will perform additional state changes
-(such as booting an inactive domain, or pausing a running domain). Since
-transient domains cannot be inactive, it is required to use one of these
-flags when reverting to a disk snapshot of a transient domain.
+no vm state leaves the domain in an inactive state. Passing one of the
+I<--running>, I<--paused> or I<--stopped> flag will perform
additional
+state changes such as booting an inactive domain, pausing a running domain
+or shutting the domain down after the snapshot is reverted. Since
+transient domains cannot be inactive, it is required to use one of
+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.
But here you did fine.
ACK with the one tweak in libvirt.c.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:49 a.m.
New subject: [libvirt] [PATCHv3 4/5] snapshot: Add flag to allow hypervisor restart when reverting snapshots
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.
---
New in series.
Please see the next patch on actual use semantics. I was considering how to
split the very powerful meaning of VIR_DOMAIN_SNAPSHOT_REVERT_FORCE to something
less powerful but still useful. (I could add a flag that would allow invalidating of
snapshot/image but that wouldn't be used that much)
---
include/libvirt/libvirt.h.in | 2 ++
src/libvirt.c | 5 +++++
tools/virsh-snapshot.c | 4 ++++
tools/virsh.pod | 6 +++++-
4 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index f5bbc89..2656a58 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3874,6 +3874,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 b6b885c..098fe06 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18650,6 +18650,11 @@ error:
* possible to revert a transient domain into an inactive state, so transient
* domains require the use of one of these two flags.
*
+ * 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/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index 952dec5..a67cbd8 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 ce5d010..29e955b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2822,7 +2822,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
@@ -2842,6 +2842,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
9:28 a.m.
New subject: [libvirt] [PATCHv3 4/5] snapshot: Add flag to allow hypervisor restart when reverting snapshots
On 11/12/12 18:49, 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.
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.
---
New in series.
Please see the next patch on actual use semantics. I was considering how to
split the very powerful meaning of VIR_DOMAIN_SNAPSHOT_REVERT_FORCE to something
less powerful but still useful. (I could add a flag that would allow invalidating of
snapshot/image but that wouldn't be used that much)
---
include/libvirt/libvirt.h.in | 2 ++
src/libvirt.c | 5 +++++
tools/virsh-snapshot.c | 4 ++++
tools/virsh.pod | 6 +++++-
4 files changed, 16 insertions(+), 1 deletion(-)
I should have added code to libvirt.c that will set the _RESPAWN flag
when force is specified to save hassle. I will do it in the next version
if this idea doesn't get NACKed to oblivion.
Peter
5:42 p.m.
New subject: [libvirt] [PATCHv3 4/5] snapshot: Add flag to allow hypervisor restart when reverting snapshots
On 11/13/2012 08:28 AM, Peter Krempa wrote:
I should have added code to libvirt.c that will set the _RESPAWN flag
when force is specified to save hassle. I will do it in the next version
if this idea doesn't get NACKed to oblivion.
I'm still thinking about the original patch, but _don't_ add code to
libvirt.c that turns on one flag when another is present. Remember,
virsh linked to libvirt.so 1.0.1 can call into libvirtd linked to
libvirt.so 1.0.0, and if libvirt.c auto-sets the _RESPAWN flag in 1.0.1,
then the RPC call will fail because 1.0.0 will reject it.
Any automatic handling of assuming _RESPAWN if _FORCE is present must be
done in the individual drivers, such as qemu_driver.c.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:04 p.m.
New subject: [libvirt] [PATCHv3 5/5] snapshot: qemu: Implement reverting of external snapshots
This patch adds support for reverting of external snapshots. The support
is somewhat limited yet (you can only revert to a snapshot that has no
children or delete the children that would have their image chains
invalidated).
While reverting an external snapshot, the domain has to be taken offline
as there's no possibility to start a migration from file on a running
machine. This poses a few difficulties when the user has virt-viewer
attached as appropriate events need to be re-emitted even if the machine
doesn't change states.
---
The qemuDomainSnapshotCreateEvent helper func can't be split out as it's used by
code added in this patch only.
tweaks:
- fixed typos
- removed PMSUSPEND handling capability
- removed support for deleting guests
- reordered code to make more bulletproof
- fixed potential segfault when image is missing
- and a few others I can't remember.
---
src/qemu/qemu_driver.c | 331 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 327 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c33080d..6cc77b7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12116,6 +12116,316 @@ qemuDomainSnapshotRevertInactive(struct qemud_driver *driver,
return ret > 0 ? -1 : ret;
}
+/* helper to create lifecycle for possible outcomes of reverting snapshots
+ * the reemit parameter is used when specific events need to be reemitted
+ * so that virt-viewer for example is able to re-connect */
+static int
+qemuDomainSnapshotCreateEvent(struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainState old_state,
+ virDomainState new_state,
+ bool reemit)
+{
+ int reason = 0; /* generic unknown reason */
+ int type = -1;
+ virDomainEventPtr event;
+
+ switch (new_state) {
+ case VIR_DOMAIN_RUNNING:
+ switch (old_state) {
+ case VIR_DOMAIN_RUNNING:
+ if (!reemit)
+ break;
+ /* fallthrough */
+ case VIR_DOMAIN_NOSTATE:
+ case VIR_DOMAIN_SHUTOFF:
+ case VIR_DOMAIN_CRASHED:
+ case VIR_DOMAIN_BLOCKED:
+ case VIR_DOMAIN_SHUTDOWN:
+ type = VIR_DOMAIN_EVENT_STARTED;
+ reason = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
+ break;
+
+ case VIR_DOMAIN_PMSUSPENDED:
+ case VIR_DOMAIN_PAUSED:
+ type = VIR_DOMAIN_EVENT_RESUMED;
+ reason = VIR_DOMAIN_EVENT_RESUMED_FROM_SNAPSHOT;
+ break;
+
+ case VIR_DOMAIN_LAST:
+ /* no event */
+ break;
+ }
+ break;
+
+ case VIR_DOMAIN_PAUSED:
+ switch (old_state) {
+ case VIR_DOMAIN_NOSTATE:
+ case VIR_DOMAIN_SHUTOFF:
+ case VIR_DOMAIN_CRASHED:
+ /* the machine was started here, so we need an additional event */
+ event = virDomainEventNewFromObj(vm,
+ VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT);
+ if (!event)
+ goto no_memory;
+ qemuDomainEventQueue(driver, event);
+ /* fallthrough! */
+ case VIR_DOMAIN_PMSUSPENDED:
+ case VIR_DOMAIN_RUNNING:
+ case VIR_DOMAIN_SHUTDOWN:
+ case VIR_DOMAIN_BLOCKED:
+ type = VIR_DOMAIN_EVENT_SUSPENDED;
+ reason = VIR_DOMAIN_EVENT_SUSPENDED_FROM_SNAPSHOT;
+ break;
+
+ case VIR_DOMAIN_PAUSED:
+ case VIR_DOMAIN_LAST:
+ /* no event */
+ break;
+ }
+ break;
+
+ case VIR_DOMAIN_SHUTOFF:
+ switch (old_state) {
+ case VIR_DOMAIN_NOSTATE:
+ case VIR_DOMAIN_BLOCKED:
+ case VIR_DOMAIN_SHUTDOWN:
+ case VIR_DOMAIN_PAUSED:
+ case VIR_DOMAIN_RUNNING:
+ case VIR_DOMAIN_PMSUSPENDED:
+ type = VIR_DOMAIN_EVENT_STOPPED;
+ reason = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT;
+ break;
+
+ case VIR_DOMAIN_SHUTOFF:
+ case VIR_DOMAIN_CRASHED:
+ case VIR_DOMAIN_LAST:
+ /* no event */
+ break;
+ }
+ break;
+
+ /* fallthrough */
+ case VIR_DOMAIN_NOSTATE:
+ case VIR_DOMAIN_BLOCKED:
+ case VIR_DOMAIN_SHUTDOWN:
+ case VIR_DOMAIN_CRASHED:
+ case VIR_DOMAIN_LAST:
+ case VIR_DOMAIN_PMSUSPENDED:
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Suspicious domain state '%d' after
snapshot"),
+ new_state);
+ return -1;
+ }
+
+ if (!(event = virDomainEventNewFromObj(vm, type, reason)))
+ goto no_memory;
+
+ qemuDomainEventQueue(driver, event);
+
+ return 0;
+
+no_memory:
+ virReportOOMError();
+ return -1;
+}
+
+
+/* The domain and driver is expected to be locked */
+static int
+qemuDomainSnapshotRevertExternal(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ virDomainSnapshotObjPtr snap,
+ unsigned int flags)
+{
+ int ret = -1;
+ int loadfd = -1;
+
+ virDomainDefPtr save_def = NULL;
+ struct qemud_save_header header;
+ virFileWrapperFdPtr wrapperFd = NULL;
+
+ virDomainState old_state = virDomainObjGetState(vm, NULL);
+ virDomainDefPtr config = NULL;
+
+ /* check if domain is running and should be running afterwards:
+ * qemu has to be re-started to allow the migration from file
+ */
+ if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN ||
+ flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE) &&
+ virDomainObjIsActive(vm) &&
+ !(snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT ||
+ snap->def->state == VIR_DOMAIN_SHUTOFF)) {
+ virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
+ _("Reverting of snapshot '%s' needs to restart the
"
+ "hypervisor."), snap->def->name);
+ goto cleanup;
+ }
+
+ /* check if snapshot has children */
+ if (snap->nchildren > 0) {
+ if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
+ virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",
+ _("Reverting of snapshots with children "
+ "is not implemented yet."));
+ goto cleanup;
+ } else {
+ VIR_WARN("Invalidating image chain of snapshot '%s'.",
+ snap->def->name);
+ }
+ }
+
+ if (vm->current_snapshot) {
+ vm->current_snapshot->def->current = false;
+ if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
+ driver->snapshotDir) < 0)
+ goto cleanup;
+ vm->current_snapshot = NULL;
+ /* XXX Should we restore vm->current_snapshot after this point
+ * in the failure cases where we know there was no change? */
+ }
+
+ /* Prepare to copy the snapshot inactive xml as the config of this
+ * domain. Easiest way is by a round trip through xml.
+ *
+ * XXX Should domain snapshots track live xml rather
+ * than inactive xml? */
+ snap->def->current = true;
+ if (snap->def->dom) {
+ char *xml;
+ if (!(xml = qemuDomainDefFormatXML(driver,
+ snap->def->dom,
+ VIR_DOMAIN_XML_INACTIVE |
+ VIR_DOMAIN_XML_SECURE |
+ VIR_DOMAIN_XML_MIGRATABLE)))
+ goto cleanup;
+
+ config = virDomainDefParseString(driver->caps, xml,
+ QEMU_EXPECTED_VIRT_TYPES,
+ VIR_DOMAIN_XML_INACTIVE);
+ VIR_FREE(xml);
+ if (!config)
+ goto cleanup;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Domain definition not found in snapshot
'%s'"),
+ snap->def->name);
+ goto cleanup;
+ }
+
+ /* try to open the memory save image if one exists and it's needed */
+ if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED) &&
+ (snap->def->state == VIR_DOMAIN_RUNNING ||
+ snap->def->state == VIR_DOMAIN_PAUSED ||
+ flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING ||
+ flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
+
+ if (snap->def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
+ if (!snap->def->file) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Memory image path does nott exist for "
+ "the snapshot"));
+ goto cleanup;
+ }
+
+ if (!virFileExists(snap->def->file)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("memory save file '%s' does not
exist."),
+ snap->def->file);
+ goto cleanup;
+ }
+
+ /* open the save image file */
+ if ((loadfd = qemuDomainSaveImageOpen(driver, snap->def->file,
+ &save_def, &header,
+ false, &wrapperFd, NULL,
+ -1, false, false)) < 0)
+ goto cleanup;
+
+ /* opening succeeded, there's a big probability the revert will work.
+ * We can now get rid of the active qemu, if it runs */
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Can't revert snapshot to running state: "
+ "Memory image not found"));
+ goto cleanup;
+ }
+ }
+
+ if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
+ goto cleanup;
+
+ /* Now we destroy the (possibly) running qemu process.
+ * Unfortunately, the guest can be restored only using incoming migration.
+ */
+ if (virDomainObjIsActive(vm)) {
+ qemuProcessStop(driver, vm,
+ VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, 0);
+ virDomainAuditStop(vm, "from-snapshot");
+ }
+
+ /* assign domain definition */
+ virDomainObjAssignDef(vm, config, false);
+ config = NULL;
+
+ /* wipe and re-create disk images - qemu-img doesn't care if it exists*/
+ if (qemuDomainSnapshotCreateInactiveExternal(driver, vm, snap, false) < 0)
+ goto endjob;
+
+ /* save the config files */
+ if (virDomainSaveConfig(driver->configDir, vm->def) < 0) {
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Write to config file failed"));
+ goto endjob;
+ }
+
+ /* re-start the guest if necessary */
+ if (loadfd > 0) {
+ if ((ret = qemuDomainSaveImageLoad(conn, driver,
+ vm, &loadfd,
+ &header, snap->def->file)) < 0)
{
+ virDomainAuditStart(vm, "from-snapshot", false);
+ goto endjob;
+ }
+
+ virDomainAuditStart(vm, "from-snapshot", true);
+
+ if ((snap->def->state == VIR_DOMAIN_RUNNING &&
+ !(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) ||
+ flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) {
+ if (qemuProcessStartCPUs(driver, vm, conn,
+ VIR_DOMAIN_RUNNING_FROM_SNAPSHOT,
+ QEMU_ASYNC_JOB_NONE) < 0)
+ goto endjob;
+ }
+ }
+
+ /* create corresponding life cycle events */
+ if (qemuDomainSnapshotCreateEvent(driver, vm, old_state,
+ virDomainObjGetState(vm, NULL),
+ !!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN))
< 0)
+ goto endjob;
+
+ ret = 0;
+endjob:
+ if (qemuDomainObjEndJob(driver, vm) == 0) {
+ vm = NULL;
+ } else if (ret < 0 && !vm->persistent) {
+ qemuDomainRemoveInactive(driver, vm);
+ vm = NULL;
+ }
+
+cleanup:
+ virDomainDefFree(config);
+ virDomainDefFree(save_def);
+ VIR_FORCE_CLOSE(loadfd);
+ virFileWrapperFdFree(wrapperFd);
+
+ return ret;
+}
+
static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
unsigned int flags)
{
@@ -12133,6 +12443,8 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
virCheckFlags(VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING |
VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED |
+ VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED |
+ VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN |
VIR_DOMAIN_SNAPSHOT_REVERT_FORCE, -1);
/* We have the following transitions, which create the following events:
@@ -12176,12 +12488,23 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
"to revert to inactive snapshot"));
goto cleanup;
}
- if (snap->def->state == VIR_DOMAIN_DISK_SNAPSHOT) {
+
+ /* Check if reverting an external snapshot */
+ if (virDomainSnapshotIsExternal(snap)) {
+ ret = qemuDomainSnapshotRevertExternal(snapshot->domain->conn,
+ driver, vm, snap, flags);
+ goto cleanup;
+ }
+
+ if (flags & (VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED |
+ VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("revert to external disk snapshot not supported "
- "yet"));
+ _("VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED and "
+ "VIR_DOMAIN_SNAPSHOT_REVERT_RESPAWN are unsupported "
+ "with internal snapshots"));
goto cleanup;
}
+
if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {
if (!snap->def->dom) {
virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
@@ -12200,7 +12523,6 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
}
}
-
if (vm->current_snapshot) {
vm->current_snapshot->def->current = false;
if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
@@ -12233,6 +12555,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
goto cleanup;
}
+ /* Internal snapshot revert code starts below */
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
--
1.8.0
4393
days inactive
4394
days old
11 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa