Hi Daniel,

I'm still not keen on qmessage_context allocating a string.

If we *did* allocate a string, it should be a GString so that we can
easily append to that.  The only benefit I see from this is that we
would collect all of the output and it would reach stderr atomically.

However, I think it's better to not collect the message and just
output the pieces.  Something like the following.  The names are
horrible and I didn't document the patches well.


r~


Richard Henderson (4):
  util: Introduce LogOutput
  util: Drop QMESSAGE_CONTEXT_SKIP_MONITOR
  util/message: Use LogOutput
  util/error-report: Use LogOutput in vreport

 include/monitor/monitor.h |  4 +++
 include/qemu/log-output.h | 14 ++++++++
 include/qemu/message.h    | 21 +++--------
 monitor/monitor.c         | 54 +++++++++++++++++++++++------
 stubs/error-printf.c      | 11 ++++++
 util/error-report.c       | 73 +++++++++++++++++++--------------------
 util/log.c                | 23 +++++-------
 util/message.c            | 45 ++++++++++--------------
 8 files changed, 139 insertions(+), 106 deletions(-)
 create mode 100644 include/qemu/log-output.h

-- 
2.43.0

On Tue, Sep 02, 2025 at 12:30:06PM +0200, Richard Henderson wrote:
> Hi Daniel,
> 
> I'm still not keen on qmessage_context allocating a string.
> 
> If we *did* allocate a string, it should be a GString so that we can
> easily append to that.  The only benefit I see from this is that we
> would collect all of the output and it would reach stderr atomically.
> 
> However, I think it's better to not collect the message and just
> output the pieces.  Something like the following.  The names are
> horrible and I didn't document the patches well.

I've posted a v3 of my series which incorporates the conceptual
idea you show here. I managed to simplify it somewhat as I came
to realize the qmessage_context code never needs to write to
anything other than a "FILE *", because we always skip the context
when writing to HMP. IOW, we could simply use fprintf() directly
throughout

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 :|

On Tue, Sep 02, 2025 at 12:30:06PM +0200, Richard Henderson wrote:
> Hi Daniel,
> 
> I'm still not keen on qmessage_context allocating a string.
> 
> If we *did* allocate a string, it should be a GString so that we can
> easily append to that.  The only benefit I see from this is that we
> would collect all of the output and it would reach stderr atomically.
> 
> However, I think it's better to not collect the message and just
> output the pieces.  Something like the following.  The names are
> horrible and I didn't document the patches well.

Thanks for the suggestion, I'll have a look and see about merging
this, or something like it, into my series for a new posting.

> 
> 
> r~
> 
> 
> Richard Henderson (4):
>   util: Introduce LogOutput
>   util: Drop QMESSAGE_CONTEXT_SKIP_MONITOR
>   util/message: Use LogOutput
>   util/error-report: Use LogOutput in vreport
> 
>  include/monitor/monitor.h |  4 +++
>  include/qemu/log-output.h | 14 ++++++++
>  include/qemu/message.h    | 21 +++--------
>  monitor/monitor.c         | 54 +++++++++++++++++++++++------
>  stubs/error-printf.c      | 11 ++++++
>  util/error-report.c       | 73 +++++++++++++++++++--------------------
>  util/log.c                | 23 +++++-------
>  util/message.c            | 45 ++++++++++--------------
>  8 files changed, 139 insertions(+), 106 deletions(-)
>  create mode 100644 include/qemu/log-output.h
> 
> -- 
> 2.43.0
> 

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 :|