Change in vdsm[master]: Have the protocol detector use the Reactor
piotr.kliczewski at gmail.com
piotr.kliczewski at gmail.com
Mon Mar 16 08:26:15 UTC 2015
Piotr Kliczewski has posted comments on this change.
Change subject: Have the protocol detector use the Reactor
......................................................................
Patch Set 11:
(8 comments)
https://gerrit.ovirt.org/#/c/37098/11/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
Line 60: # we buffer first consumed message
Line 61: def read(self, size=4096, flag=None):
Line 62: result = None
Line 63: if flag == socket.MSG_PEEK:
Line 64: bytes_left = size - len(self._data)
> was it a bug?
It seems like a bug fix.
Line 65: if bytes_left > 0:
Line 66: self._data += self.connection.read(bytes_left)
Line 67: result = self._data
Line 68: else:
Line 65: if bytes_left > 0:
Line 66: self._data += self.connection.read(bytes_left)
Line 67: result = self._data
Line 68: else:
Line 69: if self._data:
> please state if self._data != '' if you don't use None anymore
We can get None from ssl_read_nbio (connection.read) so we need to check it here.
Line 70: result = self._data
Line 71: self._data = ''
Line 72: else:
Line 73: result = self.connection.read(size)
Line 250: self.key_file = key_file
Line 251: self.cert_file = cert_file
Line 252:
Line 253:
Line 254: class SSLHandshakeWrapper(object):
> it should be called SSLHandshakeDispatcher or at least state that this clas
Will change the name.
Line 255: log = logging.getLogger("ProtocolDetector.SSLHandshakeWrapper")
Line 256: SSL_HANDSHAKE_TIMEOUT = 10
Line 257:
Line 258: def __init__(
Line 264: self._give_up_at = monotonic_time() + handshake_timeout
Line 265: self._has_been_set_up = False
Line 266: self._is_handshaking = True
Line 267: self._sslctx = sslctx
Line 268: self._handshake_finished_handler = handshake_finished_handler
> what is handshake_finished_handler? please document a bit .. at least what
Done
Line 269:
Line 270: def _set_up_socket(self, dispatcher):
Line 271: client_socket = dispatcher.socket
Line 272: client_socket = self._sslctx.wrapSocket(client_socket)
Line 271: client_socket = dispatcher.socket
Line 272: client_socket = self._sslctx.wrapSocket(client_socket)
Line 273: # Older versions of M2Crypto ignore nbio and retry internally
Line 274: # if timeout is set.
Line 275: client_socket.settimeout(None)
> isn't this timeout define the session timeout? don't we need to expose this
I am not sure what you mean here but comment above can help.
Line 276: client_socket.address = client_socket.getpeername()
Line 277: try:
Line 278: client_socket.setup_ssl()
Line 279: client_socket.set_accept_state()
Line 277: try:
Line 278: client_socket.setup_ssl()
Line 279: client_socket.set_accept_state()
Line 280: except SSL.SSLError as e:
Line 281: self.log.warning("Error setting up ssl: %s", e)
> only warning? it should be error
Done
Line 282: dispatcher.close()
Line 283: return
Line 284:
Line 285: dispatcher.socket = client_socket
Line 304: if self._is_handshaking:
Line 305: try:
Line 306: self._is_handshaking = (dispatcher.socket.accept_ssl() == 0)
Line 307: except Exception as e:
Line 308: self.log.debug("Error during handshake: %s", e)
> error
Done
Line 309: dispatcher.close()
Line 310:
Line 311: if not self._is_handshaking:
https://gerrit.ovirt.org/#/c/37098/11/vdsm/protocoldetector.py
File vdsm/protocoldetector.py:
Line 35: def _create_socket(host, port):
Line 36: addr = socket.getaddrinfo(host, port, socket.AF_INET,
Line 37: socket.SOCK_STREAM)
Line 38: if not addr:
Line 39: raise Exception("Could not translate address '%s:%s'"
> raise socket.error
Done
Line 40: % (host, str(port)))
Line 41:
Line 42: server_socket = socket.socket(addr[0][0], addr[0][1], addr[0][2])
Line 43: filecontrol.set_close_on_exec(server_socket.fileno())
--
To view, visit https://gerrit.ovirt.org/37098
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If1cc2a205cdad335e8c3af4a77ed49c8977a79ba
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list