vdsm-patches November 2012

vdsm-patches@lists.fedorahosted.org

50 participants
416 discussions

Change in vdsm[master]: tests: sort the tests and remove duplicates
by Federico Simoncelli 13 Nov '12

13 Nov '12

Federico Simoncelli has uploaded a new change for review. Change subject: tests: sort the tests and remove duplicates ...................................................................... tests: sort the tests and remove duplicates Change-Id: I1ef4d7317b5e00512f078792aecd4a1c55eac302 Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M tests/Makefile.am 1 file changed, 1 insertion(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/18/9218/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index dcb0b2e..9683e7b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -47,13 +47,11 @@ resourceManagerTests.py \ restData.py \ restTests.py \ - superVdsmTests.py \ storageMailboxTests.py \ + superVdsmTests.py \ tcTests.py \ vdsClientTests.py \ volumeTests.py \ - remoteFileHandlerTests.py \ - resourceManagerTests.py \ zombieReaperTests.py \ $(NULL) -- To view, visit http://gerrit.ovirt.org/9218 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1ef4d7317b5e00512f078792aecd4a1c55eac302 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

3 5

Change in vdsm[master]: tests: add FileVolumeGetVSizeTest
by Federico Simoncelli 13 Nov '12

13 Nov '12

Federico Simoncelli has uploaded a new change for review. Change subject: tests: add FileVolumeGetVSizeTest ...................................................................... tests: add FileVolumeGetVSizeTest This patch adds a test for the getVSize method for file domains. Change-Id: Iac8b28f9fa1268f83c658bde1d0302e013584826 Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M tests/Makefile.am A tests/volumeTests.py 2 files changed, 70 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/03/9203/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index f53470f..1753c75 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -49,6 +49,7 @@ vdsClientTests.py \ remoteFileHandlerTests.py \ resourceManagerTests.py \ + volumeTests.py \ $(NULL) dist_noinst_DATA = \ diff --git a/tests/volumeTests.py b/tests/volumeTests.py new file mode 100644 index 0000000..5dfd8e0 --- /dev/null +++ b/tests/volumeTests.py @@ -0,0 +1,69 @@ +# Copyright 2012 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +import os +import shutil +import tempfile +import uuid + +from testrunner import VdsmTestCase as TestCaseBase + +from storage import outOfProcess +from storage.fileVolume import FileVolume + + +class FileDomainMockObject(object): + def __init__(self, repoPath, sdUUID): + self.repoPath = repoPath + self.sdUUID = sdUUID + + def _getRepoPath(self): + return self.repoPath + + @property + def oop(self): + return outOfProcess.getProcessPool(self.sdUUID) + + +class FileVolumeGetVSizeTest(TestCaseBase): + VOLSIZE = 1024 + SDBLKSZ = 512 + + def setUp(self): + self.repoPath = tempfile.mkdtemp() + + self.sdUUID = str(uuid.uuid4()) + self.imgUUID = str(uuid.uuid4()) + self.volUUID = str(uuid.uuid4()) + + imgPath = os.path.join(self.repoPath, self.sdUUID, "images", + self.imgUUID) + volPath = os.path.join(imgPath, self.volUUID) + + os.makedirs(imgPath) + open(volPath, "w").truncate(self.VOLSIZE * self.SDBLKSZ) + self.sdobj = FileDomainMockObject(self.repoPath, self.sdUUID) + + def tearDown(self): + shutil.rmtree(self.repoPath) + + def test(self): + volSize = FileVolume.getVSize(self.sdobj, self.imgUUID, + self.volUUID) + assert volSize == self.VOLSIZE -- To view, visit http://gerrit.ovirt.org/9203 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iac8b28f9fa1268f83c658bde1d0302e013584826 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

3 3

Change in vdsm[master]: tests: drop the sanlock-python dependency
by Federico Simoncelli 13 Nov '12

13 Nov '12

Federico Simoncelli has uploaded a new change for review. Change subject: tests: drop the sanlock-python dependency ...................................................................... tests: drop the sanlock-python dependency Testing vdsm shouldn't require sanlock to be installed and running. Change-Id: I88fac9a1f538c92f696b8f097b56bf231dae9b9f Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M tests/testrunner.py M vdsm.spec.in 2 files changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/9202/1 diff --git a/tests/testrunner.py b/tests/testrunner.py index 9851f3e..e068426 100644 --- a/tests/testrunner.py +++ b/tests/testrunner.py @@ -39,6 +39,9 @@ PERMUTATION_ATTR = "_permutations_" +# At the moment of this writing the tests don't need a mock object for sanlock +sys.modules.update({'sanlock': object()}) + def _getPermutation(f, args): @wraps(f) diff --git a/vdsm.spec.in b/vdsm.spec.in index a7614dc..782d91c 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -40,7 +40,6 @@ # BuildRequires needed by the tests during the build BuildRequires: python-ethtool BuildRequires: libvirt-python -BuildRequires: sanlock-python >= 2.3 BuildRequires: genisoimage # Autotools BuildRequires -- To view, visit http://gerrit.ovirt.org/9202 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I88fac9a1f538c92f696b8f097b56bf231dae9b9f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

2 2

Change in vdsm[master]: build on el6: import OrderedDict from elsewhere
by Dan Kenigsberg 13 Nov '12

13 Nov '12

Dan Kenigsberg has uploaded a new change for review. Change subject: build on el6: import OrderedDict from elsewhere ...................................................................... build on el6: import OrderedDict from elsewhere Commit 1db772c71e has introduced usage of collections.OrderedDict which is not part of el6's Python 2.6. This patch imports Raymond Hettinger's implementation if OrderedDict is missing in the default location. Change-Id: I92079c2639d892f283c771576ca25e333d0b1936 Signed-off-by: Dan Kenigsberg <danken(a)redhat.com> --- M vdsm.spec.in M vdsm_api/vdsmapi.py 2 files changed, 11 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/12/9212/1 diff --git a/vdsm.spec.in b/vdsm.spec.in index d268948..88e3a1f 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -42,6 +42,9 @@ BuildRequires: libvirt-python BuildRequires: sanlock-python >= 2.3 BuildRequires: genisoimage +%if 0%{?rhel} +BuildRequires: python-ordereddict +%endif # Autotools BuildRequires %if 0%{?enable_autotools} @@ -162,6 +165,9 @@ BuildArch: noarch Requires: %{name}-python = %{version}-%{release} +%if 0%{?rhel} +Requires: python-ordereddict +%endif %description jsonrpc A Json-based RPC interface that serves as the protocol for libvdsm. diff --git a/vdsm_api/vdsmapi.py b/vdsm_api/vdsmapi.py index a91b233..c68daa1 100644 --- a/vdsm_api/vdsmapi.py +++ b/vdsm_api/vdsmapi.py @@ -24,7 +24,11 @@ # Refer to the README and COPYING files for full details of the license # -from collections import OrderedDict +try: + from collections import OrderedDict + OrderedDict # make pyflakes happy +except ImportError: + from ordereddict import OrderedDict def tokenize(data): -- To view, visit http://gerrit.ovirt.org/9212 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I92079c2639d892f283c771576ca25e333d0b1936 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>

2 3

Change in vdsm[master]: tests: override the P_VDSM definition in local tests
by Federico Simoncelli 13 Nov '12

13 Nov '12

Federico Simoncelli has uploaded a new change for review. Change subject: tests: override the P_VDSM definition in local tests ...................................................................... tests: override the P_VDSM definition in local tests The P_VDSM constant is used to configure the PYTHONPATH variable for the out of process handlers and it should be overridden during the local tests (vdsm is not installed). Change-Id: I06a11ab32b0adc635fe236c6033b6b3d2257b235 Signed-off-by: Federico Simoncelli <fsimonce(a)redhat.com> --- M tests/testrunner.py 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/17/9217/1 diff --git a/tests/testrunner.py b/tests/testrunner.py index ff09862..36d76c6 100644 --- a/tests/testrunner.py +++ b/tests/testrunner.py @@ -240,6 +240,8 @@ setattr(mod, name, sub) sys.modules['vdsm.%s' % name] = getattr(mod, name) + sys.modules['vdsm.constants'].P_VDSM = "../" + def findRemove(listR, value): """used to test if a value exist, if it is, return true and remove it.""" -- To view, visit http://gerrit.ovirt.org/9217 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I06a11ab32b0adc635fe236c6033b6b3d2257b235 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>

2 2

Change in vdsm[master]: pep8: fix hsm.py
by Dan Kenigsberg 13 Nov '12

13 Nov '12

Dan Kenigsberg has uploaded a new change for review. Change subject: pep8: fix hsm.py ...................................................................... pep8: fix hsm.py Errors introduced by commit b83a0c157d5c and overlooked by my rebase of commit af99446a7cd35. Change-Id: Ib31cea93e3488ea04b3fac9ddc2cc31b4454f401 Signed-off-by: Dan Kenigsberg <danken(a)redhat.com> --- M vdsm/storage/hsm.py 1 file changed, 5 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/9213/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 8b269c3..46d1605 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -1438,7 +1438,8 @@ needFake = True else: raise se.CannotDeleteSharedVolume("Cannot delete shared image %s. " - "volImgs: %s" % (imgUUID, volsByImg)) + "volImgs: %s" % (imgUUID, + volsByImg)) # zeroImage will delete zeroed volumes at the end. if misc.parseBool(postZero): @@ -1456,12 +1457,12 @@ # 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)) + 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) - + self._spmSchedule(spUUID, "deleteImage_%s" % imgUUID, lambda: True) def validateImageMove(self, srcDom, dstDom, imgUUID): """ -- To view, visit http://gerrit.ovirt.org/9213 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib31cea93e3488ea04b3fac9ddc2cc31b4454f401 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>

2 4

Change in vdsm[master]: fix in import error
by ybronhei＠redhat.com 13 Nov '12

13 Nov '12

Yaniv Bronhaim has uploaded a new change for review. Change subject: fix in import error ...................................................................... fix in import error Signed-off-by: Yaniv Bronhaim <ybronhei(a)redhat.com> Change-Id: I6bb9823316dd051bbc4af80a4f30b5b6c60c45d3 --- M vdsm/supervdsmServer.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/14/9214/1 diff --git a/vdsm/supervdsmServer.py b/vdsm/supervdsmServer.py index 71309ff..34fefdb 100755 --- a/vdsm/supervdsmServer.py +++ b/vdsm/supervdsmServer.py @@ -29,7 +29,7 @@ import signal from multiprocessing import Pipe, Process import storage.misc as misc -import utils +from vdsm import utils try: from gluster import cli as gcli -- To view, visit http://gerrit.ovirt.org/9214 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6bb9823316dd051bbc4af80a4f30b5b6c60c45d3 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yaniv Bronhaim <ybronhei(a)redhat.com>

2 3

Change in vdsm[master]: modify hsm.py for PEP8 compliance
by zhshzhou＠linux.vnet.ibm.com 13 Nov '12

13 Nov '12

Zhou Zheng Sheng has uploaded a new change for review. Change subject: modify hsm.py for PEP8 compliance ...................................................................... modify hsm.py for PEP8 compliance clean hsm.py code style problems, and add it to PEP 8 whitelist in Makefile.am Change-Id: Ia9b2f8b9e83db6f52678b4e0865668a79bcf53f5 Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com> --- M Makefile.am M vdsm/storage/hsm.py 2 files changed, 546 insertions(+), 366 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/4530/1 -- To view, visit http://gerrit.ovirt.org/4530 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia9b2f8b9e83db6f52678b4e0865668a79bcf53f5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com>

7 26

Change in vdsm[master]: BZ#836161 - Changed code obsoleted by deleteImage() rewrite.
by ewarszaw＠redhat.com 13 Nov '12

13 Nov '12

Hello Ayal Baron, I'd like you to do a code review. Please visit http://gerrit.ovirt.org/9209 to review the following change. Change subject: BZ#836161 - Changed code obsoleted by deleteImage() rewrite. ...................................................................... BZ#836161 - Changed code obsoleted by deleteImage() rewrite. Change image.move and image.multipleMove to call the new deleteImage implementation instead of image.delete Change-Id: Icee221a3e8282194971afefd21540dc759a02aab Signed-off-by: Eduardo <ewarszaw(a)redhat.com> Reviewed-on: https://gerrit.eng.lab.tlv.redhat.com/3141 Reviewed-by: Ayal Baron <abaron(a)redhat.com> Tested-by: Daniel Paikov <dpaikov(a)redhat.com> --- M vdsm/storage/image.py M vdsm/storage/sp.py 2 files changed, 30 insertions(+), 83 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/9209/1 diff --git a/vdsm/storage/image.py b/vdsm/storage/image.py index 4cd9785..73274fe 100644 --- a/vdsm/storage/image.py +++ b/vdsm/storage/image.py @@ -67,6 +67,19 @@ TEMPORARY_VOLUME_SIZE = 20480 # in sectors (10M) +def _deleteImage(dom, imgUUID, postZero): + """This ancillary function will be removed. + + Replaces Image.delete() in Image.[copy(), move(),multimove()]. + """ + allVols = dom.getAllVolumes() + imgVols = sd.getVolsOfImage(allVols, imgUUID) + if postZero: + dom.zeroImage(dom.sdUUID, imgUUID, imgVols) + else: + dom.deleteImage(dom.sdUUID, imgUUID, imgVols) + + class Image: """ Actually represents a whole virtual disk. Consist from chain of volumes. @@ -612,7 +625,7 @@ # in destination domain, if we got the overwrite command if force: self.log.info("delete image %s on domain %s before overwriting", imgUUID, destDom.sdUUID) - self.delete(destDom.sdUUID, imgUUID, postZero, force=True) + _deleteImage(destDom, imgUUID, postZero) chains = self._createTargetImage(destDom, srcSdUUID, imgUUID) self._interImagesCopy(destDom, srcSdUUID, imgUUID, chains) @@ -627,7 +640,12 @@ vars.task.clearRecoveries() # If it's 'move' operation, we should delete src image after copying if op == MOVE_OP: - self.delete(srcSdUUID, imgUUID, postZero, force=True) + try: + dom = sdCache.produce(srcSdUUID) + _deleteImage(dom, imgUUID, postZero) + except se.StorageException: + self.log.warning("Failed to remove img: %s from srcDom %s: " + "after it was copied to: %s", imgUUID, srcSdUUID, dstSdUUID) self.log.info("%s task on image %s was successfully finished", OP_TYPES[op], imgUUID) return True @@ -676,9 +694,11 @@ """ for imgUUID in imgList: try: - self.delete(sdUUID, imgUUID, postZero, force=True) - except Exception: - pass + dom = sdCache.produce(sdUUID) + _deleteImage(dom, imgUUID, postZero) + except se.StorageException: + self.log.warning("Delete image failed for image: %s in SD: %s", + imgUUID, sdUUID, exc_info=True) def multiMove(self, srcSdUUID, dstSdUUID, imgDict, vmUUID, force): """ @@ -706,9 +726,11 @@ # Remove images from source domain only after successfull copying of all images to the destination domain for (imgUUID, postZero) in imgDict.iteritems(): try: - self.delete(srcSdUUID, imgUUID, postZero, force=True) - except Exception: - pass + dom = sdCache.produce(srcSdUUID) + _deleteImage(dom, imgUUID, postZero) + except se.StorageException: + self.log.warning("Delete image failed for image %s in SD: %s", + imgUUID, dom.sdUUID, exc_info=True) def __cleanupCopy(self, srcVol, dstVol): """ diff --git a/vdsm/storage/sp.py b/vdsm/storage/sp.py index c9489b8..fdd146c 100644 --- a/vdsm/storage/sp.py +++ b/vdsm/storage/sp.py @@ -1823,73 +1823,6 @@ image.Image(repoPath).multiMove(srcDomUUID, dstDomUUID, imgDict, vmUUID, force) - def deleteImage(self, sdUUID, imgUUID, postZero, force): - """ - Deletes an Image folder with all its volumes. - - This function assumes that imgUUID is locked. - In addition to removing image, this function also does the following: - If removing a template from a backup SD which has dependent images: - - creates a fake template. - If removing the last image which depends on a fake template: - - removes the fake template as well - :param sdUUID: The UUID of the storage domain that contains the images. - :type sdUUID: UUID - :param imgUUID: The UUID of the image you want to delete. - :type imgUUID: UUID - :param postZero: bool - :param force: Should the operation be forced. - :type force: bool - """ - # TODO: This function works on domains. No relation with pools. - # Therefore move this to the relevant *sd module - repoPath = os.path.join(self.storage_repository, self.spUUID) - img = image.Image(repoPath) - dom = sdCache.produce(sdUUID) - allVols = dom.getAllVolumes() - # Filter volumes related to this image - imgsByVol = sd.getVolsOfImage(allVols, imgUUID) - if all(len(v.imgs) == 1 for k, v in imgsByVol.iteritems()): - # This is a self contained regular image, i.e. it's either an image - # which is not based on a template or a template which has no - # derived images, e.g. not derived from a template - img.delete(sdUUID=sdUUID, imgUUID=imgUUID, postZero=postZero, force=force) - else: - # This is either a template with derived images or a derived image - # so needs further scrutiny - ts = tuple((volName, vol.imgs) for volName, vol in - imgsByVol.iteritems() if len(vol.imgs) > 1) - if len(ts) != 1: - raise se.ImageValidationError("Image points to multiple" - "templates %s in %s from %s" % - (ts, imgsByVol, allVols)) - # TODO: Lock the template, reload allVols. - # template = ts[0] = [(tName, tImgs)] - tName, tImgs = ts[0] - # getAllVolumes makes the template self img the 1st one in tImgs - templateImage = tImgs[0] - numOfDependentImgs = len(tImgs) - 1 - if templateImage != imgUUID: - # Removing an image based on a template - img.delete(sdUUID=sdUUID, imgUUID=imgUUID, postZero=postZero, force=force) - if numOfDependentImgs == 1 and dom.produceVolume(templateImage, tName).isFake(): - # Remove the fake template since last consumer was removed - img.delete(sdUUID=sdUUID, imgUUID=templateImage, postZero=False, force=True) - - # Removing a template with dependencies - elif force: - img.delete(sdUUID=sdUUID, imgUUID=templateImage, postZero=postZero, - force=force) - elif not dom.isBackup(): - raise se.ImagesActionError("Can't remove template with children %s" % - allVols) - else: - # Removing a template with dependencies in backup domain - # A fake template will be created - img.delete(sdUUID=sdUUID, imgUUID=imgUUID, postZero=postZero, force=True) - tParams = dom.produceVolume(imgUUID, tName).getVolumeParams() - img.createFakeTemplate(sdUUID=sdUUID, volParams=tParams) - def mergeSnapshots(self, sdUUID, vmUUID, imgUUID, ancestor, successor, postZero): """ Merges the source volume to the destination volume. @@ -2065,14 +1998,6 @@ vol.teardown(sdUUID, volUUID) except: self.log.warning("SP %s SD %s img %s Vol %s - teardown failed") - - def preDeleteRename(self, sdUUID, imgUUID): - repoPath = os.path.join(self.storage_repository, self.spUUID) - return image.Image(repoPath).preDeleteRename(sdUUID, imgUUID) - - def validateDelete(self, sdUUID, imgUUID): - repoPath = os.path.join(self.storage_repository, self.spUUID) - image.Image(repoPath).validateDelete(sdUUID, imgUUID) def validateVolumeChain(self, sdUUID, imgUUID): repoPath = os.path.join(self.storage_repository, self.spUUID) -- To view, visit http://gerrit.ovirt.org/9209 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icee221a3e8282194971afefd21540dc759a02aab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>

2 3

Change in vdsm[master]: BZ#836161 - Rewrite of deleteImage() complement.
by ewarszaw＠redhat.com 13 Nov '12

13 Nov '12

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(a)redhat.com> Reviewed-on: https://gerrit.eng.lab.tlv.redhat.com/3140 Reviewed-by: Ayal Baron <abaron(a)redhat.com> Tested-by: Igor Lvovsky <ilvovsky(a)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(a)redhat.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Igor Lvovsky <ilvovsky(a)redhat.com>

2 3

← Newer
1
...
34
35
36
37
38
39
40
41
42
Older →

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches November 2012