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 smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server