On Tue, Aug 25, 2026 at 11:09:46PM +0400, Marc-André Lureau wrote:
> Make the 'readline' value of MonitorMode and the
> 'human-monitor-command' command conditional on CONFIG_HMP, so
> they are only available when HMP support is compiled in.
>
> Reviewed-by: Dr. David Alan Gilbert <[email protected]>
> Signed-off-by: Marc-André Lureau <[email protected]>
> ---
> monitor/monitor.c | 8 +++++++-
> monitor/qmp-cmds.c | 2 ++
> qapi/control.json | 4 +++-
> qapi/misc.json | 3 ++-
> 4 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index da76e6e4ac19..da0c8eda86b0 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -740,13 +740,18 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp,
> Error **errp)
> ERRP_GUARD();
>
> if (!opts->has_mode) {
> - opts->mode = allow_hmp ? MONITOR_MODE_READLINE :
> MONITOR_MODE_CONTROL;
> + opts->mode =
> +#ifdef CONFIG_HMP
> + allow_hmp ? MONITOR_MODE_READLINE :
> +#endif
> + MONITOR_MODE_CONTROL;
> }
This introduces a public facing semantic change.
Currently
qemu-system-x86_64 -chardev stdio,id=foo -mon chardev=foo
will always result in an HMP monitor.
With the new code, this may result in either an HMP or a QMP
monitor, depending on whether QEMU was built with CONFIG_HMP
set or not. This is bad semantics as HMP/QMP are completely
different things to talk to and not transparently replaceable
with each other.
IMHO it needs to be:
if (!opts->has_mode) {
#ifdef CONFIG_HMP
if (allow_hmp) {
error_setg(errp, "HMP support is not built in this QEMU");
return -1;
}
opts->mode = MONITOR_MODE_CONTROL;
#else
opts->mode = allow_hmp ? MONITOR_MODE_READLINE : MONITOR_MODE_CONTROL;
#endif
}
> switch (opts->mode) {
> case MONITOR_MODE_CONTROL:
> monitor_new_qmp(opts->id, opts->chardev, opts->pretty, errp);
> break;
> +#ifdef CONFIG_HMP
> case MONITOR_MODE_READLINE:
> if (!allow_hmp) {
> error_setg(errp, "Only QMP is supported");
> @@ -758,6 +763,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp,
> Error **errp)
> }
> monitor_new_hmp(opts->id, opts->chardev, true, errp);
> break;
> +#endif /* CONFIG_HMP */
> default:
> g_assert_not_reached();
> }
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 :|
