[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5] Add timestamp to error_report()
From: |
Seiji Aguchi |
Subject: |
Re: [Qemu-devel] [PATCH v5] Add timestamp to error_report() |
Date: |
Thu, 4 Jul 2013 02:57:13 +0000 |
> -----Original Message-----
> From: Stefan Hajnoczi [mailto:address@hidden
> Sent: Wednesday, July 03, 2013 5:14 AM
> To: Seiji Aguchi
> Cc: address@hidden; address@hidden; address@hidden; address@hidden;
> address@hidden;
> address@hidden; Tomoki Sekiyama; address@hidden; address@hidden;
> address@hidden; address@hidden;
> address@hidden
> 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
> > > > address@hidden -msg timestamp=on|off
> > > > address@hidden -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.)
<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
address@hidden -msg timestamp[=on|off]
address@hidden -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.
Seiji