On Wed, Aug 26, 2026 at 01:41:55PM +0400, Marc-André Lureau wrote: > Hi > > On Wed, Aug 26, 2026 at 12:54 PM Daniel P. Berrangé <[email protected]> > wrote: > > > > On Tue, Aug 25, 2026 at 11:09:43PM +0400, Marc-André Lureau wrote: > > > Rename monitor_printf->monitor_hmp_printf, monitor_vprintf-> > > > monitor_hmp_vprintf, and monitor_printc->monitor_hmp_printc, changing > > > the first parameter from Monitor * to MonitorHMP * to enforce type > > > safety. The implementation is also simplified: monitor_hmp_vprintf now > > > directly calls g_strdup_vprintf + monitor_puts, removing the virtual > > > dispatch via moncls->vprintf. > > > > > > The dev_print() callbacks are temporarily using the MONITOR_HMP(mon) > > > cast, they are fixed in the following commits. > > > > > > Signed-off-by: Marc-André Lureau <[email protected]> > > > --- > > > audio/audio-hmp-cmds.c | 6 +- > > > backends/cryptodev-hmp-cmds.c | 9 +- > > > block/monitor/block-hmp-cmds.c | 170 +++++++++--------- > > > chardev/char-hmp-cmds.c | 12 +- > > > disas/disas-mon.c | 10 +- > > > docs/devel/style.rst | 2 +- > > > docs/devel/writing-monitor-commands.rst | 8 +- > > > dump/dump-hmp-cmds.c | 5 +- > > > hw/char/virtio-serial-bus.c | 10 +- > > > hw/core/machine-hmp-cmds.c | 213 +++++++++++----------- > > > hw/core/sysbus.c | 5 +- > > > hw/hexagon/hexagon_tlb.c | 44 ++--- > > > hw/i386/kvm/xen-stubs.c | 6 +- > > > hw/i386/kvm/xen_evtchn.c | 21 +-- > > > hw/i386/sgx-hmp-stub.c | 3 +- > > > hw/i386/sgx.c | 29 ++- > > > hw/misc/auxbus.c | 9 +- > > > hw/misc/mos6522-stub.c | 3 +- > > > hw/net/rocker/rocker-hmp-cmds.c | 146 ++++++++-------- > > > hw/pci/pci-hmp-cmds.c | 114 ++++++------ > > > hw/pci/pci-stub.c | 3 +- > > > hw/s390x/s390-skeys.c | 9 +- > > > hw/s390x/s390-stattrib.c | 20 +-- > > > hw/uefi/ovmf-log.c | 5 +- > > > hw/usb/bus.c | 11 +- > > > hw/usb/host-libusb.c | 21 ++- > > > hw/virtio/virtio-hmp-cmds.c | 297 > > > ++++++++++++++++--------------- > > > hw/xen/xen-bus.c | 5 +- > > > include/disas/disas.h | 4 +- > > > include/monitor/hmp.h | 12 +- > > > migration/dirtyrate.c | 46 +++-- > > > migration/migration-hmp-cmds.c | 301 > > > ++++++++++++++++---------------- > > > monitor/hmp-cmds.c | 141 +++++++-------- > > > monitor/hmp.c | 143 +++++++-------- > > > monitor/monitor-internal.h | 6 - > > > monitor/monitor.c | 33 ++-- > > > net/net-hmp-cmds.c | 31 ++-- > > > net/slirp.c | 31 ++-- > > > qom/qom-hmp-cmds.c | 27 ++- > > > replay/replay-debugging.c | 5 +- > > > stats/stats-hmp-cmds.c | 57 +++--- > > > stubs/hmp-cmd-info_sev.c | 3 +- > > > stubs/monitor-core.c | 2 +- > > > system/dirtylimit-hmp-cmds.c | 10 +- > > > system/qdev-monitor.c | 19 +- > > > system/runstate-hmp-cmds.c | 16 +- > > > system/tpm-hmp-cmds.c | 29 ++- > > > target/i386/cpu-apic.c | 3 +- > > > target/i386/monitor.c | 152 ++++++++-------- > > > target/i386/sev.c | 35 ++-- > > > target/m68k/monitor.c | 3 +- > > > target/ppc/monitor.c | 3 +- > > > target/riscv/monitor.c | 55 +++--- > > > target/sh4/monitor.c | 29 ++- > > > target/sparc/monitor.c | 3 +- > > > target/xtensa/monitor.c | 3 +- > > > tests/unit/test-util-sockets.c | 2 +- > > > tools/qemu-vnc/clipboard.c | 4 +- > > > tools/qemu-vnc/stubs.c | 2 +- > > > trace/trace-hmp-cmds.c | 12 +- > > > ui/ui-hmp-cmds.c | 115 ++++++------ > > > util/error-report.c | 2 +- > > > util/qemu-print.c | 11 +- > > > 63 files changed, 1219 insertions(+), 1327 deletions(-) > > > > > > > > > > > diff --git a/util/qemu-print.c b/util/qemu-print.c > > > index 5d4143d425a1..5938f2b6c338 100644 > > > --- a/util/qemu-print.c > > > +++ b/util/qemu-print.c > > > @@ -13,6 +13,7 @@ > > > #include "qemu/osdep.h" > > > #include "monitor/monitor.h" > > > #include "monitor/hmp.h" > > > +#include "qom/object.h" > > > #include "qemu/qemu-print.h" > > > > > > /* > > > @@ -23,8 +24,13 @@ > > > int qemu_vprintf(const char *fmt, va_list ap) > > > { > > > Monitor *cur_mon = monitor_cur(); > > > + > > > + /* for all monitors: QMP & HMP */ > > > if (cur_mon) { > > > - return monitor_vprintf(cur_mon, fmt, ap); > > > + /* don't use monitor_cur_hmp(), to avoid a second lookup */ > > > + MonitorHMP *hmp = (MonitorHMP *) > > > + object_dynamic_cast(OBJECT(cur_mon), TYPE_MONITOR_HMP); > > > + return monitor_hmp_vprintf(hmp, fmt, ap); > > > > This isn't the same semantics AFAICT. > > > > Original code, if monitor_cur() == QMP, we call monitor_vprintf() > > which will return -1. > > > > New code, if monitor_cur() == QMP, we will get a NULL back from > > object_dynamic_cast which we then pass into monitor_hmp_vprintf > > which will then crash on monitor_puts() IIUC. > > monitor_hmp_vprintf() has an early return, if given NULL monitor, it returns > -1.
Hmm, I feel like we should be dealing with NULL in this method, as it is surprising to be calling a monitor_hmp_XXX method in scenario where QMP is a (theoretical) possibility. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
