[ovs-dev] [PATCH] ovs python: ovs.stream.open_block() returns success even if the remote is unreachable
nusiddiq at redhat.com
nusiddiq at redhat.com
Mon Jul 2 16:15:42 UTC 2018
From: Numan Siddique <nusiddiq at redhat.com>
Calling ovs.stream.open_block(ovs.stream.open("tcp:127.0.0.1:6641")) returns
success even if there is no server listening on 6641. To check if the connection
is established or not, Stream class makes use of ovs.socket_util.check_connection_completion().
This function returns zero if the select for the socket fd signals. It doesn't
really check if the connection was established or not.
This patch fixes this issue by adding a wrapper function - check_connection_completion_status()
which calls sock.connect_ex() to get the status of the connection if
ovs.socket_util.check_connection_completion() returns success.
The test cases added fails without the fix in this patch.
Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
---
python/ovs/socket_util.py | 34 ++++++++++++++++++++++++++++++++++
python/ovs/stream.py | 16 +++++++++++++---
tests/ovsdb-idl.at | 16 ++++++++++++++++
tests/test-stream.py | 32 ++++++++++++++++++++++++++++++++
4 files changed, 95 insertions(+), 3 deletions(-)
create mode 100644 tests/test-stream.py
diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 403104936..91f4532ea 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -256,6 +256,40 @@ def inet_open_active(style, target, default_port, dscp):
return get_exception_errno(e), None
+def check_connection_completion_status(sock, target, default_port):
+ status = check_connection_completion(sock)
+ if status:
+ return status
+
+ try:
+ address = inet_parse_active(target, default_port)
+ except ValueError:
+ # It's not a valid tcp target.
+ return status
+
+ # For tcp connections, check_connection_completion function returns 0
+ # when the select for the socket fd signals. But it doesn't really
+ # verify the connection was established or not. So call connect again on
+ # the socket to get the status.
+ try:
+ err = sock.connect_ex(address)
+ except socket.error as e:
+ err = get_exception_errno(e)
+ if sys.platform == 'win32' and error == errno.WSAEWOULDBLOCK:
+ # WSAEWOULDBLOCK would be the equivalent on Windows
+ # for EINPROGRESS on Unix.
+ err = err.EINPROGRESS
+
+ if err == errno.EINPROGRESS or err == errno.EALREADY:
+ return errno.EINPROGRESS
+
+ if err == 0 or err == errno.EISCONN:
+ return 0
+
+ sock.close()
+ return err
+
+
def get_exception_errno(e):
"""A lot of methods on Python socket objects raise socket.error, but that
exception is documented as having two completely different forms of
diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index d6c447a97..617f31966 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -119,6 +119,7 @@ class Stream(object):
bInitialState=False)
self.name = name
+ self.suffix = name.split(":", 1)[1]
if status == errno.EAGAIN:
self.state = Stream.__S_CONNECTING
elif status == 0:
@@ -191,8 +192,16 @@ class Stream(object):
if error:
return error, None
else:
- status = ovs.socket_util.check_connection_completion(sock)
- return 0, cls(sock, name, status)
+ err = ovs.socket_util.check_connection_completion_status(
+ sock, suffix, 0)
+ if err == errno.EAGAIN or err == errno.EINPROGRESS:
+ status = errno.EAGAIN
+ err = 0
+ elif err == 0:
+ status = 0
+ else:
+ status = err
+ return err, cls(sock, name, status)
@staticmethod
def _open(suffix, dscp):
@@ -246,7 +255,8 @@ class Stream(object):
def __scs_connecting(self):
if self.socket is not None:
- retval = ovs.socket_util.check_connection_completion(self.socket)
+ retval = ovs.socket_util.check_connection_completion_status(
+ self.socket, self.suffix, 0)
assert retval != errno.EINPROGRESS
elif sys.platform == 'win32':
if self.retry_connect:
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 58935faf3..d4d283db4 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1720,3 +1720,19 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set, simple3 idl-compound-index-with-re
007: check simple4: empty
008: End test
]])
+
+m4_define([CHECK_STREAM_OPEN_BLOCK_PY],
+ [AT_SETUP([$1])
+ AT_SKIP_IF([test $2 = no])
+ AT_KEYWORDS([Check PY Stream open block - $3])
+ AT_CHECK([ovsdb_start_idltest "ptcp:0:127.0.0.1"])
+ PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
+ WRONG_PORT=$(($TCP_PORT+1))
+ AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [0], [ignore])
+ AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$WRONG_PORT], [1], [ignore])
+ OVSDB_SERVER_SHUTDOWN
+ AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [1], [ignore])
+ AT_CLEANUP])
+
+CHECK_STREAM_OPEN_BLOCK_PY([Check PY2 Stream open block], [$HAVE_PYTHON], [$PYTHON])
+CHECK_STREAM_OPEN_BLOCK_PY([Check PY3 Stream open block], [$HAVE_PYTHON], [$PYTHON3])
diff --git a/tests/test-stream.py b/tests/test-stream.py
new file mode 100644
index 000000000..4a5117501
--- /dev/null
+++ b/tests/test-stream.py
@@ -0,0 +1,32 @@
+# Copyright (c) 2018, Red Hat Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import sys
+
+import ovs.stream
+
+
+def main(argv):
+ remote = argv[1]
+ err, stream = ovs.stream.Stream.open_block(
+ ovs.stream.Stream.open(remote))
+
+ if err or stream is None:
+ sys.exit(1)
+
+ sys.exit(0)
+
+
+if __name__ == '__main__':
+ main(sys.argv)
--
2.17.1
More information about the dev
mailing list