> -----Original Message----- > From: Masayoshi Mizuma [mailto:[email protected]] > Sent: Wednesday, August 8, 2018 4:06 AM > To: Qi, Fuli/斉 福利 <[email protected]>; [email protected] > Subject: Re: [ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of > invalid > path of log file > > Hi Qi, > > On 08/07/2018 09:17 AM, QI Fuli wrote: > > Currently the monitor can be started even with an invalid path of log file. > > This patch adds a detection of invalid path of log file when starting > > monitor. > > If the path of log file is invalid, the monitor will refuse to be started. > > > > Signed-off-by: QI Fuli <[email protected]> > > --- > > ndctl/monitor.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/ndctl/monitor.c b/ndctl/monitor.c index f10384b..bf1f1d3 > > 100644 > > --- a/ndctl/monitor.c > > +++ b/ndctl/monitor.c > > @@ -603,6 +603,7 @@ int cmd_monitor(int argc, const char **argv, void *ctx) > > struct util_filter_ctx fctx = { 0 }; > > struct monitor_filter_arg mfa = { 0 }; > > int i, rc; > > + FILE *f; > > > > argc = parse_options_prefix(argc, argv, prefix, options, u, 0); > > for (i = 0; i < argc; i++) { > > @@ -630,8 +631,16 @@ int cmd_monitor(int argc, const char **argv, void *ctx) > > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_syslog); > > else if (strncmp(monitor.log, "./standard", 10) == 0) > > ; /*default, already set */ > > - else > > + else { > > + f = fopen(monitor.log, "a+"); > > + if (!f) { > > + error("open %s failed\n", monitor.log); > > + rc = -errno; > > + goto out; > > + } > > + fclose(f); > > ndctl_set_log_fn((struct ndctl_ctx *)ctx, log_file); > > + } >
Masa, Thanks for your comments. > In log_file(), the log file does fallback to syslog if the fopen() fails. > In my understanding here is that the fallback is needed to save in case of the > monitor.log in trouble for example, the parent directory is removed. > And, the new fopen() check, you have added by this patch, to inform the > invalid log > path for users. > Yes, this is what I wanted to implement. > Is my understanding correct? If so, make sense to me. > Please feel free to add: > > Reviewed-by: Masayoshi Mizuma <[email protected]> > Thanks, QI > Thanks, > Masa > > > } > > > > if (monitor.daemon) { > > > _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
