[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

On 8/24/20 12:10 PM, Erik Skultety wrote:
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

Signed-off-by: Tomáš Golembiovský <tgolembi 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

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

Changing any default, even a documented one will always result in pain. If an app relies on a specific timeout value then the best would be to simply pass it. If the app trust us to chose a sensible default then it can omit the argument.

We document a lot of defaults, and a lot of them use "future proof" wording like "hypervisor default" so that we have our backs covered.

I think the documentation is correct (the default is 5 seconds, currently).


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]