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 <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Royce Lv <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
