[libvirt PATCH] virsh: guest-agent-timeout: set default value for optional argument

[Erik Skultety]

On Fri, Aug 21, 2020 at 02:34:51PM +0200, Tomáš Golembiovský wrote:
> The timeout argument for guest-agent-timeout is optional but it did not
> have proper default value specified. Also update the virsh man page
> accordingly.
>
> Signed-off-by: Tomáš Golembiovský <tgolembi at redhat.com>
> ---
>  docs/manpages/virsh.rst | 7 ++++---
>  tools/virsh-domain.c    | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 92de0b2192..6e48ae7973 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -2631,15 +2631,16 @@ guest
>
>  .. code-block::
>
> -   guest-agent-timeout domain --timeout value
> +   guest-agent-timeout domain [--timeout value]
>
>  Set how long to wait for a response from guest agent commands. By default,
>  agent commands block forever waiting for a response. ``value`` must be a
>  positive value (wait for given amount of seconds) or one of the following
>  values:
>
> -* -2 - block forever waiting for a result,
> -* -1 - reset timeout to the default value,
> +* -2 - block forever waiting for a result (used when --timeout is omitted),

--timeout could have been tagged with '*' for underscoring.

> +* -1 - reset timeout to the default value (currently defined as 5 seconds in

I agree that when "default" is mentioned we should document what the default is
otherwise it kinda loses the point to mention it. On the other hand, if we
change it in the future, users of the old libraries relying on the documented
value may be surprised it's no longer 5 seconds, so I'm a bit hesitant to
mention it in the manpage. Honestly, this is the kind of default value that IMO
doesn't make much sense to expose in the first place, either you're fine with
blocking forever or set a timeout yourself. If anything, we could mention that
one should set timeout explicitly if blocking is not acceptable.

Erik