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

[Markus Armbruster]

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:

    boot_set
    change
    commit
    cpu
    delvm
    drive_add
    drive_del
    exit_preconfig
    gdbserver
    gpa2hpa
    gpa2hva
    gva2gpa
    help
    hostfwd_add
    hostfwd_remove
    i
    info
    info capture
    info cmma
    info cpus
    info history
    info ioapic
    info irq
    info jit
    info lapic
    info mem
    info mtree
    info network
    info numa
    info opcount
    info pic
    info profile
    info qdm
    info qom-tree
    info qtree
    info ramblock
    info rdma
    info registers
    info roms
    info skeys
    info snapshots
    info sync-profile
    info tlb
    info trace-events
    info usb
    info usbhost
    info usernet
    loadvm
    log
    logfile
    mce
    migrate_set_capability
    migrate_set_parameter
    migration_mode
    mouse_button
    mouse_move
    mouse_set
    nmi
    o
    pcie_aer_inject_error
    print
    qemu-io
    savevm
    sendkey
    singlestep
    snapshot_blkdev
    snapshot_blkdev_internal
    snapshot_delete_blkdev_internal
    stopcapture
    sum
    sync-profile
    trace-event
    trace-file
    watchdog_action
    wavcapture
    x
    xp

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

>                                         and then perhaps have
> a task to track their conversion to QMP. This could possibly
> be a useful task for newbies if we make it clear that they
> wouldn't be required to undertake complex QAPI modelling in
> doing this conversion.

Yes.

Thanks for tackling this!