Nir Soffer has posted comments on this change. Change subject: protocoldetecor: SSLError handled not correctly ......................................................................
Patch Set 3: (2 comments) https://gerrit.ovirt.org/#/c/42206/3/vdsm/protocoldetector.py File vdsm/protocoldetector.py: Line 204: try: Line 205: socket.is_handshaking = (socket.accept_ssl() == 0) Line 206: except Exception as e: Line 207: self.log.debug("Error during handshake: %s", e) Line 208: self._remove_connection(socket) This is unrelated bug - the first patch I mentioned fixes this bug. Line 209: socket.close() Line 210: else: Line 211: if not socket.is_handshaking: Line 212: self._poller.modify(socket, self.READ_ONLY_MASK) Line 231: client_socket.close() Line 232: return Line 233: except SSL.SSLError: Line 234: self._remove_connection(client_socket) Line 235: client_socket.close() This duplicates the previous except, and does not log a warning. I think we should log a similar warning here. We can do: except (SSL.SSLError, socket.error) as e: if getattr(e, "errno", 0) not in (errno.EAGAIN, errno.EWOULDBLOCK): self._remove_connection(client_socket) client_socket.close() return Line 236: Line 237: if data is None: Line 238: # None is returned when ssl socket needs to read more data Line 239: return -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Piotr Kliczewski <[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
