Saggi Mizrahi has uploaded a new change for review.

Change subject: Make sure teardown() is called in case of errors
......................................................................

Make sure teardown() is called in case of errors

This also adds scopedPrepare. It should be use unless there is absolutely
no other choice!

Bug-Url: https://bugzilla.redhat.com/815359
Change-Id: I7cefbcd72434aa4d766b50b3d05aa70ba8cd910c
Signed-off-by: Saggi Mizrahi <[email protected]>
---
M vdsm/storage/image.py
M vdsm/storage/volume.py
2 files changed, 32 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/8667/1

diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py
index 9a22863..fc48894 100644
--- a/vdsm/storage/image.py
+++ b/vdsm/storage/image.py
@@ -962,32 +962,30 @@
         # Step 4: Rename successor_MERGE to successor
         # Step 5: Unsafely rebase successor's children on top of temporary 
volume
         srcVol = chain[-1]
-        srcVol.prepare(rw=True, chainrw=True, setrw=True)
-        # Find out successor's children list
-        chList = srcVolParams['children']
-        # Step 1: Create an empty volume named sucessor_MERGE with destination 
volume's parent parameters
-        newUUID = srcVol.volUUID + "_MERGE"
-        sdDom.createVolume(imgUUID=srcVolParams['imgUUID'],
-                                         size=volParams['size'], 
volFormat=volParams['volFormat'],
-                                         preallocate=volParams['prealloc'], 
diskType=volParams['disktype'],
-                                         volUUID=newUUID, 
desc=srcVolParams['descr'],
-                                         srcImgUUID=volume.BLANK_UUID, 
srcVolUUID=volume.BLANK_UUID)
+        with srcVol.scopedPrepare(rw=True, chainrw=True, setrw=True):
+            # Find out successor's children list
+            chList = srcVolParams['children']
+            # Step 1: Create an empty volume named sucessor_MERGE with 
destination volume's parent parameters
+            newUUID = srcVol.volUUID + "_MERGE"
+            sdDom.createVolume(imgUUID=srcVolParams['imgUUID'],
+                                             size=volParams['size'], 
volFormat=volParams['volFormat'],
+                                             
preallocate=volParams['prealloc'], diskType=volParams['disktype'],
+                                             volUUID=newUUID, 
desc=srcVolParams['descr'],
+                                             srcImgUUID=volume.BLANK_UUID, 
srcVolUUID=volume.BLANK_UUID)
 
-        newVol = sdDom.produceVolume(imgUUID=srcVolParams['imgUUID'], 
volUUID=newUUID)
-        newVol.prepare(rw=True, justme=True, setrw=True)
+            newVol = sdDom.produceVolume(imgUUID=srcVolParams['imgUUID'], 
volUUID=newUUID)
+            with newVol.scopedPrepare(rw=True, justme=True, setrw=True):
 
-        # Step 2: Convert successor to new volume
-        #   qemu-img convert -f qcow2 successor -O raw newUUID
-        (rc, out, err) = volume.qemuConvert(srcVolParams['path'], 
newVol.getVolumePath(),
-            srcVolParams['volFormat'], volParams['volFormat'], 
vars.task.aborting,
-            size=volParams['apparentsize'], dstvolType=newVol.getType())
-        if rc:
-            self.log.error("qemu-img convert failed: rc=%s, out=%s, err=%s",
-                            rc, out, err)
-            raise se.MergeSnapshotsError(newUUID)
+                # Step 2: Convert successor to new volume
+                #   qemu-img convert -f qcow2 successor -O raw newUUID
+                (rc, out, err) = volume.qemuConvert(srcVolParams['path'], 
newVol.getVolumePath(),
+                    srcVolParams['volFormat'], volParams['volFormat'], 
vars.task.aborting,
+                    size=volParams['apparentsize'], 
dstvolType=newVol.getType())
+                if rc:
+                    self.log.error("qemu-img convert failed: rc=%s, out=%s, 
err=%s",
+                                    rc, out, err)
+                    raise se.MergeSnapshotsError(newUUID)
 
-        newVol.teardown(sdUUID=newVol.sdUUID, volUUID=newVol.volUUID)
-        srcVol.teardown(sdUUID=srcVol.sdUUID, volUUID=srcVol.volUUID)
         if chList:
             newVol.setInternal()
 
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index b9eb356..7377964 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -22,6 +22,7 @@
 import logging
 import time
 import signal
+from contextlib import contextmanager
 
 import image
 from vdsm import constants
@@ -659,6 +660,16 @@
 
         return self.isLeaf()
 
+    @contextmanager
+    def scopedPrepare(self, rw=True, justme=False, chainrw=False, setrw=False,
+                 force=False):
+        self.prepare(rw=True, justme=False, chainrw=False, setrw=False,
+                     force=False)
+        try:
+            yield self
+        finally:
+            self.teardown(self.sdUUID, self.volUUID, justme)
+
     def prepare(self, rw=True, justme=False,
                 chainrw=False, setrw=False, force=False):
         """


--
To view, visit http://gerrit.ovirt.org/8667
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7cefbcd72434aa4d766b50b3d05aa70ba8cd910c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to