Piotr Kliczewski has posted comments on this change. Change subject: protocoldetecor: SSLError handled not correctly ......................................................................
Patch Set 5: (2 comments) https://gerrit.ovirt.org/#/c/42206/5//COMMIT_MSG Commit Message: Line 7: protocoldetecor: SSLError handled not correctly Line 8: Line 9: Whenever there was ssl issue during protocol detection the code entered Line 10: infinite loop because we haven't remove fd from pending_connections. Line 11: It is fixed by caching SSLError reading data and clean up fd correctly. > In master this code does not exist, it was replaced by new code using the r I will add the label. Line 12: Line 13: Line 14: Change-Id: I8ea5c305b19c0a7421ea74e069c3ad02d9ffd141 Line 15: Signed-off-by: pkliczewski <[email protected]> https://gerrit.ovirt.org/#/c/42206/5/vdsm/protocoldetector.py File vdsm/protocoldetector.py: Line 222: return Line 223: Line 224: try: Line 225: data = client_socket.recv(self._required_size, socket.MSG_PEEK) Line 226: except (SSL.SSLError, socket.error) as e: > this is a rather awkward way to write The issue here is that we need to remove_connection and close the socket when SSLError is raised. Moving the code as you asked would mean that we will have almost identical two blocks of code. Line 227: if getattr(e, "errno", 0) not in (errno.EAGAIN, errno.EWOULDBLOCK): Line 228: self.log.warning("Unable to read data: %s", e) Line 229: self._remove_connection(client_socket) Line 230: client_socket.close() -- To view, visit https://gerrit.ovirt.org/42206 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8ea5c305b19c0a7421ea74e069c3ad02d9ffd141 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
