Re: [PATCH v3 08/20] log: avoid repeated prefix on incremental qemu_log calls

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Sep 17, 2025 at 04:11:44PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > Some code makes multiple qemu_log calls to incrementally emit
> > a single message. Currently timestamps get prepended to all
> > qemu_log calls, even those continuing a previous incomplete
> > message.
> >
> > This changes the qemu_log so it skips adding a new line prefix,
> > if the previous qemu_log call did NOT end with a newline.
> >
> 
> Have you considered
> 
>   Fixes: 012842c07552 (log: make '-msg timestamp=on' apply to all qemu_log usage)
> 
> ?
> 
> > Reported-by: Richard Henderson <richard.henderson@linaro.org>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  util/log.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/util/log.c b/util/log.c
> > index abdcb6b311..2642a55c59 100644
> > --- a/util/log.c
> > +++ b/util/log.c
> > @@ -143,6 +143,12 @@ void qemu_log_unlock(FILE *logfile)
> >      }
> >  }
> >  
> > +/*
> > + * 'true' if the previous log message lacked a trailing '\n',
> > + * and thus the subsequent call must skip any prefix
> > + */
> > +static __thread bool incomplete;
> 
> Thread-local only because we have @log_per_thread, isn't it?

Pretty much. If you're not using log_per_thread, then code which
incrementally emits a single line using multiple qemu_log calls
is doomed in a concurrency sitation, but this __thread at least
doesn't make the situation worse than it already is.

> 
> > +
> >  void qemu_log(const char *fmt, ...)
> >  {
> >      FILE *f;
> > @@ -154,7 +160,7 @@ void qemu_log(const char *fmt, ...)
> >       * was emitted if we are delayed acquiring the
> >       * mutex
> >       */
> > -    if (message_with_timestamp) {
> > +    if (message_with_timestamp && !incomplete) {
> >          g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
> >          timestr = g_date_time_format_iso8601(dt);
> >      }
> > @@ -170,6 +176,7 @@ void qemu_log(const char *fmt, ...)
> >          va_start(ap, fmt);
> >          vfprintf(f, fmt, ap);
> >          va_end(ap);
> > +        incomplete = fmt[strlen(fmt) - 1] != '\n';
> >          qemu_log_unlock(f);
> >      }
> >  }
> 

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