4:07 a.m.
Peter Krempa (2):
qemu: snapshot: Reject internal active snapshot without memory state
qemu: snapshot: Improve detection of mixed snapshots
src/qemu/qemu_driver.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
--
1.9.3
4:07 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: snapshot: Reject internal active snapshot without memory state
A internal snapshot of a active VM with the memory snapshot disabled
explicitly would actually still take the memory snapshot. Reject it
explicitly.
Before:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=no
Domain snapshot 1401353155 created
After:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=no
error: Operation not supported: internal snapshot of a running VM must include the memory
state
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1083345
---
src/qemu/qemu_driver.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9becc0a..272687d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13495,6 +13495,15 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
align_match = false;
} else {
def->state = virDomainObjGetState(vm, NULL);
+
+ if (virDomainObjIsActive(vm) &&
+ def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("internal snapshot of a running VM "
+ "must include the memory state"));
+ goto cleanup;
+ }
+
def->memory = (def->state == VIR_DOMAIN_SHUTOFF ?
VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
--
1.9.3
9:36 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: snapshot: Reject internal active snapshot without memory state
On 05/29/2014 03:07 AM, Peter Krempa wrote:
A internal snapshot of a active VM with the memory snapshot disabled
explicitly would actually still take the memory snapshot. Reject it
explicitly.
Before:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=no
Domain snapshot 1401353155 created
After:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=no
error: Operation not supported: internal snapshot of a running VM must include the
memory state
Technically, qemu 1.7 added blockdev-snapshot-internal-sync, so we CAN
take internal disk-only snapshots now. But as that is more complex to
implement, I agree with this patch as an intermediate fix (and we'll
still have to keep this code path for qemu 1.6 and earlier, based on a
qemu_capabilities.h bit).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1083345
---
src/qemu/qemu_driver.c | 9 +++++++++
1 file changed, 9 insertions(+)
ACK, safe for 1.2.5.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:05 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: snapshot: Reject internal active snapshot without memory state
On 05/29/14 16:36, Eric Blake wrote:
On 05/29/2014 03:07 AM, Peter Krempa wrote:
> A internal snapshot of a active VM with the memory snapshot disabled
> explicitly would actually still take the memory snapshot. Reject it
> explicitly.
>
> Before:
> $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=no
> Domain snapshot 1401353155 created
>
> After:
> $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=no
> error: Operation not supported: internal snapshot of a running VM must include the
memory state
Technically, qemu 1.7 added blockdev-snapshot-internal-sync, so we CAN
take internal disk-only snapshots now. But as that is more complex to
implement, I agree with this patch as an intermediate fix (and we'll
still have to keep this code path for qemu 1.6 and earlier, based on a
qemu_capabilities.h bit).
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1083345
> ---
> src/qemu/qemu_driver.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
ACK, safe for 1.2.5.
Pushed; Thanks.
I'll open a BZ to track the addition.
Peter
4:08 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: snapshot: Improve detection of mixed snapshots
Currently we don't support mixed (external + internal) snapshots. The
code detecting the snapshot type didn't make sure that the memory image
was consistent with the snapshot type leading into strange error
message:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=external,file=/tmp/blah
error: internal error: unexpected code path
Fix the mixed detection code to detect this kind of mistake:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=external,file=/tmp/blah
error: unsupported configuration: mixing internal and external targets for a snapshot is
not yet supported
---
src/qemu/qemu_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 272687d..59185c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12755,7 +12755,9 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
/* For now, we don't allow mixing internal and external disks.
* XXX technically, we could mix internal and external disks for
* offline snapshots */
- if (found_internal && external) {
+ if ((found_internal && external) ||
+ (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && external)
||
+ (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL &&
found_internal)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("mixing internal and external targets for a snapshot
"
"is not yet supported"));
--
1.9.3
9:38 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: snapshot: Improve detection of mixed snapshots
On 05/29/2014 03:08 AM, Peter Krempa wrote:
Currently we don't support mixed (external + internal) snapshots.
The
code detecting the snapshot type didn't make sure that the memory image
was consistent with the snapshot type leading into strange error
message:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=external,file=/tmp/blah
error: internal error: unexpected code path
Fix the mixed detection code to detect this kind of mistake:
$ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=external,file=/tmp/blah
error: unsupported configuration: mixing internal and external targets for a snapshot is
not yet supported
It may still be nice to support some day down the road, but a better
error message in the meantime is worth having.
---
src/qemu/qemu_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
ACK; safe for 1.2.5.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:06 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: snapshot: Improve detection of mixed snapshots
On 05/29/14 16:38, Eric Blake wrote:
On 05/29/2014 03:08 AM, Peter Krempa wrote:
> Currently we don't support mixed (external + internal) snapshots. The
> code detecting the snapshot type didn't make sure that the memory image
> was consistent with the snapshot type leading into strange error
> message:
>
> $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=external,file=/tmp/blah
> error: internal error: unexpected code path
>
> Fix the mixed detection code to detect this kind of mistake:
>
> $ virsh snapshot-create-as --domain VM --diskspec vda,snapshot=internal --memspec
snapshot=external,file=/tmp/blah
> error: unsupported configuration: mixing internal and external targets for a
snapshot is not yet supported
It may still be nice to support some day down the road, but a better
error message in the meantime is worth having.
> ---
> src/qemu/qemu_driver.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
ACK; safe for 1.2.5.
Pushed; Thanks.
Peter
3830
days inactive
3830
days old
6 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa