Ayal Baron has posted comments on this change.
Change subject: getAllTasksList\Status with 'spm_tasks' option retrieves info
only if host SPM
......................................................................
Patch Set 8: I would prefer that you didn't submit this
(4 inline comments)
....................................................
File vdsm/API.py
Line 1362:
Line 1363: def getDevicesVisibility(self, guidList):
Line 1364: return self._irs.getDevicesVisibility(guidList)
Line 1365:
Line 1366: def getAllTasksInfo(self, options=None, spUUID=None):
I hate 'options'! grrrr.
Anyway, having both options and spUUID is redundant here (not to mention that
if anything the order should be reversed).
Just need spUUID. Pool tasks always reside on the master domain hence only SPM
can return them (and if we somehow support it otherwise then no need for
exception) so in any case all you need is spUUID and you're set.
Note that this is not addressing the comments in the previous patch.
What Edu was referring to was multiple pools and the such which is irrelevant
here.
The distinction here is between single host level tasks and cluster level tasks.
For single host there is no spUUID, for cluster there is.
Getting rid of spUUID that Edu mentioned was done for entirely different
purposes (the code there looked for spUUID in case it was not provided), but
here the assumption is that if it is not passed then pool is not relevant to
the operation at all (which is fine imo)
Line 1367: return self._irs.getAllTasksInfo(options, spUUID)
Line 1368:
Line 1369: def getAllTasksStatuses(self, options=None, spUUID=None):
Line 1370: return self._irs.getAllTasksStatuses(options, spUUID)
....................................................
File vdsm/storage/hsm.py
Line 2039: def getAllTasksStatuses(self, options=None, spUUID=None):
Line 2040: """
Line 2041: Gets the status of all public tasks.
Line 2042:
Line 2043: :param options: 'spm_tasks' means reteiving if running as spm
you're not retrieving the spm tasks, you're retrieving the pool tasks. it just
so happens that only the spm can return those at the moment.
no need for options, just spUUID.
Line 2044: spUUID: when asking spm tasks, pool uuid mast passed
Line 2045:
Line 2046: :returns: a dict of all the tasks information.
Line 2047: """
Line 2046: :returns: a dict of all the tasks information.
Line 2047: """
Line 2048: # getSharedLock(tasksResource...)
Line 2049: if 'spm_tasks' in options:
Line 2050: if spUUID is None:
just need:
if spUUID:
self.validateSPM(spUUID)
Line 2051: raise se.ResourceException("Can't get SPM Tasks",
spUUID)
Line 2052: self.validateSPM(spUUID)
Line 2053: allTasksStatus = self.taskMng.getAllTasksStatuses("spm")
Line 2054: return dict(allTasksStatus=allTasksStatus)
Line 2086: :rtype: dict
Line 2087: """
Line 2088: # getSharedLock(tasksResource...)
Line 2089: # TODO: if spUUID passed, make sure tasks are relevant only
to pool
Line 2090: if 'spm_tasks' in options:
same
Line 2091: if spUUID is None:
Line 2092: raise se.ResourceException("Can't get SPM Tasks",
spUUID)
Line 2093: self.validateSPM(spUUID)
Line 2094: allTasksInfo = self.taskMng.getAllTasksInfo("spm")
--
To view, visit http://gerrit.ovirt.org/12517
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4cbc11c924f0bd078749fea26d79b39c0dd48094
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Daniel P. Berrange <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
