vconn: Fix tracking of "connected" state.
authorBen Pfaff <blp@nicira.com>
Fri, 11 Jun 2010 22:58:14 +0000 (15:58 -0700)
committerBen Pfaff <blp@nicira.com>
Thu, 24 Jun 2010 19:40:20 +0000 (12:40 -0700)
While I was looking at the rconn code for connection backoff and retry, I
noticed that ovs-vswitchd was logging the following on each connection
attempt:

   Jun 11 15:17:41|00020|vconn_stream|ERR|send: Connection refused

The "send:" part didn't make much sense.  The configured controller was not
actually running, so the vconn code should not have been able to connect
at all, so the message should have been about a connection failing, not
about sending on a completed connection failing.

Investigation showed that different parts of the library have different
ideas about return value semantics.  vconn_open() and stream_open() both
return 0 if a connection succeeded or if one is in progress, but some of
its callers thought that it returned 0 if the connection succeeded and
EAGAIN if the connection was in progress.  This commit fixes up the callers
that had the wrong idea, by making them instead all vconn_connect() or
stream_connect() to determine whether the connection is complete.

lib/stream.c
lib/vconn-stream.c
lib/vconn.c
tests/test-vconn.c

index 667a23f..acbefc2 100644 (file)
@@ -240,14 +240,16 @@ stream_open_block(int error, struct stream **streamp)
 
     fatal_signal_run();
 
-    while (error == EAGAIN) {
-        stream_run(stream);
-        stream_run_wait(stream);
-        stream_connect_wait(stream);
-        poll_block();
-        error = stream_connect(stream);
+    if (!error) {
+        while ((error = stream_connect(stream)) == EAGAIN) {
+            stream_run(stream);
+            stream_run_wait(stream);
+            stream_connect_wait(stream);
+            poll_block();
+        }
         assert(error != EINPROGRESS);
     }
+
     if (error) {
         stream_close(stream);
         *streamp = NULL;
index 3d08874..df728d5 100644 (file)
@@ -85,13 +85,16 @@ vconn_stream_open(const char *name, char *suffix OVS_UNUSED,
 
     error = stream_open_with_default_ports(name, OFP_TCP_PORT, OFP_SSL_PORT,
                                            &stream);
-
-    if (error && error != EAGAIN) {
-        return error;
+    if (!error) {
+        error = stream_connect(stream);
+        if (!error || error == EAGAIN) {
+            *vconnp = vconn_stream_new(stream, error);
+            return 0;
+        }
     }
 
-    *vconnp = vconn_stream_new(stream, error);
-    return 0;
+    stream_close(stream);
+    return error;
 }
 
 static struct vconn_stream *
index f4b3169..b558f80 100644 (file)
@@ -278,14 +278,16 @@ vconn_open_block(const char *name, int min_version, struct vconn **vconnp)
     fatal_signal_run();
 
     error = vconn_open(name, min_version, &vconn);
-    while (error == EAGAIN) {
-        vconn_run(vconn);
-        vconn_run_wait(vconn);
-        vconn_connect_wait(vconn);
-        poll_block();
-        error = vconn_connect(vconn);
+    if (!error) {
+        while ((error == vconn_connect(vconn)) == EAGAIN) {
+            vconn_run(vconn);
+            vconn_run_wait(vconn);
+            vconn_connect_wait(vconn);
+            poll_block();
+        }
         assert(error != EINPROGRESS);
     }
+
     if (error) {
         vconn_close(vconn);
         *vconnp = NULL;
index 265bdec..adddc68 100644 (file)
@@ -141,7 +141,9 @@ test_refuse_connection(int argc OVS_UNUSED, char *argv[])
     struct fake_pvconn fpv;
     struct vconn *vconn;
 
-    expected_error = !strcmp(type, "unix") ? EPIPE : ECONNRESET;
+    expected_error = (!strcmp(type, "unix") ? EPIPE
+                      : !strcmp(type, "tcp") ? ECONNRESET
+                      : EPROTO);
 
     fpv_create(type, &fpv);
     CHECK_ERRNO(vconn_open(fpv.vconn_name, OFP_VERSION, &vconn), 0);