From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50973) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UufE1-00080H-9m for qemu-devel@nongnu.org; Thu, 04 Jul 2013 04:49:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UufDz-0001iX-UB for qemu-devel@nongnu.org; Thu, 04 Jul 2013 04:49:13 -0400 Received: from mail-ee0-x22e.google.com ([2a00:1450:4013:c00::22e]:41442) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UufDz-0001i8-KN for qemu-devel@nongnu.org; Thu, 04 Jul 2013 04:49:11 -0400 Received: by mail-ee0-f46.google.com with SMTP id d41so600938eek.19 for ; Thu, 04 Jul 2013 01:49:10 -0700 (PDT) Date: Thu, 4 Jul 2013 10:49:07 +0200 From: Stefan Hajnoczi Message-ID: <20130704084907.GC2945@stefanha-thinkpad.redhat.com> References: <51D1D04F.3010201@hds.com> <20130702090926.GC8393@stefanha-thinkpad.redhat.com> <20130703091342.GD17614@stefanha-thinkpad.muc.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 , "pbonzini@redhat.com" , "lcapitulino@redhat.com" , "lersek@redhat.com" On Thu, Jul 04, 2013 at 02:57:13AM +0000, Seiji Aguchi wrote: > > > > -----Original Message----- > > From: Stefan Hajnoczi [mailto:stefanha@gmail.com] > > Sent: Wednesday, July 03, 2013 5:14 AM > > To: Seiji Aguchi > > Cc: qemu-devel@nongnu.org; aliguori@us.ibm.com; berrange@redhat.com; kwolf@redhat.com; mtosatti@redhat.com; > > armbru@redhat.com; Tomoki Sekiyama; pbonzini@redhat.com; lcapitulino@redhat.com; lersek@redhat.com; eblake@redhat.com; > > dle-develop@lists.sourceforge.net > > Subject: Re: [PATCH v5] Add timestamp to error_report() > > > > On Tue, Jul 02, 2013 at 02:09:24PM +0000, Seiji Aguchi wrote: > > > > > > > > > > > +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. > > > > Perhaps the simplest solution is timestamp=off by default. Then there > > can be no confusion and users must do -msg timestamp=on to enable > > timestamps. > > > > If you really want to keep -msg as a shortcut for -msg timestamp=on, > > then please document explicitly that: > > 1. Without -msg timestamps are off. > > 1. With -msg timestamps are on. > > 2. -msg timestamp=off can be used to turn timestamps off again. > > My apologies for the confusion. > > The syntax, "-msg [timestamp=on|off]", was wrong. > "-msg timestamp[=on|off]" is correct. > > And there is no way to make "timestamp" optional, as far as I looked into a source code. > Therefore, the explanation should be as below. > (I think it is reasonable to keep "-msg timestamp" as a shortcut for -msg timestamp=on.) Yes, I think you are correct. I thought previously that "-msg" works but it seems an option is always required. > > +DEF("msg", HAS_ARG, QEMU_OPTION_msg, > + "-msg timestamp[=on|off]\n" > + " change the format of messages\n" > + " on|off controls leading timestamps (default:on)\n", > + QEMU_ARCH_ALL) > +STEXI > +@item -msg timestamp[=on|off] > +@findex -msg > +prepend a timestamp to each log message.(default:on) > +ETEXI > > > To be simpler, we may be able to introduce just a single -msg-timestamp. > But I think current "-msg timestamp[=on|off]" is reasonable > because other options may be introduced to msg, like log_level or debug. Yep. Stefan