Federico Simoncelli has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
(5 comments)
http://gerrit.ovirt.org/#/c/33909/3/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 335: self.epoll = select.epoll() Line 336: Line 337: self.iocb = { Line 338: self._command.stdout.fileno(): stdoutcb, Line 339: self._command.stderr.fileno(): stderrcb,
Will this work when we redirect stderr to stdout (do we have same fileno fo
Yes. Currently everything is squashed to the stderrcb, I'd better invert them and add a comment. Line 340: } Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN)
Line 341: Line 342: for fd in self.iocb: Line 343: self.epoll.register(fd, select.EPOLLIN) Line 344: Line 345: def _epoll_input(self, fileno):
I commented about this name in a previous version.
This method is handling the poll input event, the name seems consistent. Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno)
Line 344: Line 345: def _epoll_input(self, fileno): Line 346: self.iocb[fileno](os.read(fileno, io.DEFAULT_BUFFER_SIZE)) Line 347: Line 348: def _epoll_event(self, fileno):
I commented about this name in a previous version.
This method is handling the poll events, the name seems consistent. Line 349: self.epoll.unregister(fileno) Line 350: del self.iocb[fileno] Line 351: Line 352: def _epoll_timeout(self, timeout):
Line 348: def _epoll_event(self, fileno): Line 349: self.epoll.unregister(fileno) Line 350: del self.iocb[fileno] Line 351: Line 352: def _epoll_timeout(self, timeout):
Rename to _poll?
This method is polling with timeout, the name seems consistent. Line 353: fdevents = NoIntrPoll(self.epoll.poll, timeout) Line 354: Line 355: for fileno, event in fdevents: Line 356: if event & select.EPOLLIN:
Line 363: return len(self.iocb) == 0 Line 364: Line 365: def receive(self, timeout=None): Line 366: if timeout is None: Line 367: epoll_remaining = -1
We can do this in the loop bellow instead.
If we do it in the loop it will be set at every cycle unless we rely on some implicit python optimization. Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed: