Federico Simoncelli has posted comments on this change. Change subject: utils: add CommandStream class ......................................................................
Patch Set 4: (1 comment) https://gerrit.ovirt.org/#/c/33909/4/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 362: for fd in self._iocb: Line 363: self._poll.register(fd, select.EPOLLIN) Line 364: Line 365: def _poll_input(self, fileno): Line 366: self._iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) > We don't set the descriptors to non-blocking mode, so this may block when h If you have signal handlers that potentially take a long time to execute you have much more serious problems than CommandStream. I don't plan to make this non-blocking out of the box (the consumer can do that on his own since CommandStream is fully composable with a Popen object). Anyway it is obviously useful to document that receive may raise the same exceptions of read. At the moment the only interesting use-case is to stop the command (and not stop listening for events) which is accomplished by the Popen method terminate/kill. That will cause the process to exit and the file-descriptors to be closed. Line 367: Line 368: def _poll_event(self, fileno): Line 369: self._poll.unregister(fileno) Line 370: del self._iocb[fileno] -- To view, visit https://gerrit.ovirt.org/33909 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie015368bb9c5992e5c73a149277c59fc4ffbd570 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Shahar Havivi <[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
