Ayal Baron has posted comments on this change.
Change subject: Refactor StoragePool.getPoolParams()
......................................................................
Patch Set 2: Code-Review-1
(5 comments)
....................................................
Commit Message
Line 3: AuthorDate: 2013-09-13 12:40:03 +0300
Line 4: Commit: Eduardo Warszawski <ewarszaw(a)redhat.com>
Line 5: CommitDate: 2013-09-15 15:05:25 +0300
Line 6:
Line 7: Refactor StoragePool.getPoolParams()
why?
Line 8:
Line 9: Change-Id: Ibbeaaaf67a39f3ca8b27252fc631a91f266d1adc
....................................................
File vdsm/storage/hsm.py
Line 379: # exist under pool link as well (for hsm tasks)
Line 380: self.taskMng.loadDumpedTasks(self.tasksDir)
Line 381: self.taskMng.recoverDumpedTasks()
Line 382:
Line 383: poolDirs = os.listdir(self._poolsTmpDir)
where did self._poolsTmpDir come from? (bug?)
Line 384: for spUUID in poolDirs:
Line 385: poolPath = os.path.join(self.storage_repository, spUUID)
Line 386: poolParamFile = os.path.join(self._poolsTmpDir, spUUID)
Line 387: if os.path.exists(poolPath):
Line 383: poolDirs = os.listdir(self._poolsTmpDir)
Line 384: for spUUID in poolDirs:
Line 385: poolPath = os.path.join(self.storage_repository, spUUID)
Line 386: poolParamFile = os.path.join(self._poolsTmpDir, spUUID)
Line 387: if os.path.exists(poolPath):
return early, avoid indentation. I hate this pattern.
if not os.path.exists(poolPath):
continue
Line 388: try:
Line 389: hostID, scsiKey, msdUUID, masterVersion = \
Line 390: sp.getPoolParams(poolParamFile)
Line 391: except OSError:
Line 400: self.log.error("Connection to SP %s
failed",
Line 401: spUUID, exc_info=True)
Line 402: else:
Line 403: # TODO Once we support simultaneous connection to
Line 404: # multiple pools, remove following line (break)
why leave this comment? this will never happen...
no reason to separate break from _connectStoragePool
Line 405: break
Line 406:
Line 407: storageRefreshThread = threading.Thread(target=storageRefresh,
Line 408:
name="storageRefresh")
Line 1043: pool = self.getPool(spUUID)
Line 1044: except se.StoragePoolUnknown:
Line 1045: pass # pool not connected yet
Line 1046: else:
Line 1047: misc.validateN(hostID, 'hostID')
this will probably fail as you now pass hostID as string instead of int (and if not here,
possibly elsewhere since you've changed the API)
Line 1048: # Idem. See above.
Line 1049: pool.getMasterDomain(msdUUID=msdUUID,
Line 1050: masterVersion=masterVersion)
Line 1051: return
--
To view, visit
http://gerrit.ovirt.org/19232
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibbeaaaf67a39f3ca8b27252fc631a91f266d1adc
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Elad Ben Aharon <eladba1990(a)gmail.com>
Gerrit-Reviewer: Gadi Ickowicz <gickowic(a)redhat.com>
Gerrit-Reviewer: Meital Bourvine <mbourvin(a)redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes