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 <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: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes