Dan Kenigsberg has posted comments on this change. Change subject: task: prevent tasks with the same id to be queued ......................................................................
Patch Set 6: (5 comments) http://gerrit.ovirt.org/#/c/26808/6/vdsm/storage/taskManager.py File vdsm/storage/taskManager.py: Line 1: # Line 2: # Copyright 2009-2011 Red Hat, Inc. admit it, you have considerable code changes here this year. ;-) Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 6: # the Free Software Foundation; either version 2 of the License, or Line 58: Line 59: try: Line 60: if not self.tp.queueTask(task.id, method): Line 61: self.log.error("unable to queue task: %s", task.dumpTask()) Line 62: with self._tasksLock: I'm not sure that this protection is required (since we bail we never assume that a task.id is there within a critical section. Strictly speaking, we need to protect only against additions to the dict. However, protecting all changes to self._tasks is the prudent thing to do (for future careless changes). However this means more changes in this file, and merits its own patch. Line 63: del self._tasks[task.id] Line 64: raise se.AddTaskError() Line 65: self.log.debug("task queued: %s", task.id) Line 66: except Exception: Line 90: """ Line 91: self.log.debug("Request to stop all tasks") Line 92: Line 93: # Clear the task queue and terminate all pooled threads Line 94: for t in self._tasks: not a big worry, but this could be protected by the new lock, for completeness Line 95: if hasattr(t, 'stop'): Line 96: t.stop() Line 97: self.log.info(str(t)) Line 98: Line 145: Remove Tasks from managed tasks list Line 146: """ Line 147: self.log.debug("Entry.") Line 148: for taskID, task in self._tasks.items(): Line 149: if not tag or tag in task.getTags(): worth protection Line 150: self._tasks.pop(taskID, None) Line 151: self.log.debug("Return") Line 152: Line 153: def stopTask(self, taskID, force=False): Line 170: """ Clear a task according to given uuid. Line 171: """ Line 172: self.log.debug("Entry. taskID: %s", taskID) Line 173: # TODO: Should we stop here implicitly ??? Line 174: t = self.__getTask(taskID) and this Line 175: t.clean() Line 176: del self._tasks[taskID] Line 177: self.log.debug("Return.") Line 178: -- To view, visit http://gerrit.ovirt.org/26808 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b9980de08d10203a75071dae9b16c9b850a91f5 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
