diff mbox series

[RFC,ndctl,3/3] ndctl, rename monitor.conf to ndctl.conf

Message ID 20210516231427.64162-4-qi.fuli@fujitsu.com (mailing list archive)
State New
Headers show
Series Rename monitor.conf to ndctl.conf as a ndctl global config file | expand

Commit Message

QI Fuli May 16, 2021, 11:14 p.m. UTC
From: QI Fuli <qi.fuli@fujitsu.com>

Rename monitor.conf to ndctl.conf, and make it a ndclt global
configuration file that all commands can refer to.
Refactor monitor to make it work with ndctl.conf.

Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
---
 configure.ac                       |   8 +-
 ndctl/Makefile.am                  |   9 +-
 ndctl/monitor.c                    | 127 +++++------------------------
 ndctl/{monitor.conf => ndctl.conf} |  16 +++-
 4 files changed, 40 insertions(+), 120 deletions(-)
 rename ndctl/{monitor.conf => ndctl.conf} (82%)

Comments

Verma, Vishal L June 2, 2021, 6:33 a.m. UTC | #1
On Mon, 2021-05-17 at 08:14 +0900, QI Fuli wrote:
> From: QI Fuli <qi.fuli@fujitsu.com>
> 
> Rename monitor.conf to ndctl.conf, and make it a ndclt global
> configuration file that all commands can refer to.
> Refactor monitor to make it work with ndctl.conf.
> 
> Signed-off-by: QI Fuli <qi.fuli@fujitsu.com>
> ---
>  configure.ac                       |   8 +-
>  ndctl/Makefile.am                  |   9 +-
>  ndctl/monitor.c                    | 127 +++++------------------------
>  ndctl/{monitor.conf => ndctl.conf} |  16 +++-
>  4 files changed, 40 insertions(+), 120 deletions(-)
>  rename ndctl/{monitor.conf => ndctl.conf} (82%)
> 

[snip]

> @@ -601,6 +496,19 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
>  		"ndctl monitor [<options>]",
>  		NULL
>  	};
> +	const struct config configs[] = {
> +		CONF_STR("core:bus", &param.bus, NULL),
> +		CONF_STR("core:region", &param.region, NULL),
> +		CONF_STR("core:dimm", &param.dimm, NULL),
> +		CONF_STR("core:namespace", &param.namespace, NULL),
> +		CONF_STR("monitor:bus", &param.bus, NULL),
> +		CONF_STR("monitor:region", &param.region, NULL),
> +		CONF_STR("monitor:dimm", &param.dimm, NULL),
> +		CONF_STR("monitor:namespace", &param.namespace, NULL),
> +		CONF_STR("monitor:dimm-event", &monitor.dimm_event, NULL),
> +		//CONF_FILE("monitor:log", &monitor.log, NULL),
> +		CONF_END(),
> +	};
>  	const char *prefix = "./";
>  	struct util_filter_ctx fctx = { 0 };
>  	struct monitor_filter_arg mfa = { 0 };
> @@ -621,7 +529,10 @@ int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
>  	else
>  		monitor.ctx.log_priority = LOG_INFO;
>  
> -	rc = read_config_file(ctx, &monitor, &param);
> +	if (monitor.config_file)
> +		rc = parse_configs(monitor.config_file, configs);
> +	else
> +		rc = parse_configs(NDCTL_CONF_FILE, configs);
>  	if (rc)
>  		goto out;

If I'm reading this right, it looks like this is set up so that params
from the config file will override any params from the command line,
which I think can be surprising behavior. The command line should
always override anything in the config file.

I suspect we'll need to collect CLI params in a 'param_cli' structure,
config params in a 'param_config' structure, and then consolidate them
to create the final 'param' structure that gets passed to
util_filter_walk etc.
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index 5ec8d2f..ab2d8a3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -171,10 +171,10 @@  fi
 AC_SUBST([systemd_unitdir])
 AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
 
-ndctl_monitorconfdir=${sysconfdir}/ndctl
-ndctl_monitorconf=monitor.conf
-AC_SUBST([ndctl_monitorconfdir])
-AC_SUBST([ndctl_monitorconf])
+ndctl_confdir=${sysconfdir}/ndctl
+ndctl_conf=ndctl.conf
+AC_SUBST([ndctl_confdir])
+AC_SUBST([ndctl_conf])
 
 daxctl_modprobe_datadir=${datadir}/daxctl
 daxctl_modprobe_data=daxctl.conf
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index a63b1e0..b107b3d 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -7,7 +7,7 @@  BUILT_SOURCES = config.h
 config.h: $(srcdir)/Makefile.am
 	$(AM_V_GEN) echo "/* Autogenerated by ndctl/Makefile.am */" >$@ && \
 	echo '#define NDCTL_CONF_FILE \
-		"$(ndctl_monitorconfdir)/$(ndctl_monitorconf)"' >>$@
+		"$(ndctl_confdir)/$(ndctl_conf)"' >>$@
 	$(AM_V_GEN) echo '#define NDCTL_KEYS_DIR  "$(ndctl_keysdir)"' >>$@
 
 ndctl_SOURCES = ndctl.c \
@@ -42,7 +42,7 @@  keys_configdir = $(ndctl_keysdir)
 keys_config_DATA = $(ndctl_keysreadme)
 endif
 
-EXTRA_DIST += keys.readme monitor.conf ndctl-monitor.service
+EXTRA_DIST += keys.readme ndctl.conf ndctl-monitor.service
 
 if ENABLE_DESTRUCTIVE
 ndctl_SOURCES += ../test/blk_namespaces.c \
@@ -54,6 +54,7 @@  ndctl_LDADD =\
 	lib/libndctl.la \
 	../daxctl/lib/libdaxctl.la \
 	../libutil.a \
+	../libccan.a \
 	$(UUID_LIBS) \
 	$(KMOD_LIBS) \
 	$(JSON_LIBS)
@@ -73,8 +74,8 @@  ndctl_SOURCES += ../test/libndctl.c \
 		 test.c
 endif
 
-monitor_configdir = $(ndctl_monitorconfdir)
-monitor_config_DATA = $(ndctl_monitorconf)
+ndctl_configdir = $(ndctl_confdir)
+ndctl_config_DATA = $(ndctl_conf)
 
 if ENABLE_SYSTEMD_UNITS
 systemd_unit_DATA = ndctl-monitor.service
diff --git a/ndctl/monitor.c b/ndctl/monitor.c
index ca36179..ea707d2 100644
--- a/ndctl/monitor.c
+++ b/ndctl/monitor.c
@@ -10,11 +10,13 @@ 
 #include <util/filter.h>
 #include <util/util.h>
 #include <util/parse-options.h>
+#include <util/parse-configs.h>
 #include <util/strbuf.h>
 #include <ndctl/config.h>
 #include <ndctl/ndctl.h>
 #include <ndctl/libndctl.h>
 #include <sys/epoll.h>
+#include <stdbool.h>
 #define BUF_SIZE 2048
 
 /* reuse the core log helpers for the monitor logger */
@@ -463,113 +465,6 @@  out:
 	return rc;
 }
 
-static void parse_config(const char **arg, char *key, char *val, char *ident)
-{
-	struct strbuf value = STRBUF_INIT;
-	size_t arg_len = *arg ? strlen(*arg) : 0;
-
-	if (!ident || !key || (strcmp(ident, key) != 0))
-		return;
-
-	if (arg_len) {
-		strbuf_add(&value, *arg, arg_len);
-		strbuf_addstr(&value, " ");
-	}
-	strbuf_addstr(&value, val);
-	*arg = strbuf_detach(&value, NULL);
-}
-
-static int read_config_file(struct ndctl_ctx *ctx, struct monitor *_monitor,
-		struct util_filter_params *_param)
-{
-	FILE *f;
-	size_t len = 0;
-	int line = 0, rc = 0;
-	char *buf = NULL, *seek, *value, *config_file;
-
-	if (_monitor->config_file)
-		config_file = strdup(_monitor->config_file);
-	else
-		config_file = strdup(NDCTL_CONF_FILE);
-	if (!config_file) {
-		fail("strdup default config file failed\n");
-		rc = -ENOMEM;
-		goto out;
-	}
-
-	buf = malloc(BUF_SIZE);
-	if (!buf) {
-		fail("malloc read config-file buf error\n");
-		rc = -ENOMEM;
-		goto out;
-	}
-	seek = buf;
-
-	f = fopen(config_file, "r");
-	if (!f) {
-		if (_monitor->config_file) {
-			err(&monitor, "config-file: %s cannot be opened\n",
-				config_file);
-			rc = -errno;
-		}
-		goto out;
-	}
-
-	while (fgets(seek, BUF_SIZE, f)) {
-		value = NULL;
-		line++;
-
-		while (isspace(*seek))
-			seek++;
-
-		if (*seek == '#' || *seek == '\0')
-			continue;
-
-		value = strchr(seek, '=');
-		if (!value) {
-			fail("config-file syntax error, skip line[%i]\n", line);
-			continue;
-		}
-
-		value[0] = '\0';
-		value++;
-
-		while (isspace(value[0]))
-			value++;
-
-		len = strlen(seek);
-		if (len == 0)
-			continue;
-		while (isspace(seek[len-1]))
-			len--;
-		seek[len] = '\0';
-
-		len = strlen(value);
-		if (len == 0)
-			continue;
-		while (isspace(value[len-1]))
-			len--;
-		value[len] = '\0';
-
-		if (len == 0)
-			continue;
-
-		parse_config(&_param->bus, "bus", value, seek);
-		parse_config(&_param->dimm, "dimm", value, seek);
-		parse_config(&_param->region, "region", value, seek);
-		parse_config(&_param->namespace, "namespace", value, seek);
-		parse_config(&_monitor->dimm_event, "dimm-event", value, seek);
-
-		if (!_monitor->log)
-			parse_config(&_monitor->log, "log", value, seek);
-	}
-	fclose(f);
-out:
-	free(buf);
-	free(config_file);
-	return rc;
-}
-
 int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
 {
 	const struct option options[] = {
@@ -601,6 +496,19 @@  int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
 		"ndctl monitor [<options>]",
 		NULL
 	};
+	const struct config configs[] = {
+		CONF_STR("core:bus", &param.bus, NULL),
+		CONF_STR("core:region", &param.region, NULL),
+		CONF_STR("core:dimm", &param.dimm, NULL),
+		CONF_STR("core:namespace", &param.namespace, NULL),
+		CONF_STR("monitor:bus", &param.bus, NULL),
+		CONF_STR("monitor:region", &param.region, NULL),
+		CONF_STR("monitor:dimm", &param.dimm, NULL),
+		CONF_STR("monitor:namespace", &param.namespace, NULL),
+		CONF_STR("monitor:dimm-event", &monitor.dimm_event, NULL),
+		//CONF_FILE("monitor:log", &monitor.log, NULL),
+		CONF_END(),
+	};
 	const char *prefix = "./";
 	struct util_filter_ctx fctx = { 0 };
 	struct monitor_filter_arg mfa = { 0 };
@@ -621,7 +529,10 @@  int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx)
 	else
 		monitor.ctx.log_priority = LOG_INFO;
 
-	rc = read_config_file(ctx, &monitor, &param);
+	if (monitor.config_file)
+		rc = parse_configs(monitor.config_file, configs);
+	else
+		rc = parse_configs(NDCTL_CONF_FILE, configs);
 	if (rc)
 		goto out;
 
diff --git a/ndctl/monitor.conf b/ndctl/ndctl.conf
similarity index 82%
rename from ndctl/monitor.conf
rename to ndctl/ndctl.conf
index 934e2c0..85343b0 100644
--- a/ndctl/monitor.conf
+++ b/ndctl/ndctl.conf
@@ -1,14 +1,22 @@ 
-# This is the main ndctl monitor configuration file. It contains the
-# configuration directives that give ndctl monitor instructions.
-# You can change the configuration of ndctl monitor by editing this
+# This is the ndctl global configuration file. It contains the
+# configuration directives that give ndctl instructions.
+# You can change the configuration of ndctl by editing this
 # file or using [--config-file=<file>] option to override this one.
-# The changed value will work after restart ndctl monitor service.
+# The changed value will work after restart ndctl services.
 
 # In this file, lines starting with a hash (#) are comments.
 # The configurations should follow <key> = <value> style.
 # Multiple space-separated values are allowed, but except the following
 # characters: : ? / \ % " ' $ & ! * { } [ ] ( ) = < > @
 
+[core]
+# The values in the [core] section work for all ndctl commands.
+# dimm = all
+# bus = all
+# region = all
+# namespace = all
+
+[monitor]
 # The objects to monitor are filtered via dimm's name by setting key "dimm".
 # If this value is different from the value of [--dimm=<value>] option,
 # both of the values will work.