Change in vdsm[master]: VolumeMetadata: Move volumePath and validation

alitke at redhat.com alitke at redhat.com
Fri Sep 18 20:46:24 UTC 2015


Adam Litke has uploaded a new change for review.

Change subject: VolumeMetadata: Move volumePath and validation
......................................................................

VolumeMetadata: Move volumePath and validation

Change-Id: I5b38519df298d6e7b9a4bba2642d615976e0ea04
Signed-off-by: Adam Litke <alitke at redhat.com>
---
M tests/sdm_indirection_tests.py
M vdsm/storage/blockVolume.py
M vdsm/storage/fileVolume.py
M vdsm/storage/volume.py
4 files changed, 99 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/83/46383/1

diff --git a/tests/sdm_indirection_tests.py b/tests/sdm_indirection_tests.py
index 6ce9df5..a27ff5c 100644
--- a/tests/sdm_indirection_tests.py
+++ b/tests/sdm_indirection_tests.py
@@ -500,10 +500,17 @@
         ['volUUID', '6aab5eb4-2a8b-4cb7-a0b7-bc6f61de3e18'],
         ['repoPath', '/rhev/data-center'],
         ['imagePath', '/a/b'],
+        ['volumePath', '/a/b/c'],
         ['voltype', None],
         ])
     def test_property(self, prop, val):
         self.assertEqual(getattr(self.source_inst, prop), val)
+
+    @permutations([
+        ['getVolumePath', 0],
+        ])
+    def test_common_functions(self, fn, nargs):
+        self.checker.check_call(fn, nargs)
 
 
 class BlockVolumeTests(VolumeTestMixin, VdsmTestCase):
@@ -525,3 +532,9 @@
         ])
     def test_file_property(self, prop, val):
         self.assertEqual(getattr(self.source_inst, prop), val)
+
+    @permutations([
+        ['_getMetaVolumePath', 1],
+        ])
+    def test_functions(self, fn, nargs):
+        self.checker.check_call(fn, nargs)
diff --git a/vdsm/storage/blockVolume.py b/vdsm/storage/blockVolume.py
index 2955f28..fa26270 100644
--- a/vdsm/storage/blockVolume.py
+++ b/vdsm/storage/blockVolume.py
@@ -88,6 +88,21 @@
                 raise se.ImagePathError(imageDir)
         self._imagePath = imageDir
 
+    def validateVolumePath(self):
+        """
+        Block SD supports lazy volume link creation. Note that the volume can
+        be still inactive.
+        An explicit prepare is required to validate that the volume is active.
+        """
+        if not self._imagePath:
+            self.validateImagePath()
+        volPath = os.path.join(self._imagePath, self.volUUID)
+        if not os.path.lexists(volPath):
+            srcPath = lvm.lvPath(self.sdUUID, self.volUUID)
+            self.log.debug("Creating symlink from %s to %s", srcPath, volPath)
+            os.symlink(srcPath, volPath)
+        self._volumePath = volPath
+
 
 class BlockVolume(volume.Volume):
     """ Actually represents a single volume (i.e. part of virtual disk).
@@ -363,7 +378,7 @@
 
         lvm.renameLV(self.sdUUID, self.volUUID, newUUID)
         self.md.volUUID = newUUID
-        self.volumePath = os.path.join(self.imagePath, newUUID)
+        self.md.volumePath = os.path.join(self.imagePath, newUUID)
 
     def getDevPath(self):
         """
@@ -434,21 +449,6 @@
 
             if pvolUUID != volume.BLANK_UUID:
                 cls.teardown(sdUUID=sdUUID, volUUID=pvolUUID, justme=False)
-
-    def validateVolumePath(self):
-        """
-        Block SD supports lazy volume link creation. Note that the volume can
-        be still inactive.
-        An explicit prepare is required to validate that the volume is active.
-        """
-        if not self.imagePath:
-            self.md.validateImagePath()
-        volPath = os.path.join(self.imagePath, self.volUUID)
-        if not os.path.lexists(volPath):
-            srcPath = lvm.lvPath(self.sdUUID, self.volUUID)
-            self.log.debug("Creating symlink from %s to %s", srcPath, volPath)
-            os.symlink(srcPath, volPath)
-        self.volumePath = volPath
 
     def getVolumeTag(self, tagPrefix):
         return _getVolumeTag(self.sdUUID, self.volUUID, tagPrefix)
diff --git a/vdsm/storage/fileVolume.py b/vdsm/storage/fileVolume.py
index cfe40d0..94a64db 100644
--- a/vdsm/storage/fileVolume.py
+++ b/vdsm/storage/fileVolume.py
@@ -30,6 +30,7 @@
 import volume
 import image
 import sd
+import fileSD
 import misc
 from misc import deprecated
 import task
@@ -74,6 +75,48 @@
             raise se.ImagePathError(imageDir)
         self._imagePath = imageDir
 
+    @classmethod
+    def _metaVolumePath(cls, volPath):
+        if volPath:
+            return volPath + META_FILEEXT
+        else:
+            return None
+
+    def _getMetaVolumePath(self, vol_path=None):
+        """
+        Get the volume metadata file/link path
+        """
+        if not vol_path:
+            vol_path = self.getVolumePath()
+        return self._metaVolumePath(vol_path)
+
+    def validateMetaVolumePath(self):
+        """
+        In file volume repositories,
+        the volume metadata must exists after the image/volume is created.
+        """
+        metaVolumePath = self._getMetaVolumePath()
+        if not self.oop.fileUtils.pathExists(metaVolumePath):
+            raise se.VolumeDoesNotExist(self.volUUID)
+
+    def validateVolumePath(self):
+        """
+        In file volume repositories,
+        the volume file and the volume md must exists after
+        the image/volume is created.
+        """
+        self.log.debug("validate path for %s" % self.volUUID)
+        if not self.imagePath:
+            self.validateImagePath()
+        volPath = os.path.join(self.imagePath, self.volUUID)
+        if not self.oop.fileUtils.pathExists(volPath):
+            raise se.VolumeDoesNotExist(self.volUUID)
+
+        self._volumePath = volPath
+        domainPath = os.path.join(self.repoPath, self.sdUUID)
+        if not fileSD.FileStorageDomainManifest(domainPath).isISO():
+            self.validateMetaVolumePath()
+
 
 class FileVolume(volume.Volume):
     """ Actually represents a single volume (i.e. part of virtual disk).
@@ -109,7 +152,7 @@
             raise TypeError("halfbakedVolumeRollback takes 1 or 3 "
                             "arguments (%d given)" % len(args))
 
-        metaVolPath = cls.__metaVolumePath(volPath)
+        metaVolPath = cls.MetadataClass._metaVolumePath(volPath)
         cls.log.info("Halfbaked volume rollback for volPath=%s", volPath)
 
         if oop.getProcessPool(sdUUID).fileUtils.pathExists(volPath) and not \
@@ -119,7 +162,7 @@
     @classmethod
     def createVolumeMetadataRollback(cls, taskObj, volPath):
         cls.log.info("createVolumeMetadataRollback: volPath=%s" % (volPath))
-        metaPath = cls.__metaVolumePath(volPath)
+        metaPath = cls.MetadataClass._metaVolumePath(volPath)
         sdUUID = getDomUuidFromVolumePath(volPath)
         if oop.getProcessPool(sdUUID).os.path.lexists(metaPath):
             oop.getProcessPool(sdUUID).os.unlink(metaPath)
@@ -264,7 +307,7 @@
         cls.log.info("Volume rollback for volPath=%s", volPath)
         procPool = oop.getProcessPool(getDomUuidFromVolumePath(volPath))
         procPool.utils.rmFile(volPath)
-        procPool.utils.rmFile(cls.__metaVolumePath(volPath))
+        procPool.utils.rmFile(cls.MetadataClass._metaVolumePath(volPath))
         procPool.utils.rmFile(cls.__leaseVolumePath(volPath))
 
     @deprecated  # valid only for domain version < 3, see volume.setrw
@@ -339,7 +382,7 @@
     @classmethod
     def __putMetadata(cls, metaId, meta):
         volPath, = metaId
-        metaPath = cls.__metaVolumePath(volPath)
+        metaPath = cls.MetadataClass._metaVolumePath(volPath)
 
         with open(metaPath + ".new", "w") as f:
             for key, value in meta.iteritems():
@@ -501,14 +544,7 @@
             if e.errno != os.errno.ENOENT:
                 raise
         self.md.volUUID = newUUID
-        self.volumePath = volPath
-
-    @classmethod
-    def __metaVolumePath(cls, volPath):
-        if volPath:
-            return volPath + META_FILEEXT
-        else:
-            return None
+        self.md.volumePath = volPath
 
     @classmethod
     def __leaseVolumePath(cls, vol_path):
@@ -518,12 +554,7 @@
             return None
 
     def _getMetaVolumePath(self, vol_path=None):
-        """
-        Get the volume metadata file/link path
-        """
-        if not vol_path:
-            vol_path = self.getVolumePath()
-        return self.__metaVolumePath(vol_path)
+        return self.md._getMetaVolumePath(vol_path)
 
     def _getLeaseVolumePath(self, vol_path=None):
         """
@@ -532,32 +563,6 @@
         if not vol_path:
             vol_path = self.getVolumePath()
         return self.__leaseVolumePath(vol_path)
-
-    def validateVolumePath(self):
-        """
-        In file volume repositories,
-        the volume file and the volume md must exists after
-        the image/volume is created.
-        """
-        self.log.debug("validate path for %s" % self.volUUID)
-        if not self.imagePath:
-            self.md.validateImagePath()
-        volPath = os.path.join(self.imagePath, self.volUUID)
-        if not self.oop.fileUtils.pathExists(volPath):
-            raise se.VolumeDoesNotExist(self.volUUID)
-
-        self.volumePath = volPath
-        if not sdCache.produce(self.sdUUID).isISO():
-            self.validateMetaVolumePath()
-
-    def validateMetaVolumePath(self):
-        """
-        In file volume repositories,
-        the volume metadata must exists after the image/volume is created.
-        """
-        metaVolumePath = self._getMetaVolumePath()
-        if not self.oop.fileUtils.pathExists(metaVolumePath):
-            raise se.VolumeDoesNotExist(self.volUUID)
 
     def getVolumeSize(self, bs=BLOCK_SIZE):
         """
diff --git a/vdsm/storage/volume.py b/vdsm/storage/volume.py
index 26487bd..c865b16 100644
--- a/vdsm/storage/volume.py
+++ b/vdsm/storage/volume.py
@@ -134,6 +134,7 @@
         self.sdUUID = sdUUID
         self.imgUUID = imgUUID
         self.volUUID = volUUID
+        self._volumePath = None
         self._imagePath = None
         self.voltype = None
 
@@ -148,13 +149,26 @@
             self.validateImagePath()
         return self._imagePath
 
+    @property
+    def volumePath(self):
+        if self._volumePath is None:
+            self.validateVolumePath()
+        return self._volumePath
+
+    def getVolumePath(self):
+        """
+        Get the path of the volume file/link
+        """
+        if not self._volumePath:
+            raise se.VolumeAccessError(self.volUUID)
+        return self._volumePath
+
 
 class Volume(object):
     log = logging.getLogger('Storage.Volume')
 
     def __init__(self, md):
         self.md = md
-        self.volumePath = None
         self.validate()
 
     @property
@@ -172,6 +186,10 @@
     @property
     def repoPath(self):
         return self.md.repoPath
+
+    @property
+    def volumePath(self):
+        return self.md.volumePath
 
     @property
     def imagePath(self):
@@ -192,7 +210,7 @@
         Validate that the volume can be accessed
         """
         self.md.validateImagePath()
-        self.validateVolumePath()
+        self.md.validateVolumePath()
 
     def __str__(self):
         return str(self.volUUID)
@@ -939,12 +957,7 @@
         self.setParentMeta(puuid)
 
     def getVolumePath(self):
-        """
-        Get the path of the volume file/link
-        """
-        if not self.volumePath:
-            raise se.VolumeAccessError(self.volUUID)
-        return self.volumePath
+        return self.md.getVolumePath()
 
     def getVmVolumeInfo(self):
         """


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5b38519df298d6e7b9a4bba2642d615976e0ea04
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>


More information about the vdsm-patches mailing list