Nir Soffer has posted comments on this change. Change subject: vdsm: Shuting down protocol detector fails ......................................................................
Patch Set 1: Code-Review-1 (2 comments) Wrong fix. Even worse - I don't believe you can get this error during normal operation. The only way to get this would be to stop the acceptor twice. Please check the log in the bug and find why the acceptor was stopped twice, and fix the bad code doing this. http://gerrit.ovirt.org/#/c/29556/1/vdsm/protocoldetector.py File vdsm/protocoldetector.py: Line 163: try: Line 164: os.write(self._write_fd, '1') Line 165: except OSError as e: Line 166: if e.errno == errno.EPIPE: Line 167: pass This will not work. You will pass, and then move to line 168, go into the elif on line 170, and raise in line 171 Line 168: if e.errno == errno.EINTR: Line 169: self.wakeup() Line 170: elif e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK): Line 171: raise Line 166: if e.errno == errno.EPIPE: Line 167: pass Line 168: if e.errno == errno.EINTR: Line 169: self.wakeup() Line 170: elif e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK): > unrelated question: why do we swallow EAGAIN error? Why not try again? EPIPE should be here with EGAIN - these are the errors we can handle. Why we should ignore EAGAIN: This pipe receive no writes until you call stop, and there is no chance that it would block. If it does, then the other side must be readable and the socket will wakeup without this write. Line 171: raise Line 172: Line 173: def _cleanup_wakeup_pipe(self): Line 174: try: -- To view, visit http://gerrit.ovirt.org/29556 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82119a61835fe335f2aa5da29fb8d3f2b8ae33fc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[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
