[libvirt] [PATCHv2 0/2] Deal with snapshots in PMSUSPENDED state
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
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
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
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
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
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
participants (2)
-
Eric Blake -
Peter Krempa