[PATCH for-11.0 0/5] some improvements around error reporting

[PATCH for-11.0 0/5] some improvements around error reporting

[Vladimir Sementsov-Ogievskiy]

Hi all. The main thing I wanted to do is to enable timestamps
for errer_report() and friends when QMP monitor is active.
Analyzing logs without timestamps is painful.

And some enhancements here and there around.

Vladimir Sementsov-Ogievskiy (5):
  ui/vnc: don't use of error_printf_unless_qmp()
  monitor: remove unused error_printf_unless_qmp() function
  monitor: rework monitor_cur_is_qmp() into monitor_cur_is_hmp()
  error: print error_report timestamp when QMP monitor is active
  error-report: fix doc for vreport()

 include/monitor/monitor.h      |  5 +----
 monitor/monitor.c              | 30 +++---------------------------
 stubs/error-printf.c           |  5 -----
 stubs/monitor-core.c           |  5 +++++
 tests/unit/test-util-sockets.c |  1 +
 ui/vnc.c                       |  9 ++++-----
 util/error-report.c            | 25 +++++++++++++++----------
 7 files changed, 29 insertions(+), 51 deletions(-)

-- 
2.48.1

[PATCH 1/5] ui/vnc: don't use of error_printf_unless_qmp()

[Vladimir Sementsov-Ogievskiy]

That's not a big deal to print these messages to log, if QMP
monitor is active. And these two are the only users of this
API. To simplify things, let's use more usual error_report() and
info_report(), and drop error_printf_unless_qmp() in the following
commit.

Drop \n from messages, as vreport() adds one.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 ui/vnc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index e6bcf0e1cf..ffeed5788e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3534,8 +3534,8 @@ int vnc_display_password(const char *id, const char *password)
         return -EINVAL;
     }
     if (vd->auth == VNC_AUTH_NONE) {
-        error_printf_unless_qmp("If you want use passwords please enable "
-                                "password auth using '-vnc ${dpy},password'.\n");
+        error_report("If you want use passwords please enable "
+                     "password auth using '-vnc ${dpy},password'.");
         return -EINVAL;
     }
 
@@ -3574,9 +3574,8 @@ static void vnc_display_print_local_addr(VncDisplay *vd)
         qapi_free_SocketAddress(addr);
         return;
     }
-    error_printf_unless_qmp("VNC server running on %s:%s\n",
-                            addr->u.inet.host,
-                            addr->u.inet.port);
+    info_report("VNC server running on %s:%s", addr->u.inet.host,
+                addr->u.inet.port);
     qapi_free_SocketAddress(addr);
 }
 
-- 
2.48.1

[PATCH 2/5] monitor: remove unused error_printf_unless_qmp() function

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/monitor/monitor.h |  3 ---
 monitor/monitor.c         | 24 ------------------------
 stubs/error-printf.c      |  5 -----
 3 files changed, 32 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index c3740ec616..296690e1f1 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -59,7 +59,4 @@ void monitor_register_hmp(const char *name, bool info,
 void monitor_register_hmp_info_hrt(const char *name,
                                    HumanReadableText *(*handler)(Error **errp));
 
-int error_vprintf_unless_qmp(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
-int error_printf_unless_qmp(const char *fmt, ...) G_GNUC_PRINTF(1, 2);
-
 #endif /* MONITOR_H */
diff --git a/monitor/monitor.c b/monitor/monitor.c
index c5a5d30877..b5007c1851 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -281,30 +281,6 @@ int error_vprintf(const char *fmt, va_list ap)
     return vfprintf(stderr, fmt, ap);
 }
 
-int error_vprintf_unless_qmp(const char *fmt, va_list ap)
-{
-    Monitor *cur_mon = monitor_cur();
-
-    if (!cur_mon) {
-        return vfprintf(stderr, fmt, ap);
-    }
-    if (!monitor_cur_is_qmp()) {
-        return monitor_vprintf(cur_mon, fmt, ap);
-    }
-    return -1;
-}
-
-int error_printf_unless_qmp(const char *fmt, ...)
-{
-    va_list ap;
-    int ret;
-
-    va_start(ap, fmt);
-    ret = error_vprintf_unless_qmp(fmt, ap);
-    va_end(ap);
-    return ret;
-}
-
 static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = {
     /* Limit guest-triggerable events to 1 per second */
     [QAPI_EVENT_RTC_CHANGE]        = { 1000 * SCALE_MS },
diff --git a/stubs/error-printf.c b/stubs/error-printf.c
index 0e326d8010..1afa0f62ca 100644
--- a/stubs/error-printf.c
+++ b/stubs/error-printf.c
@@ -16,8 +16,3 @@ int error_vprintf(const char *fmt, va_list ap)
     }
     return vfprintf(stderr, fmt, ap);
 }
-
-int error_vprintf_unless_qmp(const char *fmt, va_list ap)
-{
-    return error_vprintf(fmt, ap);
-}
-- 
2.48.1

[PATCH 3/5] monitor: rework monitor_cur_is_qmp() into monitor_cur_is_hmp()

[Vladimir Sementsov-Ogievskiy]

Condition in error_vprintf() becomes simpler. Also we are going
to reuse monitor_cur_is_hmp() in the following commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 include/monitor/monitor.h | 2 +-
 monitor/monitor.c         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 296690e1f1..b531108eeb 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -15,7 +15,7 @@ extern QemuOptsList qemu_mon_opts;
 
 Monitor *monitor_cur(void);
 Monitor *monitor_set_cur(Coroutine *co, Monitor *mon);
-bool monitor_cur_is_qmp(void);
+bool monitor_cur_is_hmp(void);
 
 void monitor_init_globals(void);
 void monitor_init_globals_core(void);
diff --git a/monitor/monitor.c b/monitor/monitor.c
index b5007c1851..8ca17e9326 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -109,11 +109,11 @@ Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
 /**
  * Is the current monitor, if any, a QMP monitor?
  */
-bool monitor_cur_is_qmp(void)
+bool monitor_cur_is_hmp(void)
 {
     Monitor *cur_mon = monitor_cur();
 
-    return cur_mon && monitor_is_qmp(cur_mon);
+    return cur_mon && !monitor_is_qmp(cur_mon);
 }
 
 /**
@@ -275,7 +275,7 @@ int error_vprintf(const char *fmt, va_list ap)
 {
     Monitor *cur_mon = monitor_cur();
 
-    if (cur_mon && !monitor_cur_is_qmp()) {
+    if (monitor_cur_is_hmp()) {
         return monitor_vprintf(cur_mon, fmt, ap);
     }
     return vfprintf(stderr, fmt, ap);
-- 
2.48.1

[PATCH 4/5] error: print error_report timestamp when QMP monitor is active

[Vladimir Sementsov-Ogievskiy]

We skip printing timestamp when _any_ monitor is active. But
then, in production (where QMP is usually used) we lack timestamps
in logs.

Let's go a bit further, and use same logic to detect HMP monitor
in the whole util/error-report.c like in error_vprintf().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 stubs/monitor-core.c           |  5 +++++
 tests/unit/test-util-sockets.c |  1 +
 util/error-report.c            | 23 ++++++++++++++---------
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
index 1894cdfe1f..275cb0cbfa 100644
--- a/stubs/monitor-core.c
+++ b/stubs/monitor-core.c
@@ -7,6 +7,11 @@ Monitor *monitor_cur(void)
     return NULL;
 }
 
+bool monitor_cur_is_hmp(void)
+{
+    return false;
+}
+
 Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)
 {
     return NULL;
diff --git a/tests/unit/test-util-sockets.c b/tests/unit/test-util-sockets.c
index ee66d727c3..4b7f408902 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -74,6 +74,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
 Monitor *monitor_cur(void) { return cur_mon; }
 Monitor *monitor_set_cur(Coroutine *co, Monitor *mon) { abort(); }
 int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { abort(); }
+bool monitor_cur_is_hmp(void) { return false; }
 
 #ifndef _WIN32
 static void test_socket_fd_pass_name_good(void)
diff --git a/util/error-report.c b/util/error-report.c
index 1b17c11de1..7ffbcf2123 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -144,7 +144,7 @@ static void print_loc(void)
     int i;
     const char *const *argp;
 
-    if (!monitor_cur() && g_get_prgname()) {
+    if (!monitor_cur_is_hmp() && g_get_prgname()) {
         error_printf("%s:", g_get_prgname());
         sep = " ";
     }
@@ -188,15 +188,20 @@ static void vreport(report_type type, const char *fmt, va_list ap)
 {
     gchar *timestr;
 
-    if (message_with_timestamp && !monitor_cur()) {
-        timestr = real_time_iso8601();
-        error_printf("%s ", timestr);
-        g_free(timestr);
-    }
+    if (!monitor_cur_is_hmp()) {
+        if (message_with_timestamp) {
+            timestr = real_time_iso8601();
+            error_printf("%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 && !monitor_cur()) {
-        error_printf("%s ", error_guest_name);
+        /*
+         * Only prepend guest name if -msg guest-name and -name guest=...
+         * are set.
+         */
+        if (error_with_guestname && error_guest_name) {
+            error_printf("%s ", error_guest_name);
+        }
     }
 
     print_loc();
-- 
2.48.1

[PATCH 5/5] error-report: fix doc for vreport()

[Vladimir Sementsov-Ogievskiy]

We actually don't print any errors to QMP monitor.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 util/error-report.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/error-report.c b/util/error-report.c
index 7ffbcf2123..0f3b1c0354 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -177,7 +177,7 @@ real_time_iso8601(void)
 }
 
 /*
- * Print a message to current monitor if we have one, else to stderr.
+ * Print a message to current HMP monitor if we have one, else to stderr.
  * @report_type is the type of message: error, warning or informational.
  * Format arguments like vsprintf().  The resulting message should be
  * a single phrase, with no newline or trailing punctuation.
-- 
2.48.1

Re: [PATCH for-11.0 0/5] some improvements around error reporting

[Daniel P. Berrangé]

On Thu, Nov 27, 2025 at 08:33:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all. The main thing I wanted to do is to enable timestamps
> for errer_report() and friends when QMP monitor is active.
> Analyzing logs without timestamps is painful.
> 
> And some enhancements here and there around.

This is significantly overlapping  with what I've been working on here:

  https://lists.nongnu.org/archive/html/qemu-devel/2025-09/msg05222.html

I had to pause that temporarily but intend to post a v5 for 11.0 dev
cycle

> 
> Vladimir Sementsov-Ogievskiy (5):
>   ui/vnc: don't use of error_printf_unless_qmp()
>   monitor: remove unused error_printf_unless_qmp() function
>   monitor: rework monitor_cur_is_qmp() into monitor_cur_is_hmp()
>   error: print error_report timestamp when QMP monitor is active
>   error-report: fix doc for vreport()
> 
>  include/monitor/monitor.h      |  5 +----
>  monitor/monitor.c              | 30 +++---------------------------
>  stubs/error-printf.c           |  5 -----
>  stubs/monitor-core.c           |  5 +++++
>  tests/unit/test-util-sockets.c |  1 +
>  ui/vnc.c                       |  9 ++++-----
>  util/error-report.c            | 25 +++++++++++++++----------
>  7 files changed, 29 insertions(+), 51 deletions(-)
> 
> -- 
> 2.48.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH for-11.0 0/5] some improvements around error reporting

[Vladimir Sementsov-Ogievskiy]

On 27.11.25 23:51, Daniel P. Berrangé wrote:
> On Thu, Nov 27, 2025 at 08:33:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all. The main thing I wanted to do is to enable timestamps
>> for errer_report() and friends when QMP monitor is active.
>> Analyzing logs without timestamps is painful.
>>
>> And some enhancements here and there around.
> 
> This is significantly overlapping  with what I've been working on here:
> 
>    https://lists.nongnu.org/archive/html/qemu-devel/2025-09/msg05222.html
> 
> I had to pause that temporarily but intend to post a v5 for 11.0 dev
> cycle
> 

Oh, I am late) Seems all my commits duplicate yours. Except 05)

-- 
Best regards,
Vladimir