| Message ID | 20180807131756.23673-4-qi.fuli@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some fixups and improvements ndctl monitor | expand |
Hi Qi, On 08/07/2018 09:17 AM, QI Fuli wrote: > This patch is used to add timestamp and process id to log messages when logging > messages to a file. > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > --- > ndctl/monitor.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/ndctl/monitor.c b/ndctl/monitor.c > index 2f3d751..d29e378 100644 > --- a/ndctl/monitor.c > +++ b/ndctl/monitor.c > @@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority, const char *file, > { > FILE *f; > char *buf; > + struct timespec ts; > + char timestamp[32]; > > if (vasprintf(&buf, format, args) < 0) { > fail("vasprintf error\n"); > @@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int priority, const char *file, > notice(ctx, "%s\n", buf); > goto end; > } > - fprintf(f, "%s", buf); > + > + if (priority != LOG_NOTICE) { Why is the timestamp not needed in case of LOG_NOTICE...? > + clock_gettime(CLOCK_REALTIME, &ts); > + sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec); > + fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf); What is the pid for...? Thanks, Masa > + } else > + fprintf(f, "%s", buf); > +> fflush(f); > fclose(f); > end: >
> -----Original Message----- > From: Masayoshi Mizuma [mailto:msys.mizuma@gmail.com] > Sent: Wednesday, August 8, 2018 3:40 AM > To: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>; linux-nvdimm@lists.01.org > Subject: Re: [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log messages > in log_file() > > Hi Qi, > > On 08/07/2018 09:17 AM, QI Fuli wrote: > > This patch is used to add timestamp and process id to log messages > > when logging messages to a file. > > > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > --- > > ndctl/monitor.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index 2f3d751..d29e378 > > 100644 > > --- a/ndctl/monitor.c > > +++ b/ndctl/monitor.c > > @@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int > > priority, const char *file, { > > FILE *f; > > char *buf; > > + struct timespec ts; > > + char timestamp[32]; > > > > if (vasprintf(&buf, format, args) < 0) { > > fail("vasprintf error\n"); > > @@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int priority, const > char *file, > > notice(ctx, "%s\n", buf); > > goto end; > > } > > - fprintf(f, "%s", buf); > > + > > + if (priority != LOG_NOTICE) { > Hi Masa, Thanks for your comments. > Why is the timestamp not needed in case of LOG_NOTICE...? Because LOG_NOTICE level is only used for smart event notification in monitor. Since the timestamp and pid are already added to json format messages by notify_dimm_event(), so there is no need to add them again. > > > + clock_gettime(CLOCK_REALTIME, &ts); > > + sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec); > > + fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf); > > What is the pid for...? > Multiple monitor can be run at the same time and they may send messages to a common log file. In this case, the pid could help users to know the monitor where each message came from. Thanks, QI > Thanks, > Masa > > > + } else > > + fprintf(f, "%s", buf); > > +> fflush(f); > > fclose(f); > > end: > > >
On 08/07/2018 08:54 PM, Qi, Fuli wrote: >> -----Original Message----- >> From: Masayoshi Mizuma [mailto:msys.mizuma@gmail.com] >> Sent: Wednesday, August 8, 2018 3:40 AM >> To: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>; linux-nvdimm@lists.01.org >> Subject: Re: [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log messages >> in log_file() >> >> Hi Qi, >> >> On 08/07/2018 09:17 AM, QI Fuli wrote: >>> This patch is used to add timestamp and process id to log messages >>> when logging messages to a file. >>> >>> Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> >>> --- >>> ndctl/monitor.c | 11 ++++++++++- >>> 1 file changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/ndctl/monitor.c b/ndctl/monitor.c index 2f3d751..d29e378 >>> 100644 >>> --- a/ndctl/monitor.c >>> +++ b/ndctl/monitor.c >>> @@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int >>> priority, const char *file, { >>> FILE *f; >>> char *buf; >>> + struct timespec ts; >>> + char timestamp[32]; >>> >>> if (vasprintf(&buf, format, args) < 0) { >>> fail("vasprintf error\n"); >>> @@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int priority, const >> char *file, >>> notice(ctx, "%s\n", buf); >>> goto end; >>> } >>> - fprintf(f, "%s", buf); >>> + >>> + if (priority != LOG_NOTICE) { >> > Hi Masa, > > Thanks for your comments. > >> Why is the timestamp not needed in case of LOG_NOTICE...? > > Because LOG_NOTICE level is only used for smart event notification in monitor. > Since the timestamp and pid are already added to json format messages by notify_dimm_event(), > so there is no need to add them again. I see, thanks. > >> >>> + clock_gettime(CLOCK_REALTIME, &ts); >>> + sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec); >>> + fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf); >> >> What is the pid for...? >> > Multiple monitor can be run at the same time and they may send messages to a common log file. > In this case, the pid could help users to know the monitor where each message came from. OK, got it. Your patch is good to me. Please feel free to add: Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks, Masa > > Thanks, > QI > >> Thanks, >> Masa >> >>> + } else >>> + fprintf(f, "%s", buf); >>> +> fflush(f); >>> fclose(f); >>> end: >>> >> >
diff --git a/ndctl/monitor.c b/ndctl/monitor.c index 2f3d751..d29e378 100644 --- a/ndctl/monitor.c +++ b/ndctl/monitor.c @@ -84,6 +84,8 @@ static void log_file(struct ndctl_ctx *ctx, int priority, const char *file, { FILE *f; char *buf; + struct timespec ts; + char timestamp[32]; if (vasprintf(&buf, format, args) < 0) { fail("vasprintf error\n"); @@ -99,7 +101,14 @@ static void log_file(struct ndctl_ctx *ctx, int priority, const char *file, notice(ctx, "%s\n", buf); goto end; } - fprintf(f, "%s", buf); + + if (priority != LOG_NOTICE) { + clock_gettime(CLOCK_REALTIME, &ts); + sprintf(timestamp, "%10ld.%09ld", ts.tv_sec, ts.tv_nsec); + fprintf(f, "[%s] [%d] %s", timestamp, getpid(), buf); + } else + fprintf(f, "%s", buf); + fflush(f); fclose(f); end:
This patch is used to add timestamp and process id to log messages when logging messages to a file. Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> --- ndctl/monitor.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)