> > +DEF("msg", HAS_ARG, QEMU_OPTION_msg,
> > + "-msg [timestamp=on|off]\n"
> > + " change the format of messages\n"
> > + " timestamp=on|off enables leading timestamps (default:on)\n",
> > + QEMU_ARCH_ALL)
> > +STEXI
> > +@item -msg timestamp=on|off
> > +@findex -msg
> > +prepend a timestamp to each log message.
> > +(disabled by default)
> > +ETEXI
>
> I am confused. If the user specifies -msg then enable_timestamp_msg is
> on by default. If the user does not specify -msg then
> enable_timestmap_msg is off. Did I get that right?
Yes.
>
> This means that the default behavior of QEMU does not change but you can
> add -msg to enable timestamps.
>
> I'm happy with this but find the documentation confusing.
I can remove "(disabled by default)" if needed.
>
> > diff --git a/util/qemu-time.c b/util/qemu-time.c
> > new file mode 100644
> > index 0000000..3862788
> > --- /dev/null
> > +++ b/util/qemu-time.c
> > @@ -0,0 +1,26 @@
> > +/*
> > + * Time handling
> > + *
> > + * Copyright (C) 2013 Hitachi Data Systems Corp.
> > + *
> > + * Authors:
> > + * Seiji Aguchi <seiji.agu...@hds.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include "qemu/time.h"
> > +
> > +void qemu_get_timestamp_str(char *timestr, size_t len)
> > +{
> > + GTimeVal tv;
> > + gchar *tmp_str = NULL;
> > +
> > + /* Size of len should be at least TIMESTAMP_LEN to avoid truncation */
> > + assert(len >= TIMESTAMP_LEN);
> > +
> > + g_get_current_time(&tv);
> > + tmp_str = g_time_val_to_iso8601(&tv);
> > + g_strlcpy((gchar *)timestr, tmp_str, len);
> > + g_free(tmp_str);
> > +}
>
> Why strcpy into a fixed-size buffer when glib gives us a heap-allocated
> string?
>
> This patch would be simpler by dropping qemu-time.c/time.h
I plan to introduce timestamp to monitor_printf() and fprintf()
near future.
In this case, it is better from a maintenance POV to keep time-handling
functionality in a file that's separate from the qemu error file, so it can
be reused
elsewhere in QEMU if needed.
It is suggested by Daniel.
http://marc.info/?l=qemu-devel&m=136741113218944&w=2
> and simply
> doing:
>
> if (enable_timestamp_msg) {
> GTimeVal tv;
> gchar *timestamp;
>
> g_get_current_time(&tv);
> timestamp = g_time_val_to_iso8601(&tv);
> error_printf("%s ", timestamp);
> g_free(timestamp);
I'm concerned that we may have potential memory leak
If someone neglect to call the g_free().
That is why I use the fixed-size buffer.
Seiji
