Re: [PATCH v6 20/27] util: don't skip error prefixes when QMP is active

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> The vreport() function will print to HMP if available, otherwise
> to stderr. In the event that vreport() is called during execution
> of a QMP command, it will print to stderr, but mistakenly omit the
> message prefixes (timestamp, guest name, program name).
>
> This new usage of monitor_is_cur_qmp() from vreport() requires that
> we add a stub to satisfy linking of non-system emulator binaries.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  stubs/monitor-core.c           | 5 +++++
>  tests/unit/test-util-sockets.c | 1 +
>  util/error-report.c            | 8 ++++++++
>  3 files changed, 14 insertions(+)
>
> diff --git a/stubs/monitor-core.c b/stubs/monitor-core.c
> index a7c32297c9..078a5012e9 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..b9f2453e29 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_qmp(void) { abort(); };
>  
>  #ifndef _WIN32
>  static void test_socket_fd_pass_name_good(void)
> diff --git a/util/error-report.c b/util/error-report.c
> index b34a2bc7bc..052f0f68b8 100644
> --- a/util/error-report.c
> +++ b/util/error-report.c
> @@ -227,6 +227,14 @@ static void vreport(report_type type, const char *fmt, va_list ap)
>      Monitor *cur = monitor_cur();
>      gchar *timestr;
>  
> +    /*
> +     * When current monitor is QMP, messages must go to stderr
> +     * and have prefixes added

Let's end the sentence with a period.

> +     */
> +    if (monitor_cur_is_qmp()) {
> +        cur = NULL;
> +    }
> +
>      if (message_with_timestamp && !cur) {
>          timestr = real_time_iso8601();
>          fprintf(stderr, "%s ", timestr);

Reviewed-by: Markus Armbruster <armbru@redhat.com>