On Thu, Jul 04, 2013 at 02:57:13AM +0000, Seiji Aguchi wrote:
>
>
> > -----Original Message-----
> > From: Stefan Hajnoczi [mailto:stefa...@gmail.com]
> > Sent: Wednesday, July 03, 2013 5:14 AM
> > To: Seiji Aguchi
> > Cc: qemu-devel@nongnu.org; aligu...@us.ibm.com; berra...@redhat.com;
> > kw...@redhat.com; mtosa...@redhat.com;
> > arm...@redhat.com; Tomoki Sekiyama; pbonz...@redhat.com;
> > lcapitul...@redhat.com; ler...@redhat.com; ebl...@redhat.com;
> > dle-deve...@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.
> <snip>
> +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
> <snip>
>
> 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
