[Libguestfs] [PATCH libnbd v2 1/6] api: Synchronous connect waits til all connections are connected.

[Richard W.M. Jones]

If not using multi-conn then obviously the synchronous connection
calls ‘nbd_connect_unix’, ‘nbd_connect_tcp’ and ‘nbd_connect_command’
should only return when the (one) connection object is connected.

In the multi-conn case it's not very clear what these synchronous
calls should do.  Previously I had it so that they would return as
soon as at least one connection was connected.  However this is a
problem if you are using them as a convenient way to set up a
multi-threaded main loop, because it can be that some of them have not
finished connecting, but then you issue commands on those connections
and that will fail.  The failure is pernicious because most of the
time you won't see it, only if one connection is slow.  So it's
(probably) better that the synchronous ‘nbd_connect_unix’ and
‘nbd_connect_tcp’ should connect every connection object before
returning.

For ‘nbd_connect_command’, it essentially ignored multi-conn anyway,
and I changed it so that it waits for conn[0] to get connected and
returns, the other connections (if they exist) being ignored.  It
should probably be an error for the user to enable multi-conn on the
handle and then use nbd_connect_command, but I did not make that
change yet.
---
 generator/generator |  8 +++-----
 lib/connect.c       | 48 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/generator/generator b/generator/generator
index 98897be..4492f9d 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1040,8 +1040,7 @@ C<\"qemu:dirty-bitmap:...\"> for qemu-nbd
 Connect (synchronously) over the named Unix domain socket (C<sockpath>)
 to an NBD server running on the same machine.  This call returns
 when the connection has been made.  If multi-conn is enabled, this
-begins connecting all of the connections, and returns as soon as
-any one of them is connected.";
+returns when all of the connections are connected.";
   };
 
   "connect_tcp", {
@@ -1053,9 +1052,8 @@ Connect (synchronously) to the NBD server listening on
 C<hostname:port>.  The C<port> may be a port name such
 as C<\"nbd\">, or it may be a port number as a string
 such as C<\"10809\">.  This call returns when the connection
-has been made.  If multi-conn is enabled, this begins connecting
-all of the connections, and returns as soon as
-any one of them is connected.";
+has been made.  If multi-conn is enabled, this returns when
+all of the connections are connected.";
   };
 
   "connect_command", {
diff --git a/lib/connect.c b/lib/connect.c
index 0fef87d..d711fd7 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -33,22 +33,32 @@
 #include "internal.h"
 
 static int
-wait_one_connected (struct nbd_handle *h)
+wait_all_connected (struct nbd_handle *h)
 {
   size_t i;
 
   for (;;) {
-    bool connected = false;
+    bool all_connected = true;
 
-    /* Are any connected? */
+    /* Are any not yet connected? */
     for (i = 0; i < h->multi_conn; ++i) {
-      if (nbd_unlocked_aio_is_ready (h->conns[i])) {
-        connected = true;
+      if (nbd_unlocked_aio_is_closed (h->conns[i])) {
+        set_error (0, "connection is closed");
+        return -1;
+      }
+      if (nbd_unlocked_aio_is_dead (h->conns[i])) {
+        /* Don't set the error here, keep the error set when
+         * the connection died.
+         */
+        return -1;
+      }
+      if (!nbd_unlocked_aio_is_ready (h->conns[i])) {
+        all_connected = false;
         break;
       }
     }
 
-    if (connected)
+    if (all_connected)
       break;
 
     if (nbd_unlocked_poll (h, -1) == -1)
@@ -59,7 +69,7 @@ wait_one_connected (struct nbd_handle *h)
 }
 
 /* For all connections in the CREATED state, start connecting them to
- * a Unix domain socket.  Wait until at least one is in the READY
+ * a Unix domain socket.  Wait until all connections are in the READY
  * state.
  */
 int
@@ -90,7 +100,7 @@ nbd_unlocked_connect_unix (struct nbd_handle *h, const char *sockpath)
     return -1;
   }
 
-  return wait_one_connected (h);
+  return wait_all_connected (h);
 }
 
 /* Connect to a TCP port. */
@@ -115,7 +125,7 @@ nbd_unlocked_connect_tcp (struct nbd_handle *h,
     return -1;
   }
 
-  return wait_one_connected (h);
+  return wait_all_connected (h);
 }
 
 /* Connect to a local command.  Multi-conn doesn't make much sense
@@ -132,7 +142,25 @@ nbd_unlocked_connect_command (struct nbd_handle *h, char **argv)
   if (nbd_unlocked_aio_connect_command (h->conns[0], argv) == -1)
     return -1;
 
-  return wait_one_connected (h);
+  for (;;) {
+    if (nbd_unlocked_aio_is_closed (h->conns[0])) {
+      set_error (0, "connection is closed");
+      return -1;
+    }
+    if (nbd_unlocked_aio_is_dead (h->conns[0])) {
+      /* Don't set the error here, keep the error set when
+       * the connection died.
+       */
+      return -1;
+    }
+    if (nbd_unlocked_aio_is_ready (h->conns[0]))
+      break;
+
+    if (nbd_unlocked_poll (h, -1) == -1)
+      return -1;
+  }
+
+  return 0;
 }
 
 int
-- 
2.21.0