On Thu, 02 Aug 2012 19:12:03 +0200
Markus Armbruster <arm...@redhat.com> wrote:
> Luiz Capitulino <lcapitul...@redhat.com> 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 <lcapitul...@redhat.com>
> > ---
> > 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.
Oh, true. I remembered about that when I started working on v1 but then
forgot about doing it. It would be also nice to add some examples to
docs/writing-qmp-commands.txt.
>
> >
> > 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.
>
