Thank you everyone for the review I have shared
[PATCH v2] utils/log: add qemu_log_timestamp()
in a separate mail thread with the suggested changes.
On Tue, 10 Jun 2025 at 12:37 AM, Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Mon, Jun 9, 2025 at 2:21 PM Tanish Desai <tanishdesa...@gmail.com>
> wrote:
> >
> > Moved the logic for timestamped logging (~6 lines) from
> a_nocheck__trace_foo(header) into a new qemu_log_timestamp() function in
> util/log.c. This avoids code duplication across binaries and enables reuse
> as a standalone utility.
> > Encapsulation helps reduce build size significantly, particularly when
> many trace points are present. On Ubuntu 22 with
> > ./configure --target-list=aarch64-softmmu --enable-kvm
> --enable-trace-backends=log,
> > this change reduced the build directory size from ~1435.27 MB to ~1412
> MB (~23 MB saved).
> > Notable reductions include:
> > trace/: ~2.6 MB
> > libqemuutil.a.p: ~3 MB
> > A detailed report of size changes (in bytes) for relevant folders and
> subfolders will follow in a trailing mail.
>
> Nice, the output of size(1) on qemu-system-x86_64 is reduced by 3%
> (839 KB) when built with gcc 15.1.1 on x86_64:
>
> text data bss dec hex filename
> 14712231 13652904 149496 28514631 1b31947 before
> 13852879 13652904 149496 27655279 1a5fc6f after
>
> That is in the same ballpark as the change in build directory size you
> measured.
>
> > diff --git a/util/log.c b/util/log.c
> > index b87d399e4c..996530fe7e 100644
> > --- a/util/log.c
> > +++ b/util/log.c
> > @@ -143,6 +143,24 @@ void qemu_log_unlock(FILE *logfile)
> > }
> > }
> >
> > +
> > +void qemu_log_timestamp(const char *fmt, ...)
> > +{
> > + FILE *f = qemu_log_trylock();
> > + if (f) {
> > + va_list ap;
> > + if(message_with_timestamp){
> > + struct timeval _now;
> > + gettimeofday(&_now, NULL);
> > +
> > fprintf(f,"%d@%zu.%06zu:",qemu_get_thread_id(),(size_t)_now.tv_sec,
> (size_t)_now.tv_usec);
> > + }
> > + va_start(ap, fmt);
> > + vfprintf(f, fmt, ap);
> > + va_end(ap);
> > + }
> > + qemu_log_unlock(f);
>
> Although calling qemu_log_unlock(NULL) is safe, existing callers
> invoke this function inside the if (f) { ... } body. Please follow
> that approach for consistency.
>
> Looks good aside from that.
>
