Change in vdsm[master]: BZ#836161 - Rewrite of deleteImage() complement.

ewarszaw at redhat.com ewarszaw at redhat.com
Tue Nov 13 09:07:59 UTC 2012


Hello Ayal Baron, Igor Lvovsky,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/9208

to review the following change.

Change subject: BZ#836161 - Rewrite of deleteImage() complement.
......................................................................

BZ#836161 - Rewrite of deleteImage() complement.

Volume operations should be done at the SD level to avoid
retrieving static data multiple times from disk.
Added lvm.lvDmDev() returning the dm-X for active LVs.
Use this to get active LV size without issue a lvm command.

Change-Id: I6c8c9baecee1ad7aa7789026e533aa254a738097
Complements: I304ff5cd70186ffc9789cd1ac9337efa6c5ff695
Signed-off-by: Eduardo <ewarszaw at redhat.com>
Reviewed-on: https://gerrit.eng.lab.tlv.redhat.com/3140
Reviewed-by: Ayal Baron <abaron at redhat.com>
Tested-by: Igor Lvovsky <ilvovsky at redhat.com>
---
M vdsm/storage/blockSD.py
M vdsm/storage/fileSD.py
M vdsm/storage/hsm.py
3 files changed, 63 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/08/9208/1

diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 26d4380..506059c 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -207,8 +207,10 @@
         lvm.changelv(sdUUID, volUUIDs, (("-a", "y"), ("--deltag", imgUUID),
                             ("--addtag", sd.REMOVED_IMAGE_PREFIX + imgUUID)))
     except se.StorageException as e:
-        log.debug("SD %s, Image %s pre zeroing ops failed", sdUUID, imgUUID,
-                    volUUIDs)
+        log.error("Can't activate or change LV tags in SD %s. "
+                  "failing Image %s pre zeroing operation for vols: %s. %s",
+                  sdUUID, imgUUID, volUUIDs, e)
+        raise
     # Following call to changelv is separate since setting rw permission on an
     # LV fails if the LV is already set to the same value, hence we would not
     # be able to differentiate between a real failure of deltag/addtag and one
@@ -915,11 +917,39 @@
                         if i.startswith(blockVolume.TAG_PREFIX_IMAGE) ]
         return images
 
+    def rmDCVolLinks(self, imgPath, volsImgs):
+        for vol in volsImgs:
+            lPath = os.path.join(imgPath, vol)
+            removedPaths = []
+            try:
+                os.unlink(lPath)
+            except OSError as e:
+                self.log.warning("Can't unlink %s. %s", lPath, e)
+            else:
+                removedPaths.append(lPath)
+        self.log.debug("removed: %s", removedPaths)
+        return tuple(removedPaths)
+
+    def rmDCImgDir(self, imgUUID, volsImgs):
+        imgPath = os.path.join(self.domaindir, sd.DOMAIN_IMAGES, imgUUID)
+        self.rmDCVolLinks(imgPath, volsImgs)
+        try:
+            os.rmdir(imgPath)
+        except OSError:
+            self.log.warning("Can't rmdir %s. %s", imgPath, exc_info = True)
+        else:
+            self.log.debug("removed image dir: %s", imgPath)
+        return imgPath
+
     def deleteImage(self, sdUUID, imgUUID, volsImgs):
-        return deleteVolumes(sdUUID, volsImgs)
+        toDel = tuple(vName for vName, v in volsImgs.iteritems()
+                                                    if v.imgs[0] == imgUUID)
+        deleteVolumes(sdUUID, toDel)
+        self.rmDCImgDir(imgUUID, volsImgs)
 
     def zeroImage(self, sdUUID, imgUUID, volsImgs):
-        return zeroImgVolumes(sdUUID, imgUUID, volsImgs)
+        zeroImgVolumes(sdUUID, imgUUID, volsImgs)
+        self.rmDCImgDir(imgUUID, volsImgs)
 
     def getAllVolumes(self):
         """
diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py
index 90ffecd..c4415da 100644
--- a/vdsm/storage/fileSD.py
+++ b/vdsm/storage/fileSD.py
@@ -496,9 +496,8 @@
         remove the remnants of the removed images (they could be left sometimes
         (on NFS mostly) due to lazy file removal
         """
-        removedPattern = os.path.join(
-                self.domaindir, sd.DOMAIN_IMAGES,
-                sd.REMOVED_IMAGE_PREFIX + '*')
+        removedPattern = os.path.join(self.domaindir, sd.DOMAIN_IMAGES,
+                                            sd.REMOVED_IMAGE_PREFIX + '*')
         removedImages = self.oop.glob.glob(removedPattern)
         self.log.debug("Removing remnants of deleted images %s" %
                        removedImages)
diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 4d696bf..e712102 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1308,6 +1308,8 @@
     def deleteImage(self, sdUUID, spUUID, imgUUID, postZero=False, force=False):
         """
         Delete Image folder with all volumes
+
+        force parameter is deprecated and not evaluated.
         """
         #vars.task.setDefaultException(se.ChangeMeError("%s" % args))
         self.getPool(spUUID) #Validates that the pool is connected. WHY?
@@ -1317,15 +1319,31 @@
         vars.task.getSharedLock(STORAGE, sdUUID)
         allVols = dom.getAllVolumes()
         volsByImg = sd.getVolsOfImage(allVols, imgUUID)
+        if not volsByImg:
+            self.log.error("Empty or not found image %s in SD %s. %s",
+                                                    imgUUID, sdUUID, allVols)
+            raise se.ImageDoesNotExistInSD(imgUUID, sdUUID)
+
         # on data domains, images should not be deleted if they are templates
         # being used by other images.
-        if not misc.parseBool(force) and not dom.isBackup() \
-                and not all(len(v.imgs) == 1 for v in volsByImg.itervalues()):
-                raise se.CannotDeleteSharedVolume("Cannot delete shared image"
-                    "%s. volImgs: %s"  % (imgUUID, volsByImg))
+        isTemplateWithChildren = False
+        for v in volsByImg.itervalues():
+            if len(v.imgs) > 1 and v.imgs[0] == imgUUID:
+                isTemplateWithChildren = True
+                break
+
+        if not isTemplateWithChildren:
+            needFake = False
+        elif dom.isBackup():
+            needFake = True
+        else:
+            raise se.CannotDeleteSharedVolume("Cannot delete shared image %s. "
+                                        "volImgs: %s"  % (imgUUID, volsByImg))
 
         # zeroImage will delete zeroed volumes at the end.
         if misc.parseBool(postZero):
+            # postZero implies block domain. Backup domains are always NFS
+            # hence no need to create fake template if postZero is true.
             self._spmSchedule(spUUID, "zeroImage_%s" % imgUUID, dom.zeroImage,
                               sdUUID, imgUUID, volsByImg.keys())
         else:
@@ -1337,6 +1355,11 @@
             # intended to quickly fix the integration issue with rhev-m. In 2.3
             # we should use the new resource system to synchronize the process
             # an eliminate all race conditions
+            if needFake:
+                img = image.Image(os.path.join(self.storage_repository, spUUID))
+                tName = volsByImg.iterkeys()[0]
+                tParams = dom.produceVolume(imgUUID, tName).getVolumeParams()
+                img.createFakeTemplate(sdUUID=sdUUID, volParams=tParams)
             self._spmSchedule(spUUID, "deleteImage_%s" % imgUUID, lambda : True)
 
 


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c8c9baecee1ad7aa7789026e533aa254a738097
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>


More information about the vdsm-patches mailing list