Re: [Qemu-devel] [PATCH 4/4] qapi: new qmp command nbd-server-add-bitmap

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

22.03.2018 19:19, Eric Blake wrote:
> On 03/22/2018 10:43 AM, Vladimir Sementsov-Ogievskiy wrote:
>
>>>> +# Returns: error on one of the following conditions:
>>>> +#           - the server is not running
>>>> +#           - export is not found
>>>> +#           - bitmap is not found
>>>> +#           - bitmap is disabled
>>>> +#           - bitmap is locked
>>>
>>> Do we really need to list all the error conditions?  My worry is 
>>> that a list this specific might go stale, compared to the obvious 
>>> default that the command succeeds only if it was able to expose the 
>>> bitmap and that the error message is specific enough for a human to 
>>> figure out what to fix if it failed.
>>
>>
>> Hmm, I've just doing it similar with other commands in the file. Is 
>> there any requirements on this part of qapi documentation? I can 
>> write only "# Returns: nothing on success" line, is it appropriate? 
>> blockdev-mirror do so, but other commands tries to describe errors. 
>> Looks like we lack some specified format for return code description 
>> like we have for parameters..
>
> Yeah, for returns, it's been very ad hoc.  My personal feel (although 
> it's not very well documented and certainly not enforced): all 
> commands can reasonably return errors, presumably for a good reason; 
> but exhaustively auditing WHICH errors is a huge task with little 
> benefits. A few commands return non-generic errors, but if all error 
> paths used error_setg(), there's nothing that a machine can do to tell 
> the difference between the errors, so documenting different error 
> reasons doesn't add much.
>
> Thus, if a command returns nothing on success, I'm fine with omitting 
> 'Returns:' entirely, and the doc generator permits that. But if you 
> have bothered to list Returns: for certain errors, I'm not going to 
> blindly throw away the documentation work, even though the list may 
> become incomplete over time.
>

So, only something interesting worth documenting.

Hmm, interesting: consider for example bloc-dirty-bitmap-remove. It says:
# Returns: nothing on success
#          If @node is not a valid block device or node, DeviceNotFound

But the code uses bdrv_lookup_bs, which uses simple error_setg, so it 
will return GenericError, isn't it? And, it's not the only place..

-- 
Best regards,
Vladimir