On 4/28/25 9:46 PM, Eric Blake wrote:
> From: "Richard W.M. Jones" <[email protected]>
>
> Add multi-conn option to the NBD client. This commit just adds the
> option, it is not functional.
>
> Setting this to a value > 1 permits multiple connections to the NBD
> server; a typical value might be 4. The default is 1, meaning only a
> single connection is made. If the NBD server does not advertise that
> it is safe for multi-conn then this setting is forced to 1.
>
> Signed-off-by: Richard W.M. Jones <[email protected]>
> [eblake: also expose it through QMP]
> Signed-off-by: Eric Blake <[email protected]>
> ---
> qapi/block-core.json | 8 +++++++-
> block/nbd.c | 24 ++++++++++++++++++++++++
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
Pardon my nitpicking, but it seems to me that the name "multi-conn"
doesn't really suggest "number of simultaneous NBD client connections",
especially when similarly named NBD_FLAG_CAN_MULTI_CONN_BIT represents
binary logic: supported or not supported. Maybe smth like
"multi_conns_num" would be better? Or anything else as long as it's
clear it's an int, not bool.
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7f70ec6d3cb..5c10824f35b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -4545,6 +4545,11 @@
> # until successful or until @open-timeout seconds have elapsed.
> # Default 0 (Since 7.0)
> #
> +# @multi-conn: Request the number of parallel client connections to make
> +# to the server, up to 16. If the server does not advertise support
> +# for multiple connections, or if this value is 0 or 1, all traffic
> +# is sent through a single connection. Default 1 (Since 10.1)
> +#
> # Features:
> #
> # @unstable: Member @x-dirty-bitmap is experimental.
> @@ -4558,7 +4563,8 @@
> '*tls-hostname': 'str',
> '*x-dirty-bitmap': { 'type': 'str', 'features': [ 'unstable' ] },
> '*reconnect-delay': 'uint32',
> - '*open-timeout': 'uint32' } }
> + '*open-timeout': 'uint32',
> + '*multi-conn': 'uint32' } }
>
> ##
> # @BlockdevOptionsRaw:
> diff --git a/block/nbd.c b/block/nbd.c
> index d5a2b21c6d1..5eb00e360af 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -48,6 +48,7 @@
>
> #define EN_OPTSTR ":exportname="
> #define MAX_NBD_REQUESTS 16
> +#define MAX_MULTI_CONN 16
>
> #define COOKIE_TO_INDEX(cookie) ((cookie) - 1)
> #define INDEX_TO_COOKIE(index) ((index) + 1)
> @@ -97,6 +98,7 @@ typedef struct BDRVNBDState {
> /* Connection parameters */
> uint32_t reconnect_delay;
> uint32_t open_timeout;
> + uint32_t multi_conn;
> SocketAddress *saddr;
> char *export;
> char *tlscredsid;
> @@ -1840,6 +1842,15 @@ static QemuOptsList nbd_runtime_opts = {
> "attempts until successful or until @open-timeout
> seconds "
> "have elapsed. Default 0",
> },
> + {
> + .name = "multi-conn",
> + .type = QEMU_OPT_NUMBER,
> + .help = "If > 1 permit up to this number of connections to the "
> + "server. The server must also advertise multi-conn "
> + "support. If <= 1, only a single connection is made "
> + "to the server even if the server advertises multi-conn.
> "
> + "Default 1",
> + },
> { /* end of list */ }
> },
> };
> @@ -1895,6 +1906,10 @@ static int nbd_process_options(BlockDriverState *bs,
> QDict *options,
>
> s->reconnect_delay = qemu_opt_get_number(opts, "reconnect-delay", 0);
> s->open_timeout = qemu_opt_get_number(opts, "open-timeout", 0);
> + s->multi_conn = qemu_opt_get_number(opts, "multi-conn", 1);
> + if (s->multi_conn > MAX_MULTI_CONN) {
> + s->multi_conn = MAX_MULTI_CONN;
> + }
>
I agree with Markus that this setting value different from what's been
directly requested by user shouldn't go silent. Having some kind of
warning at the very least would be nice.
> ret = 0;
>
> @@ -1949,6 +1964,15 @@ static int nbd_open(BlockDriverState *bs, QDict
> *options, int flags,
>
> nbd_client_connection_enable_retry(s->conn);
>
> + /*
> + * We set s->multi_conn in nbd_process_options above, but now that
> + * we have connected if the server doesn't advertise that it is
> + * safe for multi-conn, force it to 1.
> + */
> + if (!(s->info.flags & NBD_FLAG_CAN_MULTI_CONN)) {
> + s->multi_conn = 1;
> + }
Same here.
> +
> return 0;
>
> fail:
