On 09.08.24 19:14, Eric Blake wrote:
Although defaulting the handshake limit to 10 seconds was a nice QoI
change to weed out intentionally slow clients, it can interfere with
integration testing done with manual NBD_OPT commands over 'nbdsh
--opt-mode'. Expose a QMP knob 'handshake-max-secs' to allow the user
to alter the timeout away from the default.
The parameter name here intentionally matches the spelling of the
constant added in commit fb1c2aaa98, and not the command-line spelling
added in the previous patch for qemu-nbd; that's because in QMP,
longer names serve as good self-documentation, and unlike the command
line, machines don't have problems generating longer spellings.
Signed-off-by: Eric Blake <ebl...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
qapi/block-export.json | 10 ++++++++++
include/block/nbd.h | 6 +++---
block/monitor/block-hmp-cmds.c | 4 ++--
blockdev-nbd.c | 26 ++++++++++++++++++--------
4 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/qapi/block-export.json b/qapi/block-export.json
index ce33fe378df..c110dd375ad 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -17,6 +17,10 @@
#
# @addr: Address on which to listen.
#
+# @handshake-max-secs: Time limit, in seconds, at which a client that
+# has not completed the negotiation handshake will be disconnected,
+# or 0 for no limit (since 9.2; default: 10).
+#
# @tls-creds: ID of the TLS credentials object (since 2.6).
#
# @tls-authz: ID of the QAuthZ authorization object used to validate
@@ -34,6 +38,7 @@
##
{ 'struct': 'NbdServerOptions',
'data': { 'addr': 'SocketAddress',
+ '*handshake-max-secs': 'uint32',
'*tls-creds': 'str',
'*tls-authz': 'str',
'*max-connections': 'uint32' } }
@@ -52,6 +57,10 @@
#
# @addr: Address on which to listen.
#
+# @handshake-max-secs: Time limit, in seconds, at which a client that
+# has not completed the negotiation handshake will be disconnected,
+# or 0 for no limit (since 9.2; default: 10).
+#
# @tls-creds: ID of the TLS credentials object (since 2.6).
#
# @tls-authz: ID of the QAuthZ authorization object used to validate
@@ -72,6 +81,7 @@
##
{ 'command': 'nbd-server-start',
'data': { 'addr': 'SocketAddressLegacy',
+ '*handshake-max-secs': 'uint32',
'*tls-creds': 'str',
'*tls-authz': 'str',
'*max-connections': 'uint32' },
Hmm, can we make common base for these two structures, to avoid adding things
always in two places? At some point would be good to somehow deprecate old
syntax and finally remove it. SocketAddressLegacy is marked as deprecated in
comment in qapi/sockets.json, but no word in deprecated.rst, and I'm unsure how
is it possible to deprecate this.. May be, the only way is introducing new
command, and deprecate nbd-server-start altogether. Aha, that should happen
when add a possibility to start multiple nbd servers.
--
Best regards,
Vladimir
