Federico Simoncelli has posted comments on this change.

Change subject: getAllTasksList\Status with spUUID retrieves info only if host 
is the SPM
......................................................................


Patch Set 17: I would prefer that you didn't submit this

(2 inline comments)

One question.

....................................................
File vdsm/storage/hsm.py
Line 2051:         # SPM status changes (HSM tasks or empty task list after 
cleaning SPM
Line 2052:         # info)
Line 2053:         # To reduce the race we validate SPM status after retreiving
Line 2054:         # empty task list.
Line 2055:         allTasksStatus = self.taskMng.getAllTasksStatuses("spm")
Since we're correlating this with the spm status, isn't this racy?
What if we fetch the tasks status (empty) and then the host becomes the spm?
Aren't we returning an empty list? (Which is what the bug is trying to fix)

sp.py:
 
  self.taskMng.loadDumpedTasks(self.tasksDir)
 
  self.spmRole = SPM_ACQUIRED
Line 2056:         if spUUID and len(allTasksStatus) == 0:
Line 2057:             self.validateSPM(spUUID)
Line 2058:         return dict(allTasksStatus=allTasksStatus)
Line 2059: 


Line 2098:         # SPM status changes (HSM tasks or empty task list after 
cleaning SPM
Line 2099:         # info)
Line 2100:         # To reduce the race we validate SPM status after retreiving
Line 2101:         # the task list.
Line 2102:         allTasksInfo = self.taskMng.getAllTasksInfo("spm")
Same here.
Line 2103:         if spUUID and len(allTasksInfo) == 0:
Line 2104:             self.validateSPM(spUUID)
Line 2105:         return dict(allTasksInfo=allTasksInfo)
Line 2106: 


--
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: 17
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: Federico Simoncelli <[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: liron aravot <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to