Deepak C Shetty has posted comments on this change. Change subject: [WIP] Add new repository management code ......................................................................
Patch Set 17: (3 inline comments) Few Qs and suggestions. .................................................... File vdsm/storage/imageRepository/engines/engineUtils.py Line 76: _cls_lock = threading.Lock() Line 77: Line 78: @property Line 79: def sanlockFD(self): Line 80: if SanlockLockingBackend._sanlock_fd is not None: s/_sanlock_fd/_sanlockFD ? Line 81: return SanlockLockingBackend._sanlock_fd Line 82: Line 83: with self._cls_lock: Line 84: if SanlockLockingBackend._sanlock_fd is not None: Line 159: VOL_MD_SUFFIX = ".md.json" Line 160: TMP_SUFFIX = ".tmp" Line 161: LOCK_SUFFIX = ".lock" Line 162: Line 163: def getCapabilites(self): Having getCapabilities @classmethod will be useful, so that clients don't necessarily have to instantiate to know the capabilities. Line 164: caps = [CAP_SPARSE_FILES, Line 165: CAP_NATIVE_THIN_PROVISIONING, Line 166: CAP_VOLUME_PREPARE_REQUIRED] Line 167: Line 169: caps.append(CAP_MULTI_HOST) Line 170: Line 171: return caps Line 172: Line 173: def __init__(self, path, formatStr, multiHost=False): IIUC, The signature doesn't match with localfs/__init__.py where u have return FsEngine(path, FORMAT, LocalFsLockBackend()) Do you intend to take the lockImpl as part of init or is it ok to set the lockImpl via a classmethod function ? Doing the latter will allow getCapabilities to be a classmethod as well Line 174: self._formatStr = formatStr Line 175: self._root = os.path.abspath(path) Line 176: self.sanityCheck() Line 177: self._multiHost = multiHost -- To view, visit http://gerrit.ovirt.org/6247 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib09c89cf982b475f45d26b2428fe05e2f4565dab Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Deepak C Shetty <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Igor Lvovsky <[email protected]> Gerrit-Reviewer: Royce Lv <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shu Ming <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
