Re: [PATCH 2/2] nbd/server: Allow users to adjust handshake limit in QMP

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On Wed, Feb 05, 2025 at 07:55:56AM +0100, Markus Armbruster wrote:
>> Eric Blake <eblake@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.

ACK

>> 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.

I like "seconds".

Thanks!