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