Re: [PATCH 2/5] qemu-options: document keep-alive-count/idle/interval for -netdev stream

[Markus Armbruster <armbru@redhat.com>]

Mitsuru Kariya <Mitsuru.Kariya@oss.nttdata.com> writes:

> Thank you for reviewing, and I'm sorry for the late reply.
>
> Markus Armbruster <armbru@redhat.com> writes:
>> You document these options only for -netdev stream.  They also work
>> for -netdev dgram, possibly more (didn't check).  Shouldn't we
>> document them every where they work?
>
> As you pointed out, options such as keep-alive* are accepted by -netdev
> dgram as well. However, since these options are TCP-specific, they have
> no effect on sockets created by -netdev dgram.
> In fact, while these options are applied to sockets in
> inet_set_sockopts(), it is only called from inet_connect_saddr() and
> inet_listen_saddr(), which are for TCP, and not from inet_dgram_saddr(),
> which is for UDP. (I also confirmed this with strace.)
>
> Since the options are effectively no-ops there, I thought documenting
> them was unnecessary — but would you prefer that I add some note about
> it?

We should not accept options and then silently ignore them.

Can we reject them instead?

> For completeness, I also checked places other than -netdev stream and
> -incoming tcp:.
> For -blockdev driver=nbd and -drive driver=nbd, the keep-alive* options
> do take effect.  However, the documentation for -blockdev refers
> readers to the blockdev-add QMP command, and -drive is documented as
> accepting the same options as -blockdev, so I concluded that no
> additional description is needed there.
>
> As for -netdev socket, -drive file=nbd:, -chardev socket, and -vnc,
> they can also be used with TCP connections, but the keep-alive* options
> cannot be specified with them.

Thanks for checking!