[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/3] qmp: Make Quorum error events more palatabl
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 2/3] qmp: Make Quorum error events more palatable. |
Date: |
Fri, 21 Feb 2014 17:17:24 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 02/21/2014 04:58 PM, Benoît Canet wrote:
> Insert quorum QMP events documentation alphabetically.
>
> Also change the "ret" errno value by "error" being an strerror() in the
> QUORUM_REPORT_BAD qmp event.
>
> Signed-off-by: Benoit Canet <address@hidden>
> ---
> block/quorum.c | 5 ++--
> docs/qmp/qmp-events.txt | 72
> +++++++++++++++++++++++-----------------------
> tests/qemu-iotests/081.out | 2 +-
> 3 files changed, 40 insertions(+), 39 deletions(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 6c28239..3ef8eaa 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -200,11 +200,12 @@ static void quorum_report_bad(QuorumAIOCB *acb, char
> *node_name, int ret)
> {
> QObject *data;
> assert(node_name);
> - data = qobject_from_jsonf("{ 'ret': %d"
> + data = qobject_from_jsonf("{ 'error': %s"
> ", 'node-name': %s"
> ", 'sector-num': %" PRId64
> ", 'sectors-count': %d }",
> - ret, node_name, acb->sector_num,
> acb->nb_sectors);
> + strerror(ret), node_name, acb->sector_num,
Is ret positive or negative? Our typical convention is to use -errno
values for return (since we prefer reporting errors as less than 0),
which may mean needing strerror(-ret) here.
Also, I'd suggest making 'error' optional, and only omit it when ret is
non-zero. Otherwise...
> +{ "event": "QUORUM_REPORT_BAD",
> + "data": { "error": "Success", "node-name": "1.raw", "sector-num":
> 345435,
we get these weird-looking "error": "Success" strings. Look at how
BLOCK_JOB_COMPLETED documents things - I'd just lift that full paragraph:
- "error": Error message (json-string, optional)
Only present on failure. This field contains a human-readable
error message. There are no semantics other than that
streaming
has failed and clients should not try to interpret the error
string.
(When we eventually add events into qapi-schema.json as a first-class
citizen, it will be listed as '*error':'str')
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature