Adam Litke has posted comments on this change. Change subject: StorageDomainManifest: introduce class heirarchy ......................................................................
Patch Set 1: (5 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. Done Line 176: self.imageGarbageCollector() Line 177: self._registerResourceNamespaces() Line 178: Line 179: @property 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? No good reason. I'll change it. 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. ok, thanks... +2'd that one. 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 This is for transition purposes. _metadata is used by a lot of functions in StorageDomain which will eventually be moved to StorageDomainManifest. Exposing a property now preserves access semantics during refactoring so functions do not need to be changed before they are moved into the manifest. Once everything is moved over we should be able to remove this property 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. Done 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: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Freddy Rolland <[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
