Nir Soffer has uploaded a new change for review. Change subject: protocoldetector: Introduce _close_connection ......................................................................
protocoldetector: Introduce _close_connection In http://gerrit.ovirt.org/33982 we learned that having to both remove a connection and close the socket is error prone. We have 4 places where we do this, and one other place where we remove a connection, and close the socket only if detection failed. This patch simplifies the rules and removes duplicate calls to self._remove_connection() followed by socket.close() by introducing _close_connection(). _close_connection() is used for all fatal errors, and _remove_connection() is used only when detection is successful. Having this method, we can clean up connection handling around protocol detection. Previously we used to remove the connection before trying to detect the protocol, and if protocol detection failed, the socket was closed. This also fixes logging, so we first report that a protocol was detected, and then report that the connection was removed. Change-Id: I522dbe8d6e0bccaa9e85dba3f28bed44d80988eb Signed-off-by: Nir Soffer <[email protected]> --- M vdsm/protocoldetector.py 1 file changed, 10 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/33988/1 diff --git a/vdsm/protocoldetector.py b/vdsm/protocoldetector.py index 3636e3a..ad2392b 100644 --- a/vdsm/protocoldetector.py +++ b/vdsm/protocoldetector.py @@ -134,8 +134,7 @@ self.log.debug("Cleaning Acceptor") for _, (_, client_socket) in self._pending_connections.items(): - self._remove_connection(client_socket) - client_socket.close() + self._close_connection(client_socket) self._poller.unregister(self._socket) self._poller.unregister(self._read_fd) @@ -146,8 +145,7 @@ def _cleanup_pending_connections(self): for _, (accepted, client_socket) in self._pending_connections.items(): if time.time() - accepted > self.CLEANUP_INTERVAL: - self._remove_connection(client_socket) - client_socket.close() + self._close_connection(client_socket) def detect_protocol(self, data): for handler in self._handlers: @@ -219,13 +217,16 @@ host, port = socket.getpeername() self.log.debug("Connection removed from %s:%d", host, port) + def _close_connection(self, socket): + self._remove_connection(socket) + socket.close() + def _process_handshake(self, socket): try: socket.is_handshaking = (socket.accept_ssl() == 0) except Exception as e: self.log.debug("Error during handshake: %s", e) - self._remove_connection(socket) - socket.close() + self._close_connection(socket) else: if not socket.is_handshaking: self._poller.modify(socket, self.READ_ONLY_MASK) @@ -246,24 +247,23 @@ except socket.error as e: if e.errno not in (errno.EAGAIN, errno.EWOULDBLOCK): self.log.warning("Unable to read data: %s", e) - self._remove_connection(client_socket) - client_socket.close() + self._close_connection(client_socket) return if data is None: # None is returned when ssl socket needs to read more data return - self._remove_connection(client_socket) try: handler = self.detect_protocol(data) except _CannotDetectProtocol: self.log.warning("Unrecognized protocol: %r", data) - client_socket.close() + self._close_connection(client_socket) else: host, port = client_socket.getpeername() self.log.debug("Detected protocol %s from %s:%d", handler.NAME, host, port) + self._remove_connection(client_socket) handler.handleSocket(client_socket, (host, port)) def _create_socket(self, host, port): -- To view, visit http://gerrit.ovirt.org/33988 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I522dbe8d6e0bccaa9e85dba3f28bed44d80988eb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
