Nir Soffer has posted comments on this change.
Change subject: utils: add CommandStream class ......................................................................
Patch Set 3:
(7 comments)
http://gerrit.ovirt.org/#/c/33909/3/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 331: Line 332: class CommandStream(object): Line 333: def __init__(self, command, stdoutcb, stderrcb): Line 334: self._command = command Line 335: self.epoll = select.epoll() Should be private. Line 336: Line 337: self.iocb = { Line 338: self._command.stdout.fileno(): stdoutcb, Line 339: self._command.stderr.fileno(): stderrcb,
Line 333: def __init__(self, command, stdoutcb, stderrcb): Line 334: self._command = command Line 335: self.epoll = select.epoll() Line 336: Line 337: self.iocb = { Same - private Line 338: self._command.stdout.fileno(): stdoutcb, Line 339: self._command.stderr.fileno(): stderrcb, Line 340: } Line 341:
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? 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. Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed:
Line 365: def receive(self, timeout=None): Line 366: if timeout is None: Line 367: epoll_remaining = -1 Line 368: else: Line 369: endtime = monotonic_time() + timeout I like to call this "deadline", in case you are not attached to "endtime". Line 370: Line 371: while not self.closed: Line 372: if timeout is not None: Line 373: epoll_remaining = endtime - monotonic_time()
Line 368: else: Line 369: endtime = monotonic_time() + timeout Line 370: Line 371: while not self.closed: Line 372: if timeout is not None: It is little ugly to check every time for timeout is None. Line 373: epoll_remaining = endtime - monotonic_time() Line 374: if epoll_remaining <= 0: Line 375: break Line 376:
Line 371: while not self.closed: Line 372: if timeout is not None: Line 373: epoll_remaining = endtime - monotonic_time() Line 374: if epoll_remaining <= 0: Line 375: break But since we do check, lets put the -1 here?
else: epoll_remaining = -1
We can simplify more if we use some big default timeout:
def receive(self, timeout=sys.maxint): now = monotonic_time() deadline = now + timeout while not self.closed and now < deadline: self._poll(deadline - now) now = monotonic_time() Line 376: Line 377: self._epoll_timeout(epoll_remaining) Line 378: Line 379: