Re: [Libguestfs] [PATCH nbdkit 1/7] server: Implement NBD_FLAG_CAN_MULTI_CONN.

Eric Blake
2019-05-15T18:31:57Z

On 1/4/19 4:08 PM, Richard W.M. Jones wrote:
>>From the protocol document:
>
>   "NBD_FLAG_CAN_MULTI_CONN: Indicates that the server operates
>   entirely without cache, or that the cache it uses is shared among
>   all connections to the given device. In particular, if this flag is
>   present, then the effects of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA MUST
>   be visible across all connections when the server sends its reply to
>   that command to the client. In the absence of this flag, clients
>   SHOULD NOT multiplex their commands over more than one connection to
>   the export."
>
> This is implemented in nbdkit by a boolean function which plugins may
> implement if they believe they satisfy the requirements above.  It can
> also be passed through or modified by filters.  It defaults to false,
> which is the safe default.
> ---
>  docs/nbdkit-filter.pod     |  4 ++++
>  docs/nbdkit-plugin.pod     | 20 ++++++++++++++++++++
>  docs/nbdkit-protocol.pod   |  4 ++++
>  include/nbdkit-filter.h    |  3 +++
>  include/nbdkit-plugin.h    |  2 ++
>  server/internal.h          |  1 +
>  server/protocol.h          |  1 +
>  server/connections.c       |  9 +++++++++
>  server/filters.c           | 24 ++++++++++++++++++++++++
>  server/plugins.c           | 17 +++++++++++++++++
>  tests/test-layers-filter.c | 10 ++++++++++
>  tests/test-layers-plugin.c |  8 ++++++++
>  tests/test-layers.c        |  6 ++++++
>  13 files changed, 109 insertions(+)

> +++ b/server/connections.c
> @@ -84,6 +84,7 @@ struct connection {
>    bool can_trim;
>    bool can_zero;
>    bool can_fua;
> +  bool can_multi_conn;
>    bool using_tls;
>
>    int sockin, sockout;
> @@ -463,6 +464,14 @@ compute_eflags (struct connection *conn, uint16_t
*flags)
>      conn->is_rotational = true;
>    }
>
> +  fl = backend->can_multi_conn (backend, conn);
> +  if (fl == -1)
> +    return -1;
> +  if (fl) {
> +    eflags |= NBD_FLAG_CAN_MULTI_CONN;
> +    conn->can_multi_conn = true;
> +  }

One more tweak: we should never advertise multi-conn if we are
serializing connections (a client attempting a second connection will
block on that connection, which defeats the point).  I'm applying this
followup:

+++ b/server/protocol-handshake.c
@@ -101,12 +101,16 @@ protocol_compute_eflags (struct connection *conn,
uint16_t *flags)
     conn->is_rotational = true;
   }

-  fl = backend->can_multi_conn (backend, conn);
-  if (fl == -1)
-    return -1;
-  if (fl) {
-    eflags |= NBD_FLAG_CAN_MULTI_CONN;
-    conn->can_multi_conn = true;
+  /* multi-conn is useless if parallel connections are not allowed */
+  if (backend->thread_model (backend) >
+      NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS) {
+    fl = backend->can_multi_conn (backend, conn);
+    if (fl == -1)
+      return -1;
+    if (fl) {
+      eflags |= NBD_FLAG_CAN_MULTI_CONN;
+      conn->can_multi_conn = true;
+    }
   }

   fl = backend->can_cache (backend, conn);
--
2.20.1



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