Re: [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 9/8/21 5:09 PM, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
>> We are still adding HMP commands without any QMP counterparts. This is
>> done because there are a reasonable number of scenarios where the cost
>> of designing a QAPI data type for the command is not justified.
>>
>> This has the downside, however, that we will never be able to fully
>> isolate the monitor code from the remainder of QEMU internals. It is
>> desirable to be able to get to a point where subsystems in QEMU are
>> exclusively implemented using QAPI types and never need to have any
>> knowledge of the monitor APIs.
>>
>> The way to get there is to stop adding commands to HMP only. All
>> commands must be implemented using QMP, and any HMP implementation
>> be a shim around the QMP implementation.
>>
>> We don't want to compromise our supportability of QMP long term though.
>>
>> This series proposes that we relax our requirements around fine grained
>> QAPI data design,
> 
> Specifics?  QMP command returns a string, HMP wrapper prints that
> string?
> 
>>                   but with the caveat that any command taking this
>> design approach is mandated to use the 'x-' name prefix.
>>
>> This tradeoff should be suitable for any commands we have been adding
>> exclusively to HMP in recent times, and thus mean we have mandate QMP
>> support for all new commands going forward.
>>
>> This series illustrates the concept by converting the "info registers"
>> HMP to invoke a new 'x-query-registers' QMP command. Note that only
>> the i386 CPU target is converted to work with this new approach, so
>> this series needs to be considered incomplete. If we go forward with
>> this idea, then a subsequent version of this series would need to
>> obviously convert all other CPU targets.
>>
>> After doing that conversion the only use of qemu_fprintf() would be
>> the disas.c file. Remaining uses of qemu_fprintf and qemu_printf
>> could be tackled in a similar way and eventually eliminate the need
>> for any of these printf wrappers in QEMU.
>>
>> NB: I added docs to devel/writing-qmp-commands.rst about the two
>> design approaches to QMP. I didn't see another good place to put
>> an explicit note that we will not add any more HMP-only commands.
>> Obviously HMP/QMP maintainers control this in their reviews of
>> patches, and maybe that's sufficient ?
> 
> We could add devel/writing-hmp-commands.rst, or go with a single
> document devel/writing-monitor-commands.rst.
> 
>> NB: if we take this approach we'll want to figure out how many
>> HMP-only commands we actually have left and then perhaps have
>> HMP-only commands we actually have left
> 
> Yes.
> 
> For many HMP commands, a QMP commands with the same name exists, and the
> former is probably a wrapper around the latter.  Same for HMP "info FOO"
> and QMP query-FOO.
> 
> HMP commands without such a match:

(1) Handy HMP commands while debugging:

- help
- info *
- i/o
- loadvm/savevm
- trace-event/trace-file
- wavcapture/stopcapture
- xp

Eventually also:

- hostfwd_add/hostfwd_remove
- log
- logfile
- print
- sendkey

(2) I suppose these are pre-QMP and wonder about their
    usefulness in the monitor (I'd certainly use a QMP
    equivalent to test).

- migrate_set_capability
- migrate_set_parameter
- migration_mode
- mouse_button
- mouse_move
- mouse_set
- nmi
- pcie_aer_inject_error
- exit_preconfig
- singlestep
- snapshot_blkdev_internal
- snapshot_delete_blkdev_internal

(3) I don't use them, I expect them to belong
    in either (1) or (2).

>     boot_set
>     change
>     commit
>     cpu
>     delvm
>     drive_add
>     drive_del
>     gdbserver
>     gpa2hpa
>     gpa2hva
>     gva2gpa
>     mce
>     qemu-io
>     snapshot_blkdev
>     sum
>     sync-profile
>     watchdog_action

> 
> This is 77 out of 170 HMP commands.  I was hoping for fewer.