[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 3/3] snapshot: rudimentary qemu support for atomic disk snapshot



On 03/17/2012 05:33 PM, Eric Blake wrote:
Taking an external snapshot of just one disk is atomic, without having
to pause and resume the VM.  This also paves the way for later patches
to interact with the new qemu 'transaction' monitor command.

The various scenarios when requesting atomic are:
online, 1 disk, old qemu - safe, allowed by this patch
online, more than 1 disk, old qemu - failure, this patch
offline snapshot - safe, once a future patch implements offline disk snapshot
online, 1 or more disks, new qemu - safe, once future patch uses transaction

Taking an online system checkpoint snapshot is atomic, since it is
done via a single 'savevm' monitor command.

Taking an offline system checkpoint snapshot currently uses multiple
qemu-img invocations with no provision for cleanup of partial failure,
so for now we mark it unsupported.

* src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): Support
new flag for single-disk setups.
(qemuDomainSnapshotDiskPrepare): Check for atomic here.
(qemuDomainSnapshotCreateDiskActive): Skip pausing the VM when
atomic supported.
(qemuDomainSnapshotIsAllowed): Use bool instead of int.
---

Patches 1/3 and 2/3 unchanged.
v2: consider interactions of atomic with non-disk-snapshots

  src/qemu/qemu_driver.c |   56 +++++++++++++++++++++++++++++++++--------------
  1 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 85f8cf7..9e62738 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10279,6 +10295,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
                                                 &vm, snap, flags)<  0)
              goto cleanup;
      } else if (!virDomainObjIsActive(vm)) {
+        if (flags&  VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) {
+            qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                            _("atomic snapshots of inactive domains not "
+                              "implemented yet"));
+            goto cleanup;

I wonder if we shouldn't add a error code for unimplemented functionality to differentiate it from unsupported configurations. (Well, but using a configuration that uses unimplemented functionality makes it an unsupported configuration basically, so I don't really mind)


+        }
          if (qemuDomainSnapshotCreateInactive(driver, vm, snap)<  0)
              goto cleanup;
      } else {

ACK,

Peter.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]