[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all |
Date: |
Wed, 8 Sep 2021 18:15:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
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.
- Re: [PATCH 4/5] qapi: introduce x-query-registers QMP command, (continued)
- [PATCH 5/5] monitor: rewrite 'info registers' in terms of 'x-query-registers', Daniel P . Berrangé, 2021/09/08
- Re: [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all, Ján Tomko, 2021/09/08
- Re: [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all, Markus Armbruster, 2021/09/08
- Re: [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all,
Philippe Mathieu-Daudé <=
- Re: [PATCH 0/5] Stop adding HMP-only commands, allow QMP for all, Paolo Bonzini, 2021/09/09