Re: [Qemu-devel] [PATCH 07/11] qmp: Wean off qerror_report()

[Eric Blake]

On 06/13/2015 08:20 AM, Markus Armbruster wrote:
> The traditional QMP command handler interface
> 
>     int qmp_FOO(Monitor *mon, const QDict *params, QObject **ret_data);
> 
> doesn't provide for returning an Error object.  Instead, the handler
> is expected to stash it in the monitor with qerror_report().
> 
> When we rebased QMP on top of QAPI, we didn't change this interface.
> Instead, commit 776574d introduced "middle mode" as a temporary aid
> for converting existing QMP commands to QAPI one by one.  More than
> three years later, we're still using it.
> 
> Middle mode has two effects:

Are these effects documented anywhere, as in docs/qapi-code-gen.txt?
Should they be, or are we better off trying to get rid of middle mode?

> 
> * Instead of the native input marshallers
> 
>       static void qmp_marshal_input_FOO(QDict *, QObject **, Error **)
> 
>   it generates input marshallers conforming to the traditional QMP
>   command handler interface.
> 
> * It suppresses generation of code to register them with
>   qmp_register_command()
> 
>   This permits giving them internal linkage.
> 
> As long as we need qmp-commands.hx, we can't use the registry behind
> qmp_register_command(), so the latter has to stay for now.

Is there a qapi marking we can add for this effect, similar to
'gen':false?  For that matter...

> 
> The former has to go to get rid of qerror_report().  Changing all QMP
> commands to fit the QAPI mold in one go was impractical back when we
> started, but by now there are just a few stragglers left:
> do_qmp_capabilities(), qmp_qom_set(), qmp_qom_get(), qmp_object_add(),
> qmp_netdev_add(), do_device_add().

...many of these are already using 'gen':false!

> 
> Switch middle mode to generate native input marshallers, and adapt the
> stragglers.  Simplifies both the monitor code and the stragglers.
> 
> Rename do_qmp_capabilities() to qmp_capabilities(), and
> do_device_add() to qmp_device_add, because that's how QMP command
> handlers are named today.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  hmp.c                     |  5 ++++-
>  include/monitor/monitor.h |  7 +++---
>  include/monitor/qdev.h    |  3 ++-
>  include/net/net.h         |  2 +-
>  monitor.c                 | 24 ++++++---------------
>  net/net.c                 | 16 ++++++--------
>  qdev-monitor.c            | 15 ++++++-------
>  qmp-commands.hx           |  4 ++--
>  qmp.c                     | 55 
> +++++++++++------------------------------------
>  scripts/qapi-commands.py  | 41 ++++++-----------------------------
>  util/qemu-error.c         |  4 ++--
>  11 files changed, 50 insertions(+), 126 deletions(-)

Relatively nice diffstat.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature