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

Patch 1/6 was posted before and I didn't change it:

  https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00134

That doesn't necessarily mean I shouldn't change it, I'm posting
it again because the other patches depend on it.

The main change in this series is we add three new API functions:

nbd_aio_is_created    - connection has just been created
nbd_aio_is_connecting - is connecting or handshaking / option negotiation
nbd_aio_is_processing - is issuing or receiving a reply

Also in this change I used nbd_aio_is_connecting to simplify the
handling of synchronous connects.

Rich.

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

Simple refactoring adding a new API.
---
 generator/generator | 11 +++++++++++
 lib/aio.c           |  6 ++++++
 lib/connect.c       |  8 ++++----
 lib/rw.c            |  4 ++--
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/generator/generator b/generator/generator
index 4492f9d..be37fd1 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1537,6 +1537,17 @@ has detected that the file descriptor associated with
this
 connection is writable.";
   };
 
+  "aio_is_created", {
+    default_call with
+    args = []; ret = RBool;
+    shortdesc = "check if the connection has just been created";
+    longdesc = "\
+Return true if this connection has just been created.  This
+is the state before the connection object has started
+connecting to a server.  In this state the handle can start
+to be connected by calling functions such as C<nbd_aio_connect>.";
+  };
+
   "aio_is_ready", {
     default_call with
     args = []; ret = RBool;
diff --git a/lib/aio.c b/lib/aio.c
index d8458ed..79c89d5 100644
--- a/lib/aio.c
+++ b/lib/aio.c
@@ -47,6 +47,12 @@ nbd_unlocked_aio_notify_write (struct nbd_connection *conn)
   return nbd_internal_run (conn->h, conn, notify_write);
 }
 
+int
+nbd_unlocked_aio_is_created (struct nbd_connection *conn)
+{
+  return conn->state == STATE_START;
+}
+
 int
 nbd_unlocked_aio_is_ready (struct nbd_connection *conn)
 {
diff --git a/lib/connect.c b/lib/connect.c
index d711fd7..a5a262d 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -68,7 +68,7 @@ wait_all_connected (struct nbd_handle *h)
   return 0;
 }
 
-/* For all connections in the CREATED state, start connecting them to
+/* For all connections in the initial state, start connecting them to
  * a Unix domain socket.  Wait until all connections are in the READY
  * state.
  */
@@ -87,7 +87,7 @@ nbd_unlocked_connect_unix (struct nbd_handle *h, const char
*sockpath)
 
   started = false;
   for (i = 0; i < h->multi_conn; ++i) {
-    if (h->conns[i]->state == STATE_START) {
+    if (nbd_unlocked_aio_is_created (h->conns[i])) {
       if (nbd_unlocked_aio_connect (h->conns[i],
                                     (struct sockaddr *) &sun, len) == -1)
         return -1;
@@ -113,7 +113,7 @@ nbd_unlocked_connect_tcp (struct nbd_handle *h,
 
   started = false;
   for (i = 0; i < h->multi_conn; ++i) {
-    if (h->conns[i]->state == STATE_START) {
+    if (nbd_unlocked_aio_is_created (h->conns[i])) {
       if (nbd_unlocked_aio_connect_tcp (h->conns[i], hostname, port) == -1)
         return -1;
       started = true;
@@ -134,7 +134,7 @@ nbd_unlocked_connect_tcp (struct nbd_handle *h,
 int
 nbd_unlocked_connect_command (struct nbd_handle *h, char **argv)
 {
-  if (h->conns[0]->state != STATE_START) {
+  if (!nbd_unlocked_aio_is_created (h->conns[0])) {
     set_error (0, "first connection in this handle is not in the created
state, this is likely to be caused by a programming error in the calling
program");
     return -1;
   }
diff --git a/lib/rw.c b/lib/rw.c
index 9dfce97..861ab67 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -46,9 +46,9 @@ pick_connection (struct nbd_handle *h)
       break;
     }
     /* At least one connection is busy, not dead and not sitting in
-     * the initial CREATED state.
+     * the initial state.
      */
-    if (h->conns[i]->state != STATE_START &&
+    if (!nbd_unlocked_aio_is_created (h->conns[i]) &&
         !nbd_unlocked_aio_is_dead (h->conns[i]))
       error = EBUSY;
 
-- 
2.21.0

---
 generator/generator | 35 ++++++++++++++++++++++++++++++++++-
 lib/internal.h      |  1 +
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/generator/generator b/generator/generator
index be37fd1..f665475 100755
--- a/generator/generator
+++ b/generator/generator
@@ -2209,7 +2209,40 @@ let generate_lib_states_c ()    pr " 
default:\n";
   pr "    abort (); /* Should never happen, but keeps GCC happy.
*/\n";
   pr "  }\n";
-  pr "}\n"
+  pr "}\n";
+  pr "\n";
+
+  pr "/* Map a state group to its parent group. */\n";
+  pr "enum state_group\n";
+  pr "nbd_internal_state_group_parent (enum state_group group)\n";
+  pr "{\n";
+  pr "  switch (group) {\n";
+  pr "  case GROUP_TOP:\n";
+  pr "    return GROUP_TOP;\n";
+  let rec loop prefix = function
+    | [] -> ()
+    | State _ :: rest ->
+       loop prefix rest
+    | Group (name, group) :: rest ->
+       let enum +         "GROUP" ^ String.concat ""
(List.map ((^) "_") prefix) ^ "_" ^ name in
+       pr "  case %s:\n" enum;
+       if prefix = [] then
+         pr "    return GROUP_TOP;\n"
+       else (
+         let parent = "GROUP" ^ String.concat "" (List.map
((^) "_") prefix) in
+         pr "    return %s;\n" parent
+       );
+       loop (prefix @ [name]) group;
+       loop prefix rest
+  in
+  loop [] state_machine;
+  pr "  default:\n";
+  pr "    abort (); /* Should never happen, but keeps GCC happy.
*/\n";
+  pr "  }\n";
+  pr "};\n"
+
+
 
 (*----------------------------------------------------------------------*)
 
diff --git a/lib/internal.h b/lib/internal.h
index 3f2b729..1f742da 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -273,6 +273,7 @@ extern int nbd_internal_run (struct nbd_handle *h, struct
nbd_connection *conn,
                              enum external_event ev);
 extern const char *nbd_internal_state_short_string (enum state state);
 extern enum state_group nbd_internal_state_group (enum state state);
+extern enum state_group nbd_internal_state_group_parent (enum state_group
group);
 
 /* utils.c */
 extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp);
-- 
2.21.0

---
 generator/generator | 37 ++++++++++++++++++++++++++++---------
 lib/aio.c           | 26 ++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/generator/generator b/generator/generator
index f665475..d31f060 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1344,9 +1344,12 @@ let connection_calls = [
     args = [ SockAddrAndLen ("addr", "addrlen") ]; ret =
RErr;
     shortdesc = "connect to the NBD server";
     longdesc = "\
-Begin connecting to the NBD server.  You can check if the
-connection has connected to the server and completed the NBD
-handshake by calling C<nbd_aio_is_ready> on the connection.";
+Begin connecting to the NBD server.
+
+You can check if the connection is still connecting by calling
+C<nbd_aio_is_connecting>, or if it has connected to the server
+and completed the NBD handshake by calling C<nbd_aio_is_ready>,
+on the connection.";
   };
 
   "aio_connect_tcp", {
@@ -1355,9 +1358,11 @@ handshake by calling C<nbd_aio_is_ready> on the
connection.";
     shortdesc = "connect to the NBD server over a TCP port";
     longdesc = "\
 Begin connecting to the NBD server listening on C<hostname:port>.
-You can check if the connection has connected to the server and
-completed the NBD handshake by calling C<nbd_aio_is_ready> on
-the connection.";
+
+You can check if the connection is still connecting by calling
+C<nbd_aio_is_connecting>, or if it has connected to the server
+and completed the NBD handshake by calling C<nbd_aio_is_ready>,
+on the connection.";
   };
 
   "aio_connect_command", {
@@ -1366,9 +1371,12 @@ the connection.";
     shortdesc = "connect to the NBD server";
     longdesc = "\
 Run the command as a subprocess and begin connecting to it over
-stdin/stdout.  You can check if the connection has connected to
-the server and completed the NBD handshake by calling
-C<nbd_aio_is_ready> on the connection.";
+stdin/stdout.
+
+You can check if the connection is still connecting by calling
+C<nbd_aio_is_connecting>, or if it has connected to the server
+and completed the NBD handshake by calling C<nbd_aio_is_ready>,
+on the connection.";
   };
 
   "aio_pread", {
@@ -1548,6 +1556,17 @@ connecting to a server.  In this state the handle can
start
 to be connected by calling functions such as C<nbd_aio_connect>.";
   };
 
+  "aio_is_connecting", {
+    default_call with
+    args = []; ret = RBool;
+    shortdesc = "check if the connection is connecting or
handshaking";
+    longdesc = "\
+Return true if this connection is connecting to the server
+or in the process of handshaking and negotiating options
+which happens before the connection object becomes ready to
+issue commands (see C<nbd_aio_is_ready>).";
+  };
+
   "aio_is_ready", {
     default_call with
     args = []; ret = RBool;
diff --git a/lib/aio.c b/lib/aio.c
index 79c89d5..1152c9e 100644
--- a/lib/aio.c
+++ b/lib/aio.c
@@ -53,6 +53,32 @@ nbd_unlocked_aio_is_created (struct nbd_connection *conn)
   return conn->state == STATE_START;
 }
 
+static int
+is_connecting_group (enum state_group group)
+{
+  switch (group) {
+  case GROUP_TOP:
+    return 0;
+  case GROUP_CONNECT:
+  case GROUP_CONNECT_TCP:
+  case GROUP_CONNECT_COMMAND:
+  case GROUP_MAGIC:
+  case GROUP_OLDSTYLE:
+  case GROUP_NEWSTYLE:
+    return 1;
+  default:
+    return is_connecting_group (nbd_internal_state_group_parent (group));
+  }
+}
+
+int
+nbd_unlocked_aio_is_connecting (struct nbd_connection *conn)
+{
+  enum state_group group = nbd_internal_state_group (conn->state);
+
+  return is_connecting_group (group);
+}
+
 int
 nbd_unlocked_aio_is_ready (struct nbd_connection *conn)
 {
-- 
2.21.0

---
 generator/generator | 14 ++++++++++++++
 lib/aio.c           | 22 ++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/generator/generator b/generator/generator
index d31f060..60dfb72 100755
--- a/generator/generator
+++ b/generator/generator
@@ -1578,6 +1578,20 @@ waiting for a reply.  In this state the handle is ready
to
 issue commands.";
   };
 
+  "aio_is_processing", {
+    default_call with
+    args = []; ret = RBool;
+    shortdesc = "check if the connection is processing a command";
+    longdesc = "\
+Return true if this connection is connected to the NBD server,
+the handshake has completed, and the connection is processing
+commands (either writing out a request or reading a reply).
+
+Note the ready state (C<nbd_aio_is_ready>) is not included.
+In the ready state commands may be I<in flight> (the I<server>
+is processing them), but libnbd is not processing them.";
+  };
+
   "aio_is_dead", {
     default_call with
     args = []; ret = RBool;
diff --git a/lib/aio.c b/lib/aio.c
index 1152c9e..c7764f8 100644
--- a/lib/aio.c
+++ b/lib/aio.c
@@ -85,6 +85,28 @@ nbd_unlocked_aio_is_ready (struct nbd_connection *conn)
   return conn->state == STATE_READY;
 }
 
+static int
+is_processing_group (enum state_group group)
+{
+  switch (group) {
+  case GROUP_TOP:
+    return 0;
+  case GROUP_ISSUE_COMMAND:
+  case GROUP_REPLY:
+    return 1;
+  default:
+    return is_processing_group (nbd_internal_state_group_parent (group));
+  }
+}
+
+int
+nbd_unlocked_aio_is_processing (struct nbd_connection *conn)
+{
+  enum state_group group = nbd_internal_state_group (conn->state);
+
+  return is_processing_group (group);
+}
+
 int
 nbd_unlocked_aio_is_dead (struct nbd_connection *conn)
 {
-- 
2.21.0

The new API nbd_aio_is_connecting allows us to more accurately check
if a connection is still connecting/handshaking.  This change also
checks all of the connections reached the ready state (and not an
error state), and handles that check through a single common function.
---
 lib/connect.c | 67 +++++++++++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 29 deletions(-)

diff --git a/lib/connect.c b/lib/connect.c
index a5a262d..c5970d2 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -32,39 +32,61 @@
 
 #include "internal.h"
 
+static int
+error_unless_ready (struct nbd_connection *conn)
+{
+  if (nbd_unlocked_aio_is_ready (conn))
+    return 0;
+
+  /* Why did it fail? */
+  if (nbd_unlocked_aio_is_closed (conn)) {
+    set_error (0, "connection is closed");
+    return -1;
+  }
+
+  if (nbd_unlocked_aio_is_dead (conn))
+    /* Don't set the error here, keep the error set when
+     * the connection died.
+     */
+    return -1;
+
+  /* Should probably never happen. */
+  set_error (0, "connection in an unexpected state (%s)",
+             nbd_internal_state_short_string (conn->state));
+  return -1;
+}
+
 static int
 wait_all_connected (struct nbd_handle *h)
 {
   size_t i;
 
   for (;;) {
-    bool all_connected = true;
+    bool all_done = true;
 
     /* Are any not yet connected? */
     for (i = 0; i < h->multi_conn; ++i) {
-      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;
+      if (nbd_unlocked_aio_is_connecting (h->conns[i])) {
+        all_done = false;
         break;
       }
     }
 
-    if (all_connected)
+    if (all_done)
       break;
 
     if (nbd_unlocked_poll (h, -1) == -1)
       return -1;
   }
 
+  /* All connections should be in the READY state, unless there was an
+   * error on one of them.
+   */
+  for (i = 0; i < h->multi_conn; ++i) {
+    if (error_unless_ready (h->conns[i]) == -1)
+      return -1;
+  }
+
   return 0;
 }
 
@@ -142,25 +164,12 @@ nbd_unlocked_connect_command (struct nbd_handle *h, char
**argv)
   if (nbd_unlocked_aio_connect_command (h->conns[0], argv) == -1)
     return -1;
 
-  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;
-
+  while (nbd_unlocked_aio_is_connecting (h->conns[0])) {
     if (nbd_unlocked_poll (h, -1) == -1)
       return -1;
   }
 
-  return 0;
+  return error_unless_ready (h->conns[0]);
 }
 
 int
-- 
2.21.0

On 5/22/19 4:50 AM, Richard W.M. Jones wrote:
> Patch 1/6 was posted before and I didn't change it:
> 
>   https://www.redhat.com/archives/libguestfs/2019-May/thread.html#00134
> 
> That doesn't necessarily mean I shouldn't change it, I'm
posting
> it again because the other patches depend on it.
> 
> The main change in this series is we add three new API functions:
> 
> nbd_aio_is_created    - connection has just been created
> nbd_aio_is_connecting - is connecting or handshaking / option negotiation
> nbd_aio_is_processing - is issuing or receiving a reply
> 
> Also in this change I used nbd_aio_is_connecting to simplify the
> handling of synchronous connects.
Series LGTM

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 5/22/19 4:50 AM, Richard W.M. Jones wrote:
> 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.
Ouch - I just realized that we created a different problem. If the
server does not add multi-conn because it does NOT permit parallel
connections (such as 'nbdkit --filter=noparallel memory 1m
serialize=connections'), then a client requesting
nbd_set_multi_conn(nbd, 2) will now hang because we will never be able
to move all our connections through handshake. While we can still try to
fire off multiple connections at once, we need to add some logic in
nbd_connect_unix() and nbd_connect_tcp() to check can_multi_conn after
the FIRST connection completes (whichever one wins), and if
!can_multi_conn(), it would be nice if we could automatically kill off
the losers, swap the winning connection into conns[0], and then behave
as if we implicitly called set_multi_conn(nbd, 1), so that the
synchronous connect command will succeed after all. In such a case, the
user can learn after the fact via nbd_can_multi_conn and
nbd_get_multi_conn that things were changed because multi_conn was not
supported after all.

Or maybe we want both 'set_multi_conn' (connect hard-fails if the FIRST
connection reports !can_multi_conn) vs. 'request_multi_conn' (connect
downgrades gracefully), similar to how request_meta_context downgrades
gracefully if the server lacks SR or a particular meta context.

On IRC we were chatting about how multi-conn > 1 makes no sense with
connect_command() (where we are limited to exactly one channel over
stdin/stdout), so it may also be worth adding some logic to have
connect_command fail if set_multi_conn is > 1, and to likewise have
set_multi_conn(2) fail if the existing connection is already started in
command mode (do we even track which mode a commands started in, and/or
insist that all connections used the same nbd_connect_*? Or can you
mix-and-match a TCP and a Unix socket into a multi-conn struct nbd_handle?)

On the other hand, a user that is aware of multi-conn being optional may
manually try to make one connection, check can_multi_conn, and THEN call
set_multi_conn(2), with some expectation of being able to then connect
the remaining slots (again, if we stored the nbd_connect_* used by the
FIRST connection, then all the remaining connections requested could
automatically be fired off to start connecting to the same server - and
you'd need a synchronous way to wait for them all to reach stable
state).  Perhaps that's even a reason to have two separate API - one
that is callable only while you are in created state (a request of what
to try if the server supports it), and the other callable only after
your single connection has completed handshake (to then expand to
actually get the additional connections now that it is known to be
safe).  At any rate, I don't think we're done with the design in this
area.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org