Adam Litke has uploaded a new change for review. Change subject: StorageDomainManifest: introduce class heirarchy ......................................................................
StorageDomainManifest: introduce class heirarchy Many of the functions in the StorageDomain class family serve to provide information about the storage domain and its contents (ie. images and volumes). Other functions manipulate image or volume metadata in a fine-grained, reusable way (ie. extendVolume). As we refactor the storage code to operate without an SPM the new code will benefit from reusing these Storage Domain "manifest" functions. In order to make reuse clean and safe, we want to refactor the StorageDomain class by splitting the reusable elements into a StorageDomainManifest class. The StorageDomain object will initialize and use a StorageDomainManifest object for much of its normal operations. Meanwhile, the new SDM code may use a StorageDomainManifest object directly without having access to the non-sharable parts of StorageDomain. In an effort to reduce churn during the refactoring, when moving a method into its new class, I will leave behind a stub in the original class which simply passes the call through to the method's new location. Once all code has been refactored, I'll remove stubs for any methods which are not currently accessed outside of the class. Change-Id: Ia275424a8d423722efb229a4d8545f035fe5fa51 Signed-off-by: Adam Litke <[email protected]> --- M vdsm/storage/blockSD.py M vdsm/storage/fileSD.py M vdsm/storage/imageRepository/formatConverter.py M vdsm/storage/sd.py 4 files changed, 64 insertions(+), 19 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/41993/1 diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 490b624..8674dba 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -395,14 +395,22 @@ return {'mdathreshold': mda_free_ok, 'mdavalid': mda_size_ok} -class BlockStorageDomain(sd.StorageDomain): +class BlockStorageDomainManifest(sd.StorageDomainManifest): mountpoint = os.path.join(sd.StorageDomain.storage_repository, sd.DOMAIN_MNT_POINT, sd.BLOCKSD_DIR) def __init__(self, sdUUID): domaindir = os.path.join(self.mountpoint, sdUUID) metadata = selectMetadata(sdUUID) - sd.StorageDomain.__init__(self, sdUUID, domaindir, metadata) + sd.StorageDomainManifest.__init__(self, sdUUID, domaindir, metadata) + + +class BlockStorageDomain(sd.StorageDomain): + ManifestClass = BlockStorageDomainManifest + + def __init__(self, sdUUID): + manifest = self.ManifestClass(sdUUID) + sd.StorageDomain.__init__(self, manifest) lvm.activateLVs(self.sdUUID, SPECIAL_LVS) self.metavol = lvm.lvPath(self.sdUUID, sd.METADATA) @@ -1316,7 +1324,7 @@ def refresh(self): self.refreshDirTree() lvm.invalidateVG(self.sdUUID) - self._metadata = selectMetadata(self.sdUUID) + self.replaceMetadata(selectMetadata(self.sdUUID)) @staticmethod def findDomainPath(sdUUID): diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index 9ceda8d..6add0f4 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -145,26 +145,34 @@ PersistentDict(FileMetadataRW(metafile)), FILE_SD_MD_FIELDS) -class FileStorageDomain(sd.StorageDomain): +class FileStorageDomainManifest(sd.StorageDomainManifest): def __init__(self, domainPath): + sdUUID = os.path.basename(domainPath) + # Using glob might look like the simplest thing to do but it isn't # If one of the mounts is stuck it'll cause the entire glob to fail # and you wouldn't be able to access any domain self.log.debug("Reading domain in path %s", domainPath) self.mountpoint = os.path.dirname(domainPath) self.remotePath = os.path.basename(self.mountpoint) + domaindir = os.path.join(self.mountpoint, sdUUID) self.metafile = os.path.join(domainPath, sd.DOMAIN_META_DATA, sd.METADATA) - - sdUUID = os.path.basename(domainPath) - validateFileSystemFeatures(sdUUID, self.mountpoint) - metadata = FileSDMetadata(self.metafile) - domaindir = os.path.join(self.mountpoint, sdUUID) - sd.StorageDomain.__init__(self, sdUUID, domaindir, metadata) + sd.StorageDomainManifest.__init__(self, sdUUID, domaindir, metadata) - if not self.oop.fileUtils.pathExists(self.metafile): - raise se.StorageDomainMetadataNotFound(sdUUID, self.metafile) + +class FileStorageDomain(sd.StorageDomain): + ManifestClass = FileStorageDomainManifest + + def __init__(self, domainPath): + manifest = self.ManifestClass(domainPath) + validateFileSystemFeatures(manifest.sdUUID, manifest.mountpoint) + sd.StorageDomain.__init__(self, manifest) + + if not self.oop.fileUtils.pathExists(manifest.metafile): + raise se.StorageDomainMetadataNotFound(manifest.sdUUID, + manifest.metafile) self.imageGarbageCollector() self._registerResourceNamespaces() @@ -259,7 +267,7 @@ }) def getReadDelay(self): - stats = misc.readspeed(self.metafile, 4096) + stats = misc.readspeed(self.manifest.metafile, 4096) return stats['seconds'] def getFileList(self, pattern, caseSensitive): @@ -537,7 +545,7 @@ return True def getRemotePath(self): - return self.remotePath + return self.manifest.remotePath def getRealPath(self): """ diff --git a/vdsm/storage/imageRepository/formatConverter.py b/vdsm/storage/imageRepository/formatConverter.py index b0720ca..5aaafef 100644 --- a/vdsm/storage/imageRepository/formatConverter.py +++ b/vdsm/storage/imageRepository/formatConverter.py @@ -60,7 +60,7 @@ if chkMetadata[sd.DMDK_VERSION] == int(targetVersion): # Switching to the newMetadata (successful upgrade), the oldMetadata # was cleared after all. - domain._metadata = chkMetadata + domain.replaceMetadata(chkMetadata) log.debug("Conversion of domain %s to tag based metadata completed, " "target version = %s", domain.sdUUID, targetVersion) else: diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index a771abf..ab86987 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -291,6 +291,15 @@ } +class StorageDomainManifest(object): + log = logging.getLogger("Storage.StorageDomainManifest") + + def __init__(self, sdUUID, domaindir, metadata): + self.sdUUID = sdUUID + self.domaindir = domaindir + self._metadata = metadata + + class StorageDomain(object): log = logging.getLogger("Storage.StorageDomain") storage_repository = config.get('irs', 'repository') @@ -304,10 +313,8 @@ 3: (clusterlock.SANLock, True), } - def __init__(self, sdUUID, domaindir, metadata): - self.sdUUID = sdUUID - self.domaindir = domaindir - self._metadata = metadata + def __init__(self, manifest): + self.manifest = manifest self._lock = threading.Lock() self.stat = None self._clusterLock = self._makeClusterLock() @@ -316,6 +323,28 @@ if self.stat: threading.Thread(target=self.stat.stop).start() + @property + def sdUUID(self): + return self.manifest.sdUUID + + @property + def domaindir(self): + return self.manifest.domaindir + + @property + def mountpoint(self): + return self.manifest.mountpoint + + @property + def _metadata(self): + return self.manifest._metadata + + def replaceMetadata(self, md): + """ + Used by FormatConverter to replace the metadata reader/writer + """ + self.manifest._metadata = md + def prepareMailbox(self): """ This method has been introduced in order to prepare the mailbox -- To view, visit https://gerrit.ovirt.org/41993 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia275424a8d423722efb229a4d8545f035fe5fa51 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
