nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Masayoshi Mizuma <msys.mizuma@gmail.com>
To: qi.fuli@jp.fujitsu.com, linux-nvdimm@lists.01.org
Subject: Re: [ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of invalid path of log file
Date: Tue, 7 Aug 2018 15:05:31 -0400	[thread overview]
Message-ID: <49365d81-6695-1c67-8026-d4ccb81285b0@gmail.com> (raw)
In-Reply-To: <20180807131756.23673-2-qi.fuli@jp.fujitsu.com>

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 <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 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);
> +		}

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.

Is my understanding correct? If so, make sense to me. 
Please feel free to add:

    Reviewed-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com>

Thanks,
Masa

>  	}
>  
>  	if (monitor.daemon) {
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

  reply	other threads:[~2018-08-07 19:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-07 13:17 [ndctl PATCH 0/4] Some fixups and improvements ndctl monitor QI Fuli
2018-08-07 13:17 ` [ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of invalid path of log file QI Fuli
2018-08-07 19:05   ` Masayoshi Mizuma [this message]
2018-08-08  1:01     ` Qi, Fuli
2018-08-07 13:17 ` [ndctl PATCH 2/4] ndctl, monitor: set default log destination to syslog if "--daemon" is specified QI Fuli
2018-08-07 19:30   ` Masayoshi Mizuma
2018-08-08  1:31     ` Qi, Fuli
2018-08-08 13:35       ` Masayoshi Mizuma
2018-08-08 15:59         ` Qi, Fuli
2018-08-08 16:24           ` Masayoshi Mizuma
2018-08-07 13:17 ` [ndctl PATCH 3/4] ndctl, monitor: add timestamp and pid to log messages in log_file() QI Fuli
2018-08-07 18:39   ` Masayoshi Mizuma
2018-08-08  0:54     ` Qi, Fuli
2018-08-08  1:24       ` Masayoshi Mizuma
2018-08-07 13:17 ` [ndctl PATCH 4/4] ndctl, monitor: add [Install] Section to systemd unit file of ndctl-monitor QI Fuli
2018-08-07 18:41   ` Masayoshi Mizuma
2018-08-10 20:42 ` [ndctl PATCH 0/4] Some fixups and improvements ndctl monitor Verma, Vishal L

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49365d81-6695-1c67-8026-d4ccb81285b0@gmail.com \
    --to=msys.mizuma@gmail.com \
    --cc=linux-nvdimm@lists.01.org \
    --cc=qi.fuli@jp.fujitsu.com \
    --subject='Re: [ndctl PATCH 1/4] ndctl, monitor: fix the lack of detection of invalid path of log file' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).