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

[Michal Privoznik]

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

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

Michal