From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Utwmd-0003RT-WB for qemu-devel@nongnu.org; Tue, 02 Jul 2013 05:22:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UtwmZ-0002Ih-4Q for qemu-devel@nongnu.org; Tue, 02 Jul 2013 05:21:59 -0400 Received: from mail-wi0-x236.google.com ([2a00:1450:400c:c05::236]:39712) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UtwaY-0005rS-Q7 for qemu-devel@nongnu.org; Tue, 02 Jul 2013 05:09:31 -0400 Received: by mail-wi0-f182.google.com with SMTP id m6so3970503wiv.3 for ; Tue, 02 Jul 2013 02:09:29 -0700 (PDT) Date: Tue, 2 Jul 2013 11:09:26 +0200 From: Stefan Hajnoczi Message-ID: <20130702090926.GC8393@stefanha-thinkpad.redhat.com> References: <51D1D04F.3010201@hds.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51D1D04F.3010201@hds.com> Subject: Re: [Qemu-devel] [PATCH v5] Add timestamp to error_report() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Seiji Aguchi Cc: kwolf@redhat.com, aliguori@us.ibm.com, mtosatti@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, dle-develop@lists.sourceforge.net, tomoki.sekiyama@hds.com, pbonzini@redhat.com, lcapitulino@redhat.com, lersek@redhat.com On Mon, Jul 01, 2013 at 02:54:07PM -0400, Seiji Aguchi wrote: > diff --git a/qemu-options.hx b/qemu-options.hx > index ca6fdf6..a6dac1a 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -3102,3 +3102,15 @@ HXCOMM This is the last statement. Insert new options before this line! > STEXI > @end table > ETEXI > + > +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? 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. > 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 > + * > + * 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 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); }