Change in vdsm[master]: freeze: Cleanup snapshot using freeze() and thaw()

nsoffer at redhat.com nsoffer at redhat.com
Tue Jul 7 23:16:59 UTC 2015


Nir Soffer has uploaded a new change for review.

Change subject: freeze: Cleanup snapshot using freeze() and thaw()
......................................................................

freeze: Cleanup snapshot using freeze() and thaw()

Snapshot code was written before libvirt.virDomain.fsFreeze() and
libvirt.virDomain.fsThaw() were available. We used the quiesce flag
(VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) to request freezing of guest
filesystems during the snapshot.

Because libvirt error reporting was not good enough, the code assumed
that *any* exception raised from snapshotCreateXML() means failure to
freeze the guest filesystems, and tried to take a snapshot again without
the quiesce flag. This error handling is evil and wrong, and requesting
snapshot twice after a failure sounds like a bad idea.

This patches cleans up the code using the new freeze() and thaw()
methods. We freeze() the guest before the snapshot, and thaw()
afterwards, invoking snapshot exactly once.

We handle only libvirtError around snapshotCreateXML(). Any other error
is not expected and will fail loudly.

When taking a memory snapshot, we are not freezing the guest, because
qemu is pausing the vm in this case. We keep this behavior for now.

Change-Id: Ice74b5619246c979f24909f4ffc3688b3adef6cb
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 16 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/98/43298/1

diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 1ec03d4..3263733 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -3092,7 +3092,9 @@
             snap.appendChild(_memorySnapshot(memoryVolPath))
         else:
             snapFlags |= libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY
-            snapFlags |= libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE
+
+        # When creating memory snapshot libvirt will pause the vm
+        should_freeze = not memoryParams
 
         snapxml = snap.toprettyxml()
         # TODO: this is debug information. For 3.6.x we still need to
@@ -3107,25 +3109,19 @@
         self.stopDisksStatsCollection()
 
         try:
+            if should_freeze:
+                freezed = self.freeze()
             try:
                 self._dom.snapshotCreateXML(snapxml, snapFlags)
-            except Exception as e:
-                # Trying again without VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE.
-                # At the moment libvirt is returning two generic errors
-                # (INTERNAL_ERROR, ARGUMENT_UNSUPPORTED) which are too broad
-                # to be caught (BZ#845635).
-                snapFlags &= (~libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE)
-                # Here we don't need a full stacktrace (exc_info) but it's
-                # still interesting knowing what was the error
-                self.log.debug("Snapshot failed using the quiesce flag, "
-                               "trying again without it (%s)", e)
-                try:
-                    self._dom.snapshotCreateXML(snapxml, snapFlags)
-                except Exception as e:
-                    self.log.exception("Unable to take snapshot")
-                    if memoryParams:
-                        self.cif.teardownVolumePath(memoryVol)
-                    return errCode['snapshotErr']
+            except libvirt.libvirtError:
+                self.log.exception("Unable to take snapshot")
+                return errCode['snapshotErr']
+            finally:
+                # Must always thaw, even if freeze failed; in case the guest
+                # did freeze the filesystems, but failed to reply in time.
+                # Libvirt is using same logic (see src/qemu/qemu_driver.c).
+                if should_freeze:
+                    self.thaw()
 
             # We are padding the memory volume with block size of zeroes
             # because qemu-img truncates files such that their size is
@@ -3151,9 +3147,8 @@
 
         # Returning quiesce to notify the manager whether the guest agent
         # froze and flushed the filesystems or not.
-        return {'status': doneCode, 'quiesce':
-                (snapFlags & libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE
-                    == libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE)}
+        return {'status': doneCode,
+                'quiesce': should_freeze and freezed["status"]["code"] == 0}
 
     def diskReplicateStart(self, srcDisk, dstDisk):
         try:


-- 
To view, visit https://gerrit.ovirt.org/43298
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ice74b5619246c979f24909f4ffc3688b3adef6cb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>


More information about the vdsm-patches mailing list