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 <ben...@irqsave.net> > --- > 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