Eric Blake <ebl...@redhat.com> writes: > 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 <arm...@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.
While I occasionally do fold cleanups into mechanical changes, e.g in PATCH 15, I'm reluctant to fold them into code motion. >> + 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). I share your preference, but I want to keep the changes in PCI code to a trivial minimum, to keep this series firmly in the monitor subsystem. > But that's minor; whether or not you clean things up, > > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!
