On Wed, Feb 05, 2025 at 07:55:56AM +0100, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
> > 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.
> >
> > { 'struct': 'NbdServerOptions',
> > 'data': { 'addr': 'SocketAddress',
> > + '*handshake-max-secs': 'uint32',
> > '*tls-creds': 'str',
> > '*tls-authz': 'str',
> > '*max-connections': 'uint32' } }
>
> Standard question on time: are we confident the granularity will
> suffice?
The default if this is unspecified is 10 seconds. Anything subsecond
requires a fast negotiation between client and server; the more likely
use of this parameter is setting it extremely large (or to 0 to
disable) in order to allow lengthy gdb sessions without the timeout
cutting things short. So I think seconds is okay.
>
> On naming... We use "seconds" (StatsUnit in qapi/stats.json), and "sec"
> (SnapshotInfo in qapi/block-core.json), but not "secs". Do we care?
Avoiding abbreviations and using 'seconds' is fine by me, especially
since this won't be typed by many users but remains more of a tool for
use in a debugging arsenel. I can respin with the longer name, but
will wait for any other review comments first.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
