Re: [Qemu-devel] [PATCH 30/34] qemu-ga: switch to the new error format on the wire

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On Wed, Aug 01, 2012 at 10:02:50PM -0300, Luiz Capitulino wrote:
> IMPORTANT: this BREAKS qemu-ga compatibility for the error response.
> 
> Instead of returning something like:
> 
> { "error": { "class": "InvalidParameterValue",
>              "data": {"name": "mode", "expected": "halt|powerdown|reboot" } } }
> 
> qemu-ga now returns:
> 
>  { "error": { "class": "GenericError",
>               "desc": "Parameter 'mode' expects halt|powerdown|reboot" } }

Specific error responses weren't part of the documented API, so I think
anything reliant on those is making invalid assumptions; there
should always be a catch-all for unknown/unexpected error
messages/error payloads.

> 
> Notice that this is also a bug fix, as qemu-ga wasn't returning the
> human message.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

So, if the libvirt folks are okay with it:

Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  Makefile.objs       |  2 +-
>  qapi/qmp-core.h     |  1 +
>  qapi/qmp-dispatch.c | 10 +++++++++-
>  qemu-ga.c           |  4 ++--
>  4 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 5ebbcfa..cbfbba5 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -220,7 +220,7 @@ universal-obj-y += $(qapi-obj-y)
>  ######################################################################
>  # guest agent
> 
> -qga-obj-y = qga/ qemu-ga.o module.o
> +qga-obj-y = qga/ qemu-ga.o module.o qapi-types.o qapi-visit.o
>  qga-obj-$(CONFIG_WIN32) += oslib-win32.o
>  qga-obj-$(CONFIG_POSIX) += oslib-posix.o qemu-sockets.o qemu-option.o
> 
> diff --git a/qapi/qmp-core.h b/qapi/qmp-core.h
> index b0f64ba..00446cf 100644
> --- a/qapi/qmp-core.h
> +++ b/qapi/qmp-core.h
> @@ -49,6 +49,7 @@ void qmp_disable_command(const char *name);
>  void qmp_enable_command(const char *name);
>  bool qmp_command_is_enabled(const char *name);
>  char **qmp_get_command_list(void);
> +QObject *qmp_build_error_object(Error *errp);
> 
>  #endif
> 
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index 122c1a2..ec613f8 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -14,6 +14,7 @@
>  #include "qemu-objects.h"
>  #include "qapi/qmp-core.h"
>  #include "json-parser.h"
> +#include "qapi-types.h"
>  #include "error.h"
>  #include "error_int.h"
>  #include "qerror.h"
> @@ -109,6 +110,13 @@ static QObject *do_qmp_dispatch(QObject *request, Error **errp)
>      return ret;
>  }
> 
> +QObject *qmp_build_error_object(Error *errp)
> +{
> +    return qobject_from_jsonf("{ 'class': %s, 'desc': %s }",
> +                              ErrorClass_lookup[error_get_class(errp)],
> +                              error_get_pretty(errp));
> +}
> +
>  QObject *qmp_dispatch(QObject *request)
>  {
>      Error *err = NULL;
> @@ -119,7 +127,7 @@ QObject *qmp_dispatch(QObject *request)
> 
>      rsp = qdict_new();
>      if (err) {
> -        qdict_put_obj(rsp, "error", error_get_qobject(err));
> +        qdict_put_obj(rsp, "error", qmp_build_error_object(err));
>          error_free(err);
>      } else if (ret) {
>          qdict_put_obj(rsp, "return", ret);
> diff --git a/qemu-ga.c b/qemu-ga.c
> index 8199da7..f45bc61 100644
> --- a/qemu-ga.c
> +++ b/qemu-ga.c
> @@ -515,7 +515,7 @@ static void process_event(JSONMessageParser *parser, QList *tokens)
>          } else {
>              g_warning("failed to parse event: %s", error_get_pretty(err));
>          }
> -        qdict_put_obj(qdict, "error", error_get_qobject(err));
> +        qdict_put_obj(qdict, "error", qmp_build_error_object(err));
>          error_free(err);
>      } else {
>          qdict = qobject_to_qdict(obj);
> @@ -532,7 +532,7 @@ static void process_event(JSONMessageParser *parser, QList *tokens)
>              qdict = qdict_new();
>              g_warning("unrecognized payload format");
>              error_set(&err, QERR_UNSUPPORTED);
> -            qdict_put_obj(qdict, "error", error_get_qobject(err));
> +            qdict_put_obj(qdict, "error", qmp_build_error_object(err));
>              error_free(err);
>          }
>          ret = send_response(s, QOBJECT(qdict));
> -- 
> 1.7.11.2.249.g31c7954.dirty
>