Change in vdsm[master]: StorageDomainManifest: Move acquireVolumeMetadataSlot

alitke at redhat.com alitke at redhat.com
Wed Aug 26 20:47:35 UTC 2015


Adam Litke has uploaded a new change for review.

Change subject: StorageDomainManifest: Move acquireVolumeMetadataSlot
......................................................................

StorageDomainManifest: Move acquireVolumeMetadataSlot

Change-Id: I690ed4151428b22774415f38b350676391ccca46
Signed-off-by: Adam Litke <alitke at redhat.com>
---
M tests/sdm_indirection_tests.py
M vdsm/storage/blockSD.py
2 files changed, 98 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/89/45389/1

diff --git a/tests/sdm_indirection_tests.py b/tests/sdm_indirection_tests.py
index b4074d6..45872eb 100644
--- a/tests/sdm_indirection_tests.py
+++ b/tests/sdm_indirection_tests.py
@@ -17,6 +17,8 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
+from contextlib import contextmanager
+
 from testlib import VdsmTestCase
 from testlib import permutations, expandPermutations
 from testlib import recorded
@@ -219,6 +221,11 @@
     def _getImgExclusiveVols(self, imgUUID, volsImgs):
         pass
 
+    @recorded
+    @contextmanager
+    def acquireVolumeMetadataSlot(self, vol_name, slotSize):
+        yield
+
 
 class FakeFileDomainManifest(FakeDomainManifest):
     def __init__(self):
@@ -364,6 +371,11 @@
         self.assertEqual(512, self.dom.logBlkSize)
         self.assertEqual(512, self.dom.phyBlkSize)
 
+    def test_acquirevolumemetadataslot(self):
+        with self.dom.acquireVolumeMetadataSlot(0, 1):
+            result = [('acquireVolumeMetadataSlot', (0,1), {})]
+            self.assertEqual(self.dom._manifest.__recording__, result)
+
     @permutations([
         ['extend', 2],
         ['resizePV', 1],
diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py
index 2cbc77c..b1b18c3 100644
--- a/vdsm/storage/blockSD.py
+++ b/vdsm/storage/blockSD.py
@@ -705,6 +705,89 @@
         lvm.invalidateVG(self.sdUUID)
         self.replaceMetadata(selectMetadata(self.sdUUID))
 
+    _lvTagMetaSlotLock = threading.Lock()
+
+    @contextmanager
+    def acquireVolumeMetadataSlot(self, vol_name, slotSize):
+        # TODO: Check if the lock is needed when using
+        # getVolumeMetadataOffsetFromPvMapping()
+        with self._lvTagMetaSlotLock:
+            if self.getVersion() in VERS_METADATA_LV:
+                yield self._getVolumeMetadataOffsetFromPvMapping(vol_name)
+            else:
+                yield self._getFreeMetadataSlot(slotSize)
+
+    def _getVolumeMetadataOffsetFromPvMapping(self, vol_name):
+        dev, ext = lvm.getFirstExt(self.sdUUID, vol_name)
+        self.log.debug("vol %s dev %s ext %s" % (vol_name, dev, ext))
+        for pv in self.readMetadataMapping().values():
+            self.log.debug("MAPOFFSET: pv %s -- dev %s ext %s" %
+                           (pv, dev, ext))
+            pestart = int(pv["pestart"])
+            pecount = int(pv["pecount"])
+            if (os.path.basename(dev) == pv["guid"] and
+                    int(ext) in range(pestart, pestart + pecount)):
+
+                offs = int(ext) + int(pv["mapoffset"])
+                if offs < SD_METADATA_SIZE / sd.METASIZE:
+                    raise se.MetaDataMappingError(
+                        "domain %s: vol %s MD offset %s is bad - will "
+                        "overwrite SD's MD" % (self.sdUUID, vol_name, offs))
+                return offs
+        raise se.MetaDataMappingError("domain %s: can't map PV %s ext %s" %
+                                      (self.sdUUID, dev, ext))
+
+    def _getFreeMetadataSlot(self, slotSize):
+        occupiedSlots = self._getOccupiedMetadataSlots()
+
+        # It might look weird skipping the sd metadata when it has been moved
+        # to tags. But this is here because domain metadata and volume metadata
+        # look the same. The domain might get confused and think it has lv
+        # metadata if it finds something is written in that area.
+        freeSlot = (SD_METADATA_SIZE + self.logBlkSize - 1) / self.logBlkSize
+
+        for offset, size in occupiedSlots:
+            if offset - freeSlot > slotSize:
+                break
+
+            freeSlot = offset + size
+
+        self.log.debug("Found freeSlot %s in VG %s", freeSlot, self.sdUUID)
+        return freeSlot
+
+    def _getOccupiedMetadataSlots(self):
+        stripPrefix = lambda s, pfx: s[len(pfx):]
+        occupiedSlots = []
+        for lv in lvm.getLV(self.sdUUID):
+            if lv.name in SPECIAL_LVS:
+                # Special LVs have no mapping
+                continue
+
+            offset = None
+            size = blockVolume.VOLUME_MDNUMBLKS
+            for tag in lv.tags:
+                if tag.startswith(blockVolume.TAG_PREFIX_MD):
+                    offset = int(stripPrefix(tag, blockVolume.TAG_PREFIX_MD))
+
+                if tag.startswith(blockVolume.TAG_PREFIX_MDNUMBLKS):
+                    size = int(stripPrefix(tag,
+                                           blockVolume.TAG_PREFIX_MDNUMBLKS))
+
+                if offset is not None and size != blockVolume.VOLUME_MDNUMBLKS:
+                    # I've found everything I need
+                    break
+
+            if offset is None:
+                self.log.warn("Could not find mapping for lv %s/%s",
+                              self.sdUUID, lv.name)
+                continue
+
+            occupiedSlots.append((offset, size))
+
+        occupiedSlots.sort(key=itemgetter(0))
+        return occupiedSlots
+
+
 class BlockStorageDomain(sd.StorageDomain):
     manifestClass = BlockStorageDomainManifest
 
@@ -900,83 +983,9 @@
 
     @contextmanager
     def acquireVolumeMetadataSlot(self, vol_name, slotSize):
-        # TODO: Check if the lock is needed when using
-        # getVolumeMetadataOffsetFromPvMapping()
-        with self._lvTagMetaSlotLock:
-            if self.getVersion() in VERS_METADATA_LV:
-                yield self.getVolumeMetadataOffsetFromPvMapping(vol_name)
-            else:
-                yield self.getFreeMetadataSlot(slotSize)
-
-    def _getOccupiedMetadataSlots(self):
-        stripPrefix = lambda s, pfx: s[len(pfx):]
-        occupiedSlots = []
-        for lv in lvm.getLV(self.sdUUID):
-            if lv.name in SPECIAL_LVS:
-                # Special LVs have no mapping
-                continue
-
-            offset = None
-            size = blockVolume.VOLUME_MDNUMBLKS
-            for tag in lv.tags:
-                if tag.startswith(blockVolume.TAG_PREFIX_MD):
-                    offset = int(stripPrefix(tag, blockVolume.TAG_PREFIX_MD))
-
-                if tag.startswith(blockVolume.TAG_PREFIX_MDNUMBLKS):
-                    size = int(stripPrefix(tag,
-                                           blockVolume.TAG_PREFIX_MDNUMBLKS))
-
-                if offset is not None and size != blockVolume.VOLUME_MDNUMBLKS:
-                    # I've found everything I need
-                    break
-
-            if offset is None:
-                self.log.warn("Could not find mapping for lv %s/%s",
-                              self.sdUUID, lv.name)
-                continue
-
-            occupiedSlots.append((offset, size))
-
-        occupiedSlots.sort(key=itemgetter(0))
-        return occupiedSlots
-
-    def getFreeMetadataSlot(self, slotSize):
-        occupiedSlots = self._getOccupiedMetadataSlots()
-
-        # It might look weird skipping the sd metadata when it has been moved
-        # to tags. But this is here because domain metadata and volume metadata
-        # look the same. The domain might get confused and think it has lv
-        # metadata if it finds something is written in that area.
-        freeSlot = (SD_METADATA_SIZE + self.logBlkSize - 1) / self.logBlkSize
-
-        for offset, size in occupiedSlots:
-            if offset - freeSlot > slotSize:
-                break
-
-            freeSlot = offset + size
-
-        self.log.debug("Found freeSlot %s in VG %s", freeSlot, self.sdUUID)
-        return freeSlot
-
-    def getVolumeMetadataOffsetFromPvMapping(self, vol_name):
-        dev, ext = lvm.getFirstExt(self.sdUUID, vol_name)
-        self.log.debug("vol %s dev %s ext %s" % (vol_name, dev, ext))
-        for pv in self.readMetadataMapping().values():
-            self.log.debug("MAPOFFSET: pv %s -- dev %s ext %s" %
-                           (pv, dev, ext))
-            pestart = int(pv["pestart"])
-            pecount = int(pv["pecount"])
-            if (os.path.basename(dev) == pv["guid"] and
-                    int(ext) in range(pestart, pestart + pecount)):
-
-                offs = int(ext) + int(pv["mapoffset"])
-                if offs < SD_METADATA_SIZE / sd.METASIZE:
-                    raise se.MetaDataMappingError(
-                        "domain %s: vol %s MD offset %s is bad - will "
-                        "overwrite SD's MD" % (self.sdUUID, vol_name, offs))
-                return offs
-        raise se.MetaDataMappingError("domain %s: can't map PV %s ext %s" %
-                                      (self.sdUUID, dev, ext))
+        with self._manifest.acquireVolumeMetadataSlot(vol_name, slotSize) \
+                as slot:
+            yield slot
 
     def readMetadataMapping(self):
         return self._manifest.readMetadataMapping()


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I690ed4151428b22774415f38b350676391ccca46
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