Re: [Qemu-devel] [PATCH 07/20] monitor: Use trad. command interface for HMP pcie_aer_inject_error

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2251 bytes --]

On 05/22/2015 05:36 AM, Markus Armbruster wrote:
> All QMP commands use the "new" handler interface (mhandler.cmd_new).
> Most HMP commands still use the traditional interface (mhandler.cmd),
> but a few use the "new" one.  Complicates handle_user_command() for no
> gain, so I'm converting these to the traditional interface.
> 
> pcie_aer_inject_error's implementation is split into the
> hmp_pcie_aer_inject_error() and pcie_aer_inject_error_print().  The
> former is a peculiar crossbreed between HMP and QMP handler.  On
> success, it works like a QMP handler: store QDict through ret_data
> parameter, return 0.  Printing the QDict is left to
> pcie_aer_inject_error_print().  On failure, it works more like an HMP
> handler: print error to monitor, return negative number.
> 
> To convert to the traditional interface, turn
> pcie_aer_inject_error_print() into a command handler wrapping around
> hmp_pcie_aer_inject_error().  By convention, this command handler
> should be called hmp_pcie_aer_inject_error(), so rename the existing
> hmp_pcie_aer_inject_error() to do_pcie_aer_inject_error().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hmp-commands.hx         |  3 +--
>  hw/pci/pci-stub.c       | 14 +-------------
>  hw/pci/pcie_aer.c       | 39 ++++++++++++++++++++++-----------------
>  include/sysemu/sysemu.h |  4 +---
>  4 files changed, 25 insertions(+), 35 deletions(-)
> 

> +
> +    devfn = (int)qdict_get_int(qdict, "devfn");

Code motion, so the problem is pre-existing, but this cast is unneeded.
 Up to you if you want to clean it up now.

> +    monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
> +                   qdict_get_str(qdict, "id"),
> +                   qdict_get_str(qdict, "root_bus"),
> +                   (int) qdict_get_int(qdict, "bus"),

However, this one still is necessary, unless you tweak the format string
(if you haven't guessed, I prefer avoiding casts when other mechanisms
work equally well).

But that's minor; whether or not you clean things up,

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]