Nir Soffer has posted comments on this change. Change subject: StorageDomainManifest: introduce class heirarchy ......................................................................
Patch Set 1: (6 comments) https://gerrit.ovirt.org/#/c/41993/1/vdsm/storage/fileSD.py File vdsm/storage/fileSD.py: Line 171: sd.StorageDomain.__init__(self, manifest) Line 172: Line 173: if not self.oop.fileUtils.pathExists(manifest.metafile): Line 174: raise se.StorageDomainMetadataNotFound(manifest.sdUUID, Line 175: manifest.metafile) we check the manifest.metafile, and raise exception about the the manifest.sdUUID and manifest.metafile. The code tell us that it want to be in the manifest class. Line 176: self.imageGarbageCollector() Line 177: self._registerResourceNamespaces() Line 178: Line 179: @property Line 266: REMOTE_PATH: remotePath Line 267: }) Line 268: Line 269: def getReadDelay(self): Line 270: stats = misc.readspeed(self.manifest.metafile, 4096) Again, this is about the manifest, lets move the method to the manifest. Line 271: return stats['seconds'] Line 272: Line 273: def getFileList(self, pattern, caseSensitive): Line 274: """ https://gerrit.ovirt.org/#/c/41993/1/vdsm/storage/sd.py File vdsm/storage/sd.py: Line 313: 3: (clusterlock.SANLock, True), Line 314: } Line 315: Line 316: def __init__(self, manifest): Line 317: self.manifest = manifest Why is this public? Line 318: self._lock = threading.Lock() Line 319: self.stat = None Line 320: self._clusterLock = self._makeClusterLock() Line 321: Line 324: threading.Thread(target=self.stat.stop).start() Line 325: Line 326: @property Line 327: def sdUUID(self): Line 328: return self.manifest.sdUUID This breaks fileSDTests, assuming that sdUUID is writable. https://gerrit.ovirt.org/42006 should fix this issue, please rebase on top of it. Line 329: Line 330: @property Line 331: def domaindir(self): Line 332: return self.manifest.domaindir Line 335: def mountpoint(self): Line 336: return self.manifest.mountpoint Line 337: Line 338: @property Line 339: def _metadata(self): Unneeded, this is private. Callers in this class or subclass should access the manifest. Line 340: return self.manifest._metadata Line 341: Line 342: def replaceMetadata(self, md): Line 343: """ Line 342: def replaceMetadata(self, md): Line 343: """ Line 344: Used by FormatConverter to replace the metadata reader/writer Line 345: """ Line 346: self.manifest._metadata = md Please add a setter in the manifest class to replace the metadata. Line 347: Line 348: def prepareMailbox(self): Line 349: """ Line 350: 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: comment Gerrit-Change-Id: Ia275424a8d423722efb229a4d8545f035fe5fa51 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
