[PATCH v3 11/20] monitor: move error_vprintf() back to error-report.c

Daniel P. Berrangé posted 20 patches 2 weeks, 3 days ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, Christian Schoenebeck <qemu_oss@crudebyte.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Dr. David Alan Gilbert" <dave@treblig.org>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Stefan Weil <sw@weilnetz.de>
There is a newer version of this series
[PATCH v3 11/20] monitor: move error_vprintf() back to error-report.c
Posted by Daniel P. Berrangé 2 weeks, 3 days ago
This conceptually reverts 397d30e9401d2da96dbdf0ce49805d6d4bb68833.
The discussion around stubs in that commit does not appear to be
important to the current state of the codebase.

This makes the error_vprintf() impl source file match that
of error_printf(), and also match the header where it is declared.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/monitor.c              | 13 -------------
 stubs/monitor-core.c           |  5 +++++
 tests/unit/test-util-sockets.c |  1 +
 util/error-report.c            | 13 +++++++++++++
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/monitor/monitor.c b/monitor/monitor.c
index 03dbe5d131..e1e5dbfcbe 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -268,19 +268,6 @@ void monitor_printc(Monitor *mon, int c)
     monitor_printf(mon, "'");
 }
 
-/*
- * Print to current monitor if we have one, else to stderr.
- */
-int error_vprintf(const char *fmt, va_list ap)
-{
-    Monitor *cur_mon = monitor_cur();
-
-    if (cur_mon && !monitor_cur_is_qmp()) {
-        return monitor_vprintf(cur_mon, fmt, ap);
-    }
-    return vfprintf(stderr, fmt, ap);
-}
-
 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/monitor-core.c b/stubs/monitor-core.c
index 1894cdfe1f..b498a0f1af 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_qmp(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..bd48731ea2 100644
--- a/tests/unit/test-util-sockets.c
+++ b/tests/unit/test-util-sockets.c
@@ -72,6 +72,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
  * otherwise we get duplicate syms at link time.
  */
 Monitor *monitor_cur(void) { return cur_mon; }
+bool monitor_cur_is_qmp(void) { return false; }
 Monitor *monitor_set_cur(Coroutine *co, Monitor *mon) { abort(); }
 int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { abort(); }
 
diff --git a/util/error-report.c b/util/error-report.c
index 1b17c11de1..79b6f23e64 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -29,6 +29,19 @@ bool message_with_timestamp;
 bool error_with_guestname;
 const char *error_guest_name;
 
+/*
+ * Print to current monitor if we have one, else to stderr.
+ */
+int error_vprintf(const char *fmt, va_list ap)
+{
+    Monitor *cur_mon = monitor_cur();
+
+    if (cur_mon && !monitor_cur_is_qmp()) {
+        return monitor_vprintf(cur_mon, fmt, ap);
+    }
+    return vfprintf(stderr, fmt, ap);
+}
+
 int error_printf(const char *fmt, ...)
 {
     va_list ap;
-- 
2.50.1


Re: [PATCH v3 11/20] monitor: move error_vprintf() back to error-report.c
Posted by Markus Armbruster 1 week, 2 days ago
Daniel P. Berrangé <berrange@redhat.com> writes:

> This conceptually reverts 397d30e9401d2da96dbdf0ce49805d6d4bb68833.
> The discussion around stubs in that commit does not appear to be
> important to the current state of the codebase.

Can we figure out why Paolo's change is no longer needed?

Discussion back then:
https://lore.kernel.org/qemu-devel/37942764-b3c8-1ac1-5121-894ada7300f2@redhat.com/
https://lore.kernel.org/qemu-devel/1477326663-67817-1-git-send-email-pbonzini@redhat.com/

> This makes the error_vprintf() impl source file match that
> of error_printf(), and also match the header where it is declared.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Re: [PATCH v3 11/20] monitor: move error_vprintf() back to error-report.c
Posted by Daniel P. Berrangé 1 week, 2 days ago
On Fri, Sep 19, 2025 at 02:27:20PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > This conceptually reverts 397d30e9401d2da96dbdf0ce49805d6d4bb68833.
> > The discussion around stubs in that commit does not appear to be
> > important to the current state of the codebase.
> 
> Can we figure out why Paolo's change is no longer needed?
> 
> Discussion back then:
> https://lore.kernel.org/qemu-devel/37942764-b3c8-1ac1-5121-894ada7300f2@redhat.com/
> https://lore.kernel.org/qemu-devel/1477326663-67817-1-git-send-email-pbonzini@redhat.com/

I'm wrong, this change of mine is broken because it messes up the output
of test-vmstate still

> 
> > This makes the error_vprintf() impl source file match that
> > of error_printf(), and also match the header where it is declared.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 

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 v3 11/20] monitor: move error_vprintf() back to error-report.c
Posted by Richard Henderson 2 weeks, 3 days ago
On 9/10/25 18:03, Daniel P. Berrangé wrote:
> This conceptually reverts 397d30e9401d2da96dbdf0ce49805d6d4bb68833.
> The discussion around stubs in that commit does not appear to be
> important to the current state of the codebase.
> 
> This makes the error_vprintf() impl source file match that
> of error_printf(), and also match the header where it is declared.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   monitor/monitor.c              | 13 -------------
>   stubs/monitor-core.c           |  5 +++++
>   tests/unit/test-util-sockets.c |  1 +
>   util/error-report.c            | 13 +++++++++++++
>   4 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 03dbe5d131..e1e5dbfcbe 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -268,19 +268,6 @@ void monitor_printc(Monitor *mon, int c)
>       monitor_printf(mon, "'");
>   }
>   
> -/*
> - * Print to current monitor if we have one, else to stderr.
> - */
> -int error_vprintf(const char *fmt, va_list ap)
> -{
> -    Monitor *cur_mon = monitor_cur();
> -
> -    if (cur_mon && !monitor_cur_is_qmp()) {
> -        return monitor_vprintf(cur_mon, fmt, ap);
> -    }
> -    return vfprintf(stderr, fmt, ap);
> -}
> -
>   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/monitor-core.c b/stubs/monitor-core.c
> index 1894cdfe1f..b498a0f1af 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_qmp(void)
> +{
> +    return false;
> +}

Better as g_assert_not_reached().  It's not obvious whether hmp or qmp is a better 
fallback answer for "no monitor".  Anyway, since monitor_cur() is NULL, we should never 
get here.

With that,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> +
>   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..bd48731ea2 100644
> --- a/tests/unit/test-util-sockets.c
> +++ b/tests/unit/test-util-sockets.c
> @@ -72,6 +72,7 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
>    * otherwise we get duplicate syms at link time.
>    */
>   Monitor *monitor_cur(void) { return cur_mon; }
> +bool monitor_cur_is_qmp(void) { return false; }
>   Monitor *monitor_set_cur(Coroutine *co, Monitor *mon) { abort(); }
>   int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap) { abort(); }
>   
> diff --git a/util/error-report.c b/util/error-report.c
> index 1b17c11de1..79b6f23e64 100644
> --- a/util/error-report.c
> +++ b/util/error-report.c
> @@ -29,6 +29,19 @@ bool message_with_timestamp;
>   bool error_with_guestname;
>   const char *error_guest_name;
>   
> +/*
> + * Print to current monitor if we have one, else to stderr.
> + */
> +int error_vprintf(const char *fmt, va_list ap)
> +{
> +    Monitor *cur_mon = monitor_cur();
> +
> +    if (cur_mon && !monitor_cur_is_qmp()) {
> +        return monitor_vprintf(cur_mon, fmt, ap);
> +    }
> +    return vfprintf(stderr, fmt, ap);
> +}
> +
>   int error_printf(const char *fmt, ...)
>   {
>       va_list ap;