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: -- To view, visit http://gerrit.ovirt.org/33909 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie015368bb9c5992e5c73a149277c59fc4ffbd570 Gerrit-PatchSet: 3 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: [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
