Ayal Baron has posted comments on this change. Change subject: Distinguish between local and mounted SD's. ......................................................................
Patch Set 4: Do not submit (2 inline comments) Giving -2 until Shu Ming's comment is answered (as -1 gets lost every time a new revision is sent). .................................................... File vdsm/storage/localFsSD.py Line 102: def getRealPath(self): Line 103: return os.readlink(self.mountpoint) Line 104: Line 105: Line 106: def isLocalFsDomain(domainPath): Maybe if we try this a third time it will work. To quote Shu Ming "I don't think we need a new function here. The exsiting os.path.ismount() can be used to replace this function." Why not use os.path.ismount()? Line 107: """This ancillary function differentiates local and mounted SD's. Line 108: Line 109: Assumed that a local SD can't be mounted. Line 110: mounted: nfs, posixFS Line 116: if mPoint == '/': Line 117: continue Line 118: elif mPoint in domainPath: Line 119: # This is a mounted domain therefore not exists as a local SD. Line 120: isLocal = False Just return False. for...else semantics is confusing. having a temp variable here is redundant code. else below adds redundant indentation. it takes longer to read the code. Line 121: break Line 122: else: Line 123: isLocal = True Line 124: return isLocal -- To view, visit http://gerrit.ovirt.org/10024 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12b5ba80121b8e8d23a63ecc7aaab829bfc67a51 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Eduardo <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Daniel Paikov <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Haim Ateya <[email protected]> Gerrit-Reviewer: Shu Ming <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
