Dan Kenigsberg has posted comments on this change.
Change subject: misc: handle properly timeout=-1 in NoIntrPoll ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(3 inline comments)
.................................................... File tests/miscTests.py Line 1168: myPipe, hisPipe = os.pipe() Line 1169: self._startFakeSigchld() Line 1170: self._noIntrWatchFd(myPipe, isEpoll=False) # caught select.error Line 1171: Line 1172: def testNoTimeoutPipePoll(self): it might be the time of day, it might be me, but I do not see what you are trying to test here and why. I could have used a more fat commit message. Line 1173: def _sigChldAndWrite(fd): Line 1174: self._waitAndSigchld() Line 1175: time.sleep(self.SLEEP_INTERVAL) Line 1176: os.close(fd)
Line 1169: self._startFakeSigchld() Line 1170: self._noIntrWatchFd(myPipe, isEpoll=False) # caught select.error Line 1171: Line 1172: def testNoTimeoutPipePoll(self): Line 1173: def _sigChldAndWrite(fd): where is the "write" here? Line 1174: self._waitAndSigchld() Line 1175: time.sleep(self.SLEEP_INTERVAL) Line 1176: os.close(fd) Line 1177:
.................................................... File vdsm/storage/misc.py Line 1376: except (IOError, select.error) as e: Line 1377: if e.args[0] != errno.EINTR: Line 1378: raise Line 1379: Line 1380: if endtime is not None: I'm not sure that the following is better, but I would have used
if timeout > 0:
instead of adding None to the possible values of endtime. Line 1381: timeout = max(0, endtime - time.time()) Line 1382: Line 1383: Line 1384: def isAscii(s):
-- To view, visit http://gerrit.ovirt.org/11394 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Iefc4d17559d3335ef6699cb83923c3bd255c916b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com