diff mbox series

[ndctl,v4,1/4] cxl/monitor: Enable default_log and refactor sanity check

Message ID 20230711115344.562823-2-lizhijian@fujitsu.com (mailing list archive)
State Accepted
Commit 023706a02fed53ec3675d0eba4c0239a687981a4
Headers show
Series cxl/monitor and ndctl/monitor fixes | expand

Commit Message

Li Zhijian July 11, 2023, 11:53 a.m. UTC
The default_log(/var/log/cxl-monitor.log) should be used when no '-l'
argument is specified in daemon mode, but it was not working at all.

Simplify the sanity checks so that the default log file is assigned
correctly, and the behavior is consistent with the documentation.

Remove the unnecessary fix_filename() for monitor.log since
parse_options_prefix() has done similar stuff if needed.

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
V4: add reviewed tag and minor fixes: comment style and change log.
V2: exchange order of previous patch1 and patch2 # Alison
    a few commit log updated
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
 cxl/monitor.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

Comments

Alison Schofield July 11, 2023, 5:57 p.m. UTC | #1
On Tue, Jul 11, 2023 at 07:53:41PM +0800, Li Zhijian wrote:
> The default_log(/var/log/cxl-monitor.log) should be used when no '-l'
> argument is specified in daemon mode, but it was not working at all.
> 
> Simplify the sanity checks so that the default log file is assigned
> correctly, and the behavior is consistent with the documentation.
> 
> Remove the unnecessary fix_filename() for monitor.log since
> parse_options_prefix() has done similar stuff if needed.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> V4: add reviewed tag and minor fixes: comment style and change log.
> V2: exchange order of previous patch1 and patch2 # Alison
>     a few commit log updated
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
>  cxl/monitor.c | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/cxl/monitor.c b/cxl/monitor.c
> index e3469b9a4792..d8245ed8d0e9 100644
> --- a/cxl/monitor.c
> +++ b/cxl/monitor.c
> @@ -164,6 +164,7 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx)
>  	};
>  	const char *prefix ="./";
>  	int rc = 0, i;
> +	const char *log;
>  
>  	argc = parse_options_prefix(argc, argv, prefix, options, u, 0);
>  	for (i = 0; i < argc; i++)
> @@ -171,32 +172,33 @@ int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx)
>  	if (argc)
>  		usage_with_options(u, options);
>  
> +	/* sanity check */
> +	if (monitor.daemon && monitor.log && !strncmp(monitor.log, "./", 2)) {
> +		error("relative path or 'standard' are not compatible with daemon mode\n");
> +		return -EINVAL;
> +	}
> +
>  	log_init(&monitor.ctx, "cxl/monitor", "CXL_MONITOR_LOG");
> -	monitor.ctx.log_fn = log_standard;
> +	if (monitor.log)
> +		log = monitor.log;
> +	else
> +		log = monitor.daemon ? default_log : "./standard";
>  
>  	if (monitor.verbose)
>  		monitor.ctx.log_priority = LOG_DEBUG;
>  	else
>  		monitor.ctx.log_priority = LOG_INFO;
>  
> -	if (monitor.log) {
> -		if (strncmp(monitor.log, "./", 2) != 0)
> -			fix_filename(prefix, (const char **)&monitor.log);
> -		if (strncmp(monitor.log, "./standard", 10) == 0 && !monitor.daemon) {
> -			monitor.ctx.log_fn = log_standard;
> -		} else {
> -			const char *log = monitor.log;
> -
> -			if (!monitor.log)
> -				log = default_log;
> -			monitor.log_file = fopen(log, "a+");
> -			if (!monitor.log_file) {
> -				rc = -errno;
> -				error("open %s failed: %d\n", monitor.log, rc);
> -				goto out;
> -			}
> -			monitor.ctx.log_fn = log_file;
> +	if (strncmp(log, "./standard", 10) == 0)
> +		monitor.ctx.log_fn = log_standard;
> +	else {
> +		monitor.log_file = fopen(log, "a+");
> +		if (!monitor.log_file) {
> +			rc = -errno;
> +			error("open %s failed: %d\n", log, rc);
> +			goto out;
>  		}
> +		monitor.ctx.log_fn = log_file;
>  	}
>  
>  	if (monitor.daemon) {
> -- 
> 2.29.2
>
diff mbox series

Patch

diff --git a/cxl/monitor.c b/cxl/monitor.c
index e3469b9a4792..d8245ed8d0e9 100644
--- a/cxl/monitor.c
+++ b/cxl/monitor.c
@@ -164,6 +164,7 @@  int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx)
 	};
 	const char *prefix ="./";
 	int rc = 0, i;
+	const char *log;
 
 	argc = parse_options_prefix(argc, argv, prefix, options, u, 0);
 	for (i = 0; i < argc; i++)
@@ -171,32 +172,33 @@  int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx)
 	if (argc)
 		usage_with_options(u, options);
 
+	/* sanity check */
+	if (monitor.daemon && monitor.log && !strncmp(monitor.log, "./", 2)) {
+		error("relative path or 'standard' are not compatible with daemon mode\n");
+		return -EINVAL;
+	}
+
 	log_init(&monitor.ctx, "cxl/monitor", "CXL_MONITOR_LOG");
-	monitor.ctx.log_fn = log_standard;
+	if (monitor.log)
+		log = monitor.log;
+	else
+		log = monitor.daemon ? default_log : "./standard";
 
 	if (monitor.verbose)
 		monitor.ctx.log_priority = LOG_DEBUG;
 	else
 		monitor.ctx.log_priority = LOG_INFO;
 
-	if (monitor.log) {
-		if (strncmp(monitor.log, "./", 2) != 0)
-			fix_filename(prefix, (const char **)&monitor.log);
-		if (strncmp(monitor.log, "./standard", 10) == 0 && !monitor.daemon) {
-			monitor.ctx.log_fn = log_standard;
-		} else {
-			const char *log = monitor.log;
-
-			if (!monitor.log)
-				log = default_log;
-			monitor.log_file = fopen(log, "a+");
-			if (!monitor.log_file) {
-				rc = -errno;
-				error("open %s failed: %d\n", monitor.log, rc);
-				goto out;
-			}
-			monitor.ctx.log_fn = log_file;
+	if (strncmp(log, "./standard", 10) == 0)
+		monitor.ctx.log_fn = log_standard;
+	else {
+		monitor.log_file = fopen(log, "a+");
+		if (!monitor.log_file) {
+			rc = -errno;
+			error("open %s failed: %d\n", log, rc);
+			goto out;
 		}
+		monitor.ctx.log_fn = log_file;
 	}
 
 	if (monitor.daemon) {