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 <fsimonce(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes