On Sun, Aug 16, 2026 at 11:13:02PM +0400, Marc-André Lureau wrote:
> Instead of passing a Monitor pointer through error_vprintf_mon() and
> error_printf_mon(), call monitor_cur_hmp() directly. This removes the
> monitor parameter from the internal helpers and the manual
> object_dynamic_cast() in vreport().
>
> Signed-off-by: Marc-André Lureau <[email protected]>
> ---
> util/error-report.c | 72
> +++++++++++++++++++++++++----------------------------
> 1 file changed, 34 insertions(+), 38 deletions(-)
>
> diff --git a/util/error-report.c b/util/error-report.c
> index aaa15bc79827..70cbd174ffae 100644
> --- a/util/error-report.c
> +++ b/util/error-report.c
> @@ -30,34 +30,31 @@ bool error_with_guestname;
> const char *error_guest_name;
>
> /*
> - * Print to the current human monitor if we have one, else to stderr.
> + * Print to the current HMP monitor if we have one, else to stderr.
> */
> -static int G_GNUC_PRINTF(2, 0)
> -error_vprintf_mon(Monitor *cur_mon, const char *fmt, va_list ap)
> +static int G_GNUC_PRINTF(1, 0)
> +error_vprintf_mon(const char *fmt, va_list ap)
Please don't remove this 'cur_mon' parameter.
> {
> - /*
> - * This will return -1 if 'cur_mon' is NULL, or is QMP.
> - * IOW this will only print if in HMP, otherwise we
> - * fallback to stderr for QMP / no-monitor scenarios.
> - */
> - int ret = monitor_vprintf(cur_mon, fmt, ap);
> - if (ret == -1) {
> - ret = vfprintf(stderr, fmt, ap);
> + MonitorHMP *hmp = monitor_cur_hmp();
This calls qemu_mutex_lock/unlock, which have trace points.
As a result if you have 'log' tracing enabled, error messages
will get trace point output splattered on top.
We must fetch monitor_cur_hmp once only at the top of
vreport() and never query it again.
> +
> + if (hmp) {
> + return monitor_vprintf(MONITOR(hmp), fmt, ap);
> }
> - return ret;
> +
> + return vfprintf(stderr, fmt, ap);
> }
>
> /*
> - * Print to the current human monitor if we have one, else to stderr.
> + * Print to the current HMP monitor if we have one, else to stderr.
> */
> -static int G_GNUC_PRINTF(2, 3)
> -error_printf_mon(Monitor *cur_mon, const char *fmt, ...)
> +static int G_GNUC_PRINTF(1, 2)
> +error_printf_mon(const char *fmt, ...)
> {
> va_list ap;
> int ret;
>
> va_start(ap, fmt);
> - ret = error_vprintf_mon(cur_mon, fmt, ap);
> + ret = error_vprintf_mon(fmt, ap);
> va_end(ap);
> return ret;
> }
> @@ -67,7 +64,7 @@ error_printf_mon(Monitor *cur_mon, const char *fmt, ...)
> */
> int error_vprintf(const char *fmt, va_list ap)
> {
> - return error_vprintf_mon(monitor_cur(), fmt, ap);
> + return error_vprintf_mon(fmt, ap);
> }
>
> /*
> @@ -79,7 +76,7 @@ int error_printf(const char *fmt, ...)
> int ret;
>
> va_start(ap, fmt);
> - ret = error_vprintf_mon(monitor_cur(), fmt, ap);
> + ret = error_vprintf_mon(fmt, ap);
> va_end(ap);
> return ret;
> }
> @@ -183,13 +180,13 @@ void loc_set_file(const char *fname, int lno)
> * Print current location to current HMP monitor if we have one, else
> * to stderr.
> */
> -static void print_loc(Monitor *cur)
> +static void print_loc(MonitorHMP *hmp)
> {
> const char *sep = "";
> int i;
> const char *const *argp;
>
> - if (!cur && g_get_prgname()) {
> + if (!hmp && g_get_prgname()) {
> fprintf(stderr, "%s:", g_get_prgname());
> sep = " ";
> }
> @@ -197,20 +194,20 @@ static void print_loc(Monitor *cur)
> case LOC_CMDLINE:
> argp = cur_loc->ptr;
> for (i = 0; i < cur_loc->num; i++) {
> - error_printf_mon(cur, "%s%s", sep, argp[i]);
> + error_printf_mon("%s%s", sep, argp[i]);
> sep = " ";
> }
> - error_printf_mon(cur, ": ");
> + error_printf_mon(": ");
> break;
> case LOC_FILE:
> - error_printf_mon(cur, "%s:", (const char *)cur_loc->ptr);
> + error_printf_mon("%s:", (const char *)cur_loc->ptr);
> if (cur_loc->num) {
> - error_printf_mon(cur, "%d:", cur_loc->num);
> + error_printf_mon("%d:", cur_loc->num);
> }
> - error_printf_mon(cur, " ");
> + error_printf_mon(" ");
> break;
> default:
> - error_printf_mon(cur, "%s", sep);
> + error_printf_mon("%s", sep);
> }
> }
>
> @@ -233,45 +230,44 @@ static void vreport(report_type type, const char *fmt,
> va_list ap)
> {
> /*
> * When current monitor is QMP, messages must go to stderr
> - * and have prefixes added, so we cast to HMP, leaving 'cur'
> + * and have prefixes added, so we cast to HMP, leaving 'hmp'
> * as NULL in QMP case
> */
The comment talks about a cast hwich no longer exists here.
/*
* When current monitor is QMP, messages must go to stderr
* and have prefixes added, so only need MonitorHMP here
* not the parent Monitor.
*/
> - Monitor *cur = MONITOR(
> - object_dynamic_cast(OBJECT(monitor_cur()), TYPE_MONITOR_HMP));
> + MonitorHMP *hmp = monitor_cur_hmp();
> gchar *timestr;
>
> - if (!cur) {
> + if (!hmp) {
> qemu_flockfile(stderr);
> }
>
> - if (message_with_timestamp && !cur) {
> + if (message_with_timestamp && !hmp) {
> timestr = real_time_iso8601();
> fprintf(stderr, "%s ", timestr);
> g_free(timestr);
> }
>
> /* Only prepend guest name if -msg guest-name and -name guest=... are
> set */
> - if (error_with_guestname && error_guest_name && !cur) {
> + if (error_with_guestname && error_guest_name && !hmp) {
> fprintf(stderr, "%s ", error_guest_name);
> }
>
> - print_loc(cur);
> + print_loc(hmp);
>
> switch (type) {
> case REPORT_TYPE_ERROR:
> break;
> case REPORT_TYPE_WARNING:
> - error_printf_mon(cur, "warning: ");
> + error_printf_mon("warning: ");
> break;
> case REPORT_TYPE_INFO:
> - error_printf_mon(cur, "info: ");
> + error_printf_mon("info: ");
> break;
> }
>
> - error_vprintf_mon(cur, fmt, ap);
> - error_printf_mon(cur, "\n");
> + error_vprintf_mon(fmt, ap);
> + error_printf_mon("\n");
>
> - if (!cur) {
> + if (!hmp) {
> qemu_funlockfile(stderr);
> }
> }
>
> --
> 2.55.0.543.g5ebe2ebe4ea8
>
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 :|
