Change in vdsm[master]: fix error handling for misc.NoIntrPoll
danken at redhat.com
danken at redhat.com
Fri Nov 30 07:40:10 UTC 2012
Dan Kenigsberg has posted comments on this change.
Change subject: fix error handling for misc.NoIntrPoll
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(4 inline comments)
....................................................
Commit Message
Line 10: and epoll raise OSError.
Line 11: tested poll/epoll, pipe/file, closed pipe/pipe buffer overflow
Line 12: pipe write/read end error will not raise poll error.
Line 13: Found in:
Line 14: http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/170/
yeah, it has expired (low disk space). It would have been better to quote the exception here.
Line 15:
Line 16: Change-Id: I272654a9006fdab77e5fab608ac287416d75843e
....................................................
File tests/miscTests.py
Line 49: SUDO_USER = "root"
Line 50: SUDO_GROUP = "root"
Line 51:
Line 52: PIPE_BUF_BYTES = 65536
Line 53: RETRIES = 3
I would very much prefer these constants as class-level. they have nothing to do with other classes of this module.
I personally do not see the point of having both EPOLL_INTERVAL_SEC and SLEEP_INTERVAL.
Line 54: SLEEP_INTERVAL = 0.1
Line 55: EPOLL_INTERVAL_SEC = SLEEP_INTERVAL * RETRIES * 2
Line 56: POLL_INTERVAL_MSEC = EPOLL_INTERVAL_SEC * 1000
Line 57:
Line 1154: misc.NoIntrPoll(poller.poll, pollInterval)
Line 1155: poller.unregister(fd)
Line 1156:
Line 1157: def testWatchFile(self):
Line 1158: tempFd, tempPath = tempfile.mkstemp()
better remove this dir in a "finally" clause.
Line 1159: self._startFakeSigchld()
Line 1160: # epoll don't support regular file
Line 1161: self._noIntrWatchFd(tempFd, isEpoll=False)
Line 1162:
Line 1183: proc.join()
Line 1184:
Line 1185: def testPipeWriteEAGAIN(self):
Line 1186: def _raiseEAGAIN(pipe):
Line 1187: str = os.urandom(1 + PIPE_BUF_BYTES)
why bother urandom? isn't
'0' * (1 + PIPE_BUF_BYTES)
enough?
also, "str" is a builtin python type. better choose another name for your strings.
Line 1188: for i in range(RETRIES):
Line 1189: time.sleep(SLEEP_INTERVAL)
Line 1190: try:
Line 1191: os.write(pipe, str)
--
To view, visit http://gerrit.ovirt.org/9458
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I272654a9006fdab77e5fab608ac287416d75843e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Barak Azulay <bazulay at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list