From: Luiz Capitulino <lcapitulino@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] Re: [FOR 0.12 PATCH 18/18] QMP: add human-readable description to error response
Date: Tue, 8 Dec 2009 10:11:48 -0200 [thread overview]
Message-ID: <20091208101148.119bed20@doriath> (raw)
In-Reply-To: <1260218236-22143-19-git-send-email-armbru@redhat.com>
On Mon, 7 Dec 2009 21:37:16 +0100
Markus Armbruster <armbru@redhat.com> wrote:
> -{ "error": { "class": json-string, "data": json-value }, "id": json-value }
> +{ "error": { "class": json-string, "data": json-value, "desc": json-string },
> + "id": json-value }
>
> Where,
>
> - The "class" member contains the error class name (eg. "ServiceUnavailable")
> - The "data" member contains specific error data and is defined in a
> per-command basis, it will be an empty json-object if the error has no data
> +- The "desc" member is a human-readable error message. Clients should
> + not attempt to parse this message.
> - The "id" member contains the transaction identification associated with
> the command execution (if issued by the Client)
As we've talked on irc, I don't agree with this change.
Basically, adding 'desc' to the standard error message introduces all
the problems we've discussed about free-form English strings.
I feel that QError is becoming the worst of all proposals.
I agree with you that it's not as easy as it should be to report errors,
but as we're targeting on Clients I was convinced that we could not have the
best API internally but offer a good interface for Clients.
Now, having 'desc' as part of the standard protocol is like not having
the best API internally and offering a bad interface for Clients.
Not to mention that those strings can't be modified when the protocol
becomes stable and we're probably talking about dozens if not a hundred
of strings. Ok, there isn't a reason to change them often, but it's
still one more thing to maintain.
Having said that, I would agree to have 'desc' as part of debug
information. I have patches in my tree which adds CONFIG_DEBUG_QMP,
if one enables it information about the error location will also
be part of the error message. I would agree having 'desc' there
too.
next prev parent reply other threads:[~2009-12-08 12:12 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-07 20:36 [Qemu-devel] [FOR 0.12 PATCH 00/18] QError conversions and more Markus Armbruster
2009-12-07 20:36 ` [Qemu-devel] [FOR 0.12 PATCH 01/18] QError: new class for device encrypted errors Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 02/18] monitor: do_cont(): Don't ask for passwords Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 03/18] monitor: Fix double-prompt after "change vnc passwd BLA" Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 04/18] QError: Put error definitions in alphabetical order Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 05/18] QError: New QERR_DEVICE_LOCKED Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 06/18] QError: New QERR_DEVICE_NOT_REMOVABLE Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 07/18] monitor: convert do_eject() to QError Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 08/18] QError: New QERR_INVALID_BLOCK_FORMAT Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 09/18] QError: New QERR_SET_PASSWD_FAILED Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 10/18] QError: New QERR_VNC_SERVER_FAILED Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 11/18] monitor: convert do_change() to QObject, QError Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 12/18] QError: New QERR_FD_NOT_FOUND Markus Armbruster
2009-12-08 11:51 ` [Qemu-devel] " Luiz Capitulino
2009-12-08 12:25 ` Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 13/18] monitor: convert do_closefd() to QError Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 14/18] QError: New QERR_FD_NOT_SUPPLIED Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 15/18] New QERR_INVALID_PARAMETER Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 16/18] QError: New QERR_TOO_MANY_FILES Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 17/18] monitor: convert do_getfd() to QError Markus Armbruster
2009-12-07 20:37 ` [Qemu-devel] [FOR 0.12 PATCH 18/18] QMP: add human-readable description to error response Markus Armbruster
2009-12-08 12:11 ` Luiz Capitulino [this message]
2009-12-08 12:25 ` [Qemu-devel] " Daniel P. Berrange
2009-12-08 14:11 ` Luiz Capitulino
2009-12-08 14:50 ` Markus Armbruster
2009-12-08 12:38 ` Paolo Bonzini
2009-12-08 12:57 ` Markus Armbruster
2009-12-08 12:48 ` Markus Armbruster
2009-12-08 13:18 ` Anthony Liguori
2009-12-08 14:41 ` Luiz Capitulino
2009-12-08 11:49 ` [Qemu-devel] Re: [FOR 0.12 PATCH 00/18] QError conversions and more Luiz Capitulino
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20091208101148.119bed20@doriath \
--to=lcapitulino@redhat.com \
--cc=aliguori@us.ibm.com \
--cc=armbru@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).