9:08 a.m.
Peter Krempa (2):
qemu: snapshot: Forbid taking snapshot in invalid state
qemu: snapshot: Forbid taking/reverting snapshots in PMSUSPENDED state
src/qemu/qemu_driver.c | 33 +++++++++++++++++++++++++++++++--
1 file changed, 31 insertions(+), 2 deletions(-)
--
2.0.0
9:08 a.m.
New subject: [libvirt] [PATCHv2 1/2] qemu: snapshot: Forbid taking snapshot in invalid state
Similarly to 49a3a649a85f9d3d478be355aa8694bce889586a forbid creating
snapshots in domain states impossible to reach in qemu.
---
src/qemu/qemu_driver.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1782913..91baa7d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13393,6 +13393,26 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
goto cleanup;
}
+ /* allow snapshots only in certain states */
+ switch ((virDomainState) vm->state.state) {
+ /* valid states */
+ case VIR_DOMAIN_RUNNING:
+ case VIR_DOMAIN_PAUSED:
+ case VIR_DOMAIN_SHUTDOWN:
+ case VIR_DOMAIN_SHUTOFF:
+ case VIR_DOMAIN_CRASHED:
+ case VIR_DOMAIN_PMSUSPENDED:
+ break;
+
+ /* invalid states */
+ case VIR_DOMAIN_NOSTATE:
+ case VIR_DOMAIN_BLOCKED: /* invalid state, unused in qemu */
+ case VIR_DOMAIN_LAST:
+ virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid domain state %s"),
+ virDomainStateTypeToString(vm->state.state));
+ goto cleanup;
+ }
+
if (redefine) {
if (!virDomainSnapshotRedefinePrep(domain, vm, &def, &snap,
&update_current, flags) < 0)
--
2.0.0
11:41 a.m.
New subject: [libvirt] [PATCHv2 1/2] qemu: snapshot: Forbid taking snapshot in invalid state
On 07/21/2014 08:08 AM, Peter Krempa wrote:
Similarly to 49a3a649a85f9d3d478be355aa8694bce889586a forbid
creating
snapshots in domain states impossible to reach in qemu.
---
src/qemu/qemu_driver.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:08 a.m.
New subject: [libvirt] [PATCHv2 2/2] qemu: snapshot: Forbid taking/reverting snapshots in PMSUSPENDED state
Qemu doesn't currently support them and behaves strangely. Just forbid
them.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1079162
---
src/qemu/qemu_driver.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91baa7d..eae23d3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13401,9 +13401,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
case VIR_DOMAIN_SHUTDOWN:
case VIR_DOMAIN_SHUTOFF:
case VIR_DOMAIN_CRASHED:
- case VIR_DOMAIN_PMSUSPENDED:
break;
+ case VIR_DOMAIN_PMSUSPENDED:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("qemu doesn't support taking snapshots of "
+ "PMSUSPENDED guests"));
+ goto cleanup;
+
/* invalid states */
case VIR_DOMAIN_NOSTATE:
case VIR_DOMAIN_BLOCKED: /* invalid state, unused in qemu */
@@ -14178,8 +14183,6 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
case VIR_DOMAIN_SHUTDOWN:
case VIR_DOMAIN_SHUTOFF:
case VIR_DOMAIN_CRASHED:
- /* XXX: The following one is clearly wrong! */
- case VIR_DOMAIN_PMSUSPENDED:
/* Transitions 1, 4, 7 */
/* Newer qemu -loadvm refuses to revert to the state of a snapshot
* created by qemu-img snapshot -c. If the domain is running, we
@@ -14245,6 +14248,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
}
break;
+ case VIR_DOMAIN_PMSUSPENDED:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("qemu doesn't support reversion of snapshot taken in
"
+ "PMSUSPENDED state"));
+ goto cleanup;
+
case VIR_DOMAIN_NOSTATE:
case VIR_DOMAIN_BLOCKED:
case VIR_DOMAIN_LAST:
--
2.0.0
1:44 p.m.
New subject: [libvirt] [PATCHv2 2/2] qemu: snapshot: Forbid taking/reverting snapshots in PMSUSPENDED state
On 07/21/2014 08:08 AM, Peter Krempa wrote:
Qemu doesn't currently support them and behaves strangely. Just
forbid
them.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1079162
---
src/qemu/qemu_driver.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 91baa7d..eae23d3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13401,9 +13401,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
case VIR_DOMAIN_SHUTDOWN:
case VIR_DOMAIN_SHUTOFF:
case VIR_DOMAIN_CRASHED:
- case VIR_DOMAIN_PMSUSPENDED:
break;
+ case VIR_DOMAIN_PMSUSPENDED:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("qemu doesn't support taking snapshots of "
+ "PMSUSPENDED guests"));
+ goto cleanup;
+
This is a bit harsh; a bit nicer might be to take the snapshot anyways
but to have the snapshot be in the running state (as if the act of
taking the snapshot caused a pmwakeup event). But that's more
complicated, and changes domain state implicitly, while this approach is
vocal to the user why we can't do it (and if qemu ever DOES add support,
we just gate this code by a capability detection bit).
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:54 a.m.
New subject: [libvirt] [PATCHv2 2/2] qemu: snapshot: Forbid taking/reverting snapshots in PMSUSPENDED state
On 07/21/14 20:44, Eric Blake wrote:
On 07/21/2014 08:08 AM, Peter Krempa wrote:
> Qemu doesn't currently support them and behaves strangely. Just forbid
> them.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1079162
> ---
> src/qemu/qemu_driver.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 91baa7d..eae23d3 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -13401,9 +13401,14 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
> case VIR_DOMAIN_SHUTDOWN:
> case VIR_DOMAIN_SHUTOFF:
> case VIR_DOMAIN_CRASHED:
> - case VIR_DOMAIN_PMSUSPENDED:
> break;
>
> + case VIR_DOMAIN_PMSUSPENDED:
> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> + _("qemu doesn't support taking snapshots of "
> + "PMSUSPENDED guests"));
> + goto cleanup;
> +
This is a bit harsh; a bit nicer might be to take the snapshot anyways
but to have the snapshot be in the running state (as if the act of
taking the snapshot caused a pmwakeup event). But that's more
complicated, and changes domain state implicitly, while this approach is
vocal to the user why we can't do it (and if qemu ever DOES add support,
we just gate this code by a capability detection bit).
Well that would be a bit more wrong in my eyes than allowing the
snapshot and leaving it in a crippled state until qemu fixes it's part.
We would need to introduce a lot of code that would handle the
transitions and I doubt that it would get much use. Same as I think that
after qemu possibly fixes the problem it will be forgotten for a long time.
ACK.
Anyways, I'll push this patch now as I don't think it's worth spending
more time on this.
Peter
3776
days inactive
3777
days old
5 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa