Piotr Kliczewski 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.
It is the same bug. Having only below fix is not enough. Without above line 
vdsm still goes to infinite loop when running steps described in BZ.
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
Done
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

Reply via email to