Nir Soffer has posted comments on this change. Change subject: StorageDomainManifest: move getLeasesFilePath ......................................................................
Patch Set 2: (4 comments) https://gerrit.ovirt.org/#/c/41995/2/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 423: Line 424: return int(size) Line 425: Line 426: def getLeasesFilePath(self): Line 427: lvm.activateLVs(self.sdUUID, [sd.LEASES]) Activating lvs as a side offfect of getting the path is very wrong. Please add a TODO so we do not forget to fix this. Line 428: return lvm.lvPath(self.sdUUID, sd.LEASES) Line 429: Line 430: Line 431: class BlockStorageDomain(sd.StorageDomain): https://gerrit.ovirt.org/#/c/41995/2/vdsm/storage/sd.py File vdsm/storage/sd.py: Line 290 Line 291 Line 292 Line 293 Line 294 I'm not sure we need this super class, better if we can eliminate it. Line 305: Line 306: def replaceMetadata(self, md): Line 307: self._metadata = md Line 308: Line 309: def getLeasesFilePath(self): This is also can be in FileStorageDomainManifest Line 310: return os.path.join(self.getMDPath(), LEASES) Line 311: Line 312: def getMDPath(self): Line 313: if self.domaindir: Line 308: Line 309: def getLeasesFilePath(self): Line 310: return os.path.join(self.getMDPath(), LEASES) Line 311: Line 312: def getMDPath(self): This looks like file storage domain method that sneaked into StorageDomain. While we move stuff, I would be nice to eliminate such errors and move this the the file based manifest. Line 313: if self.domaindir: Line 314: return os.path.join(self.domaindir, DOMAIN_META_DATA) Line 315: return None Line 316: -- To view, visit https://gerrit.ovirt.org/41995 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2bb49591357e0ae77268789da78e8504864a8057 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[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
