* [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log @ 2018-07-31 5:15 QI Fuli 2018-08-01 18:14 ` Masayoshi Mizuma 0 siblings, 1 reply; 5+ messages in thread From: QI Fuli @ 2018-07-31 5:15 UTC (permalink / raw) To: linux-nvdimm When a monitor runs with [--log] option, the prefix will be dually added to monitor.log. Therefore, the monitor cannot log the smart notification to syslog. This patch is used to prevent prefix from being dually added to monitor.log. Fixes: fdf6b6844ccf ("ndctl, monitor: add a new command - monitor") Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> --- ndctl/monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ndctl/monitor.c b/ndctl/monitor.c index c6419ad..4e5daf5 100644 --- a/ndctl/monitor.c +++ b/ndctl/monitor.c @@ -614,7 +614,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx) goto out; if (monitor.log) { - fix_filename(prefix, (const char **)&monitor.log); + if (strncmp(monitor.log, "./", 2) != 0) + fix_filename(prefix, (const char **)&monitor.log); if (strncmp(monitor.log, "./syslog", 8) == 0) ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog); else if (strncmp(monitor.log, "./standard", 10) == 0) -- 2.18.0 _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log 2018-07-31 5:15 [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log QI Fuli @ 2018-08-01 18:14 ` Masayoshi Mizuma 2018-08-02 9:42 ` Qi, Fuli 0 siblings, 1 reply; 5+ messages in thread From: Masayoshi Mizuma @ 2018-08-01 18:14 UTC (permalink / raw) To: qi.fuli, linux-nvdimm Hi QI, On 07/31/2018 01:15 AM, QI Fuli wrote: > When a monitor runs with [--log] option, the prefix will be dually > added to monitor.log. Therefore, the monitor cannot log the smart > notification to syslog. This patch is used to prevent prefix from > being dually added to monitor.log. > > Fixes: fdf6b6844ccf ("ndctl, monitor: add a new command - monitor") > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > --- > ndctl/monitor.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/ndctl/monitor.c b/ndctl/monitor.c > index c6419ad..4e5daf5 100644 > --- a/ndctl/monitor.c > +++ b/ndctl/monitor.c > @@ -614,7 +614,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx) > goto out; > > if (monitor.log) { > - fix_filename(prefix, (const char **)&monitor.log); > + if (strncmp(monitor.log, "./", 2) != 0) > + fix_filename(prefix, (const char **)&monitor.log); prefix is not needed to 'syslog' and 'standard', so why don't you move the strncmp() before fix_filename(), like as: @@ -614,13 +619,14 @@ int cmd_monitor(int argc, const char **argv, void *ctx) goto out; if (monitor.log) { - fix_filename(prefix, (const char **)&monitor.log); if (strncmp(monitor.log, "./syslog", 8) == 0) ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog); else if (strncmp(monitor.log, "./standard", 10) == 0) ; /*default, already set */ - else + else { + fix_filename(prefix, (const char **)&monitor.log); ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file); + } } if (monitor.daemon) { Thanks, Masa > if (strncmp(monitor.log, "./syslog", 8) == 0) > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog); > else if (strncmp(monitor.log, "./standard", 10) == 0) > _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log 2018-08-01 18:14 ` Masayoshi Mizuma @ 2018-08-02 9:42 ` Qi, Fuli 2018-08-02 13:22 ` Masayoshi Mizuma 0 siblings, 1 reply; 5+ messages in thread From: Qi, Fuli @ 2018-08-02 9:42 UTC (permalink / raw) To: 'Masayoshi Mizuma', linux-nvdimm > -----Original Message----- > From: Masayoshi Mizuma [mailto:msys.mizuma@gmail.com] > Sent: Thursday, August 2, 2018 3:15 AM > To: Qi, Fuli/斉 福利 <qi.fuli@jp.fujitsu.com>; linux-nvdimm@lists.01.org > Subject: Re: [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log > > Hi QI, > > On 07/31/2018 01:15 AM, QI Fuli wrote: > > When a monitor runs with [--log] option, the prefix will be dually > > added to monitor.log. Therefore, the monitor cannot log the smart > > notification to syslog. This patch is used to prevent prefix from > > being dually added to monitor.log. > > > > Fixes: fdf6b6844ccf ("ndctl, monitor: add a new command - monitor") > > Signed-off-by: QI Fuli <qi.fuli@jp.fujitsu.com> > > --- > > ndctl/monitor.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index c6419ad..4e5daf5 > > 100644 > > --- a/ndctl/monitor.c > > +++ b/ndctl/monitor.c > > @@ -614,7 +614,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx) > > goto out; > > > > if (monitor.log) { > > - fix_filename(prefix, (const char **)&monitor.log); > > + if (strncmp(monitor.log, "./", 2) != 0) > > + fix_filename(prefix, (const char **)&monitor.log); > > prefix is not needed to 'syslog' and 'standard', so why don't you move the strncmp() > before fix_filename(), like as: > > @@ -614,13 +619,14 @@ int cmd_monitor(int argc, const char **argv, void *ctx) > goto out; > > if (monitor.log) { > - fix_filename(prefix, (const char **)&monitor.log); > if (strncmp(monitor.log, "./syslog", 8) == 0) > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog); > else if (strncmp(monitor.log, "./standard", 10) == 0) > ; /*default, already set */ > - else > + else { > + fix_filename(prefix, (const char > + **)&monitor.log); > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file); > + } > } > > if (monitor.daemon) { > > Thanks, > Masa Hi Masa, Thank you very much for your comments. There are two ways to set monitor.log. a) setting the argument of [--log] option b) setting the value of [log] key in configuration file When users set monitor.log by b, the prefix will not be added to monitor.log. Therefore, we should do fix_filename() before strncmp(). Thanks, QI > > > if (strncmp(monitor.log, "./syslog", 8) == 0) > > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog); > > else if (strncmp(monitor.log, "./standard", 10) == 0) > > > _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log 2018-08-02 9:42 ` Qi, Fuli @ 2018-08-02 13:22 ` Masayoshi Mizuma 2018-08-02 20:16 ` Verma, Vishal L 0 siblings, 1 reply; 5+ messages in thread From: Masayoshi Mizuma @ 2018-08-02 13:22 UTC (permalink / raw) To: qi.fuli, linux-nvdimm Hi QI, On 08/02/2018 05:42 AM, Qi, Fuli wrote: ... >>> diff --git a/ndctl/monitor.c b/ndctl/monitor.c index c6419ad..4e5daf5 >>> 100644 >>> --- a/ndctl/monitor.c >>> +++ b/ndctl/monitor.c >>> @@ -614,7 +614,8 @@ int cmd_monitor(int argc, const char **argv, void *ctx) >>> goto out; >>> >>> if (monitor.log) { >>> - fix_filename(prefix, (const char **)&monitor.log); >>> + if (strncmp(monitor.log, "./", 2) != 0) >>> + fix_filename(prefix, (const char **)&monitor.log); >> >> prefix is not needed to 'syslog' and 'standard', so why don't you move the strncmp() >> before fix_filename(), like as: >> >> @@ -614,13 +619,14 @@ int cmd_monitor(int argc, const char **argv, void *ctx) >> goto out; >> >> if (monitor.log) { >> - fix_filename(prefix, (const char **)&monitor.log); >> if (strncmp(monitor.log, "./syslog", 8) == 0) >> ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog); >> else if (strncmp(monitor.log, "./standard", 10) == 0) >> ; /*default, already set */ >> - else >> + else { >> + fix_filename(prefix, (const char >> + **)&monitor.log); >> ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file); >> + } >> } >> >> if (monitor.daemon) { >> >> Thanks, >> Masa > > Hi Masa, > > Thank you very much for your comments. > > There are two ways to set monitor.log. > a) setting the argument of [--log] option > b) setting the value of [log] key in configuration file > > When users set monitor.log by b, the prefix will not be added to monitor.log. > Therefore, we should do fix_filename() before strncmp(). Oh, my proposal patch does not cover in case of user set 'syslog' or 'standard'in config_file. I think your patch fixes the bug correctly, thanks. Please feel free to add: Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks, Masa _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log 2018-08-02 13:22 ` Masayoshi Mizuma @ 2018-08-02 20:16 ` Verma, Vishal L 0 siblings, 0 replies; 5+ messages in thread From: Verma, Vishal L @ 2018-08-02 20:16 UTC (permalink / raw) To: linux-nvdimm, qi.fuli, msys.mizuma On Thu, 2018-08-02 at 09:22 -0400, Masayoshi Mizuma wrote: > Hi QI, > > On 08/02/2018 05:42 AM, Qi, Fuli wrote: > ... > > > > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index > > > > c6419ad..4e5daf5 > > > > 100644 > > > > --- a/ndctl/monitor.c > > > > +++ b/ndctl/monitor.c > > > > @@ -614,7 +614,8 @@ int cmd_monitor(int argc, const char > > > > **argv, void *ctx) > > > > goto out; > > > > > > > > if (monitor.log) { > > > > - fix_filename(prefix, (const char > > > > **)&monitor.log); > > > > + if (strncmp(monitor.log, "./", 2) != 0) > > > > + fix_filename(prefix, (const char > > > > **)&monitor.log); > > > > > > prefix is not needed to 'syslog' and 'standard', so why don't you > > > move the strncmp() > > > before fix_filename(), like as: > > > > > > @@ -614,13 +619,14 @@ int cmd_monitor(int argc, const char > > > **argv, void *ctx) > > > goto out; > > > > > > if (monitor.log) { > > > - fix_filename(prefix, (const char > > > **)&monitor.log); > > > if (strncmp(monitor.log, "./syslog", 8) == 0) > > > ndctl_set_log_fn((struct ndctl_ctx *)ctx, > > > log_syslog); > > > else if (strncmp(monitor.log, "./standard", 10) > > > == 0) > > > ; /*default, already set */ > > > - else > > > + else { > > > + fix_filename(prefix, (const char > > > + **)&monitor.log); > > > ndctl_set_log_fn((struct ndctl_ctx *)ctx, > > > log_file); > > > + } > > > } > > > > > > if (monitor.daemon) { > > > > > > Thanks, > > > Masa > > > > Hi Masa, > > > > Thank you very much for your comments. > > > > There are two ways to set monitor.log. > > a) setting the argument of [--log] option > > b) setting the value of [log] key in configuration file > > > > When users set monitor.log by b, the prefix will not be added to > > monitor.log. > > Therefore, we should do fix_filename() before strncmp(). > > Oh, my proposal patch does not cover in case of user set 'syslog' or > 'standard'in config_file. > I think your patch fixes the bug correctly, thanks. > > Please feel free to add: > > Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> Thanks Qi, Masa - I've applied this. > > Thanks, > Masa > _______________________________________________ > Linux-nvdimm mailing list > Linux-nvdimm@lists.01.org > https://lists.01.org/mailman/listinfo/linux-nvdimm _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-02 20:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-31 5:15 [ndclt PATCH] ndctl, monitor: Fix duplicate prefix in monitor.log QI Fuli 2018-08-01 18:14 ` Masayoshi Mizuma 2018-08-02 9:42 ` Qi, Fuli 2018-08-02 13:22 ` Masayoshi Mizuma 2018-08-02 20:16 ` Verma, Vishal L
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).