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 <[email protected]> --- 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 <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
