[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 29/34] qmp: switch to the new error format on th
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 29/34] qmp: switch to the new error format on the wire |
Date: |
Thu, 02 Aug 2012 19:12:03 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Luiz Capitulino <address@hidden> writes:
> IMPORTANT: this BREAKS QMP's compatibility for the error response.
>
> This commit changes QMP's wire protocol to make use of the simpler
> error format introduced by previous commits.
>
> There are two important (and mostly incompatible) changes:
>
> 1. Almost all error classes have been replaced by GenericError. The
> only classes that are still supported for compatibility with
> libvirt are: CommandNotFound, DeviceNotActive, KVMMissingCap,
> DeviceNotFound and MigrationExpected
>
> 2. The 'data' field of the error dictionary is gone
Making use of license from qmp-commands.hx:
3. Errors, in special, are not documented. Applications should NOT check
for specific errors classes or data (it's strongly recommended to only
check for the "error" key)
> As an example, an error response like:
>
> { "error": { "class": "DeviceNotRemovable",
> "data": { "device": "virtio0" },
> "desc": "Device 'virtio0' is not removable" } }
>
> Will now be emitted as:
>
> { "error": { "class": "GenericError",
> "desc": "Device 'virtio0' is not removable" } }
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
> QMP/qmp-spec.txt | 10 +++-------
> monitor.c | 18 +++++++++++++-----
> qmp-commands.hx | 3 +--
> 3 files changed, 17 insertions(+), 14 deletions(-)
Doesn't qapi-schema.json need updating, too? It mentions specific error
classes that go away in this patch, such as IOError.
>
> diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
> index 1ba916c..a277896 100644
> --- a/QMP/qmp-spec.txt
> +++ b/QMP/qmp-spec.txt
> @@ -106,14 +106,11 @@ completed because of an error condition.
>
> The format is:
>
> -{ "error": { "class": json-string, "data": json-object, "desc": json-string
> },
> - "id": json-value }
> +{ "error": { "class": json-string, "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 "class" member contains the error class name (eg. "GenericError")
> - 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
> @@ -173,8 +170,7 @@ S: {"return": {"enabled": true, "present": true}, "id":
> "example"}
> ------------------
>
> C: { "execute": }
> -S: {"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data":
> -{}}}
> +S: {"error": {"class": "GenericError", "desc": "Invalid JSON syntax" } }
>
> 3.5 Powerdown event
> -------------------
> diff --git a/monitor.c b/monitor.c
> index aa57167..3694590 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -353,16 +353,26 @@ static void monitor_json_emitter(Monitor *mon, const
> QObject *data)
> QDECREF(json);
> }
>
> +static QDict *build_qmp_error_dict(const QError *err)
> +{
> + QObject *obj;
> +
> + obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %p } }",
> + ErrorClass_lookup[err->err_class],
> + qerror_human(err));
> +
> + return qobject_to_qdict(obj);
> +}
> +
> static void monitor_protocol_emitter(Monitor *mon, QObject *data)
> {
> QDict *qmp;
>
> trace_monitor_protocol_emitter(mon);
>
> - qmp = qdict_new();
> -
> if (!monitor_has_error(mon)) {
> /* success response */
> + qmp = qdict_new();
> if (data) {
> qobject_incref(data);
> qdict_put_obj(qmp, "return", data);
> @@ -372,9 +382,7 @@ static void monitor_protocol_emitter(Monitor *mon,
> QObject *data)
> }
> } else {
> /* error response */
> - qdict_put(mon->error->error, "desc", qerror_human(mon->error));
> - qdict_put(qmp, "error", mon->error->error);
> - QINCREF(mon->error->error);
> + qmp = build_qmp_error_dict(mon->error);
> QDECREF(mon->error);
> mon->error = NULL;
> }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index e3cf3c5..e520b51 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -435,8 +435,7 @@ Example:
> -> { "execute": "inject-nmi" }
> <- { "return": {} }
>
> -Note: inject-nmi is only supported for x86 guest currently, it will
> - returns "Unsupported" error for non-x86 guest.
> +Note: inject-nmi is only supported for x86 guests.
>
> EQMP
Suggest:
Note: inject-nmi fails when the guest doesn't support injecting.
Currently, only x86 guests do.
- Re: [Qemu-devel] [PATCH 17/34] qerror: drop QERR_SOCKET_CONNECT_IN_PROGRESS, (continued)
- [Qemu-devel] [PATCH 16/34] migration: don't rely on QERR_SOCKET_*, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 09/34] qerror: don't delay error message construction, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 18/34] error: drop unused functions, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 21/34] qapi: qapi-types.h: don't include qapi/qapi-types-core.h, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 19/34] block: block_int: include qerror.h, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 20/34] hmp: hmp.h: include qdict.h, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 22/34] qapi: generate correct enum names for camel case enums, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 23/34] qapi: don't convert enum strings to lowercase, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 29/34] qmp: switch to the new error format on the wire, Luiz Capitulino, 2012/08/01
- Re: [Qemu-devel] [PATCH 29/34] qmp: switch to the new error format on the wire,
Markus Armbruster <=
- [Qemu-devel] [PATCH 30/34] qemu-ga: switch to the new error format on the wire, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 32/34] qerror: drop qerror_table and qerror_format(), Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 27/34] qerror: add proper ErrorClass value for QERR_ macros, Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 28/34] error: add error_get_class(), Luiz Capitulino, 2012/08/01
- [Qemu-devel] [PATCH 25/34] qerror: qerror_table: don't use C99 struct initializers, Luiz Capitulino, 2012/08/01