[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 4/4] qapi: new qmp command nbd-server-add-bitmap
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH 4/4] qapi: new qmp command nbd-server-add-bitmap |
Date: |
Thu, 22 Mar 2018 19:45:21 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
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
[Qemu-block] [PATCH 2/4] nbd/server: add nbd_meta_single_query helper, Vladimir Sementsov-Ogievskiy, 2018/03/21
[Qemu-block] [PATCH 1/4] nbd/server: refactor nbd_negotiate_meta_query for several namespaces, Vladimir Sementsov-Ogievskiy, 2018/03/21
[Qemu-block] [PATCH 3/4] nbd/server: implement dirty bitmap export, Vladimir Sementsov-Ogievskiy, 2018/03/21