[PATCH v3 0/8] nfs-utils: nfsconf cli tool and code tests

[PATCH v3 0/8] nfs-utils: nfsconf cli tool and code tests

[Justin Mitchell]

Apologies for the noise, I missed some files out when I last
reposted, so here is an updated and tested version.

v3
Include files that were missed out of v2
Add the man page for nfsconftool

v2
Updated style and commenting to make the code clearer

v1
This series introduces a new cli tool to nfs-utils which provides
for the dumping of the current configuration, querying of individual
settings in a scripting friendly manner and by extention the 
scripted testing and verification of nfsconf functionality.

The previously unreferenced conf_report() is modified to produce
a sorted output, which allows you to --dump the current config in
a way thats easier to diff

As well as some improved error messages a script is included which
tests the current config parsing code by supplying various example
config files and comparing the results of the --dump output.

The cli tool also allows a simple test to check if a config item
has been set, and to retrieve the current value in a way that is
easy to call from a shell script.

As the next stage I hope to impliment config setting via the cli 
tool in order to provide an api for automation and management tools

Signed-off-by: Justin Mitchell <jumitche@redhat.com>

[PATCH v3 1/8] nfs-utils: Fix minor memory leaks

[Justin Mitchell]

Fix some minor memory leaks, typos, and trailing whitespace

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/nfs/conffile.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 29f132d..28e29b7 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -57,9 +57,9 @@
 
 static void conf_load_defaults(void);
 static char * conf_readfile(const char *path);
-static int conf_set(int , const char *, const char *, const char *, 
+static int conf_set(int , const char *, const char *, const char *,
 	const char *, int , int );
-static void conf_parse(int trans, char *buf, 
+static void conf_parse(int trans, char *buf,
 	char **section, char **subsection);
 
 struct conf_trans {
@@ -177,7 +177,7 @@ conf_remove_section_now(const char *section)
  * into SECTION of our configuration database.
  */
 static int
-conf_set_now(const char *section, const char *arg, const char *tag, 
+conf_set_now(const char *section, const char *arg, const char *tag,
 	const char *value, int override, int is_default)
 {
 	struct conf_binding *node = 0;
@@ -186,7 +186,7 @@ conf_set_now(const char *section, const char *arg, const char *tag,
 		conf_remove_now(section, tag);
 	else if (conf_get_section(section, arg, tag)) {
 		if (!is_default) {
-			xlog(LOG_INFO, "conf_set: duplicate tag [%s]:%s, ignoring...\n", 
+			xlog(LOG_INFO, "conf_set: duplicate tag [%s]:%s, ignoring...",
 				section, tag);
 		}
 		return 1;
@@ -220,7 +220,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		return;
 
 	/* Strip off any leading blanks */
-	while (isblank(*line)) 
+	while (isblank(*line))
 		line++;
 
 	/* Lines starting with '#' or ';' are comments.  */
@@ -241,7 +241,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		}
 
 		/* Strip off any blanks after '[' */
-		while (isblank(*line)) 
+		while (isblank(*line))
 			line++;
 
 		/* find the closing ] */
@@ -256,7 +256,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		*(ptr--) = '\0';
 
 		/* Strip off any blanks before ']' */
-		while (ptr >= line && isblank(*ptr)) 
+		while (ptr >= line && isblank(*ptr))
 			*(ptr--)='\0';
 
 		/* look for an arg to split from the section name */
@@ -289,7 +289,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 		}
 		*ptr = '\0';
 		*subsection = strdup(val);
-		if (!*subsection) 
+		if (!*subsection)
 			xlog_warn("conf_parse_line: %d: malloc arg failed", lineno);
 		return;
 	}
@@ -535,8 +535,8 @@ conf_init_file(const char *conf_file)
 	conf_load_file(conf_file);
 }
 
-/* 
- * Empty the config and free up any used memory 
+/*
+ * Empty the config and free up any used memory
  */
 void
 conf_cleanup(void)
@@ -618,7 +618,7 @@ conf_match_num(const char *section, const char *tag, int x)
 		xlog(LOG_INFO, "conf_match_num: %s:%s %d==%d?", section, tag, val, x);
 		return x == val;
 	case 3:
-		xlog(LOG_INFO, "conf_match_num: %s:%s %d<=%d<=%d?", section, 
+		xlog(LOG_INFO, "conf_match_num: %s:%s %d<=%d<=%d?", section,
 			tag, min, x, max);
 		return min <= x && max >= x;
 	default:
@@ -642,7 +642,7 @@ char *
 conf_get_str_with_def(const char *section, const char *tag, char *def)
 {
 	char * result = conf_get_section(section, NULL, tag);
-	if (!result) 
+	if (!result)
 		return def;
 	return result;
 }
@@ -659,7 +659,7 @@ retry:
 	for (; cb; cb = LIST_NEXT (cb, link)) {
 		if (strcasecmp(section, cb->section) != 0)
 			continue;
-		if (arg && strcasecmp(arg, cb->arg) != 0)
+		if (arg && (cb->arg == NULL || strcasecmp(arg, cb->arg) != 0))
 			continue;
 		if (strcasecmp(tag, cb->tag) != 0)
 			continue;
@@ -917,6 +917,8 @@ conf_set(int transaction, const char *section, const char *arg,
 fail:
 	if (node->tag)
 		free(node->tag);
+	if (node->arg)
+		free(node->arg);
 	if (node->section)
 		free(node->section);
 	if (node)
@@ -987,8 +989,8 @@ conf_end(int transaction, int commit)
 			if (commit) {
 				switch (node->op) {
 				case CONF_SET:
-					conf_set_now(node->section, node->arg, 
-						node->tag, node->value, node->override, 
+					conf_set_now(node->section, node->arg,
+						node->tag, node->value, node->override,
 						node->is_default);
 					break;
 				case CONF_REMOVE:
@@ -1004,6 +1006,8 @@ conf_end(int transaction, int commit)
 			TAILQ_REMOVE (&conf_trans_queue, node, link);
 			if (node->section)
 				free(node->section);
+			if (node->arg)
+				free(node->arg);
 			if (node->tag)
 				free(node->tag);
 			if (node->value)
@@ -1066,7 +1070,7 @@ conf_report (void)
 					diff_arg = 1;
 				}
 				/* Dump this entry.  */
-				if (!current_section || strcmp(cb->section, current_section) 
+				if (!current_section || strcmp(cb->section, current_section)
 							|| diff_arg) {
 					if (current_section || diff_arg) {
 						len = strlen (current_section) + 3;
@@ -1077,19 +1081,19 @@ conf_report (void)
 							goto mem_fail;
 
 						if (current_arg)
-							snprintf(dnode->s, len, "[%s \"%s\"]", 
+							snprintf(dnode->s, len, "[%s \"%s\"]",
 								current_section, current_arg);
 						else
 							snprintf(dnode->s, len, "[%s]", current_section);
 
-						dnode->next = 
+						dnode->next =
 							(struct dumper *)calloc(1, sizeof (struct dumper));
 						dnode = dnode->next;
 						if (!dnode)
 							goto mem_fail;
 
 						dnode->s = "";
-						dnode->next = 
+						dnode->next =
 							(struct dumper *)calloc(1, sizeof (struct dumper));
 						dnode = dnode->next;
 						if (!dnode)
-- 
1.8.3.1

[PATCH v3 2/8] nfs-utils: Make config includes relative to current config

[Justin Mitchell]

When including additional config files with the include directive
make relative paths be relative to the current config file instead
of to the process's cwd.

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/nfs/conffile.c | 76 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 63 insertions(+), 13 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 28e29b7..95dda99 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -49,6 +49,7 @@
 #include <errno.h>
 #include <err.h>
 #include <syslog.h>
+#include <libgen.h>
 
 #include "conffile.h"
 #include "xlog.h"
@@ -60,7 +61,7 @@ static char * conf_readfile(const char *path);
 static int conf_set(int , const char *, const char *, const char *,
 	const char *, int , int );
 static void conf_parse(int trans, char *buf,
-	char **section, char **subsection);
+	char **section, char **subsection, const char *filename);
 
 struct conf_trans {
 	TAILQ_ENTRY (conf_trans) link;
@@ -206,12 +207,49 @@ conf_set_now(const char *section, const char *arg, const char *tag,
 	return 0;
 }
 
+/* Attempt to construct a relative path to the new file */
+static char *
+relative_path(const char *oldfile, const char *newfile)
+{
+	char *tmpcopy = NULL;
+	char *dir = NULL;
+	char *relpath = NULL;
+	size_t pathlen;
+
+	if (!newfile)
+		return strdup(oldfile);
+
+	if (newfile[0] == '/')
+		return strdup(newfile);
+
+	tmpcopy = strdup(oldfile);
+	if (!tmpcopy)
+		goto mem_err;
+
+	dir = dirname(tmpcopy);
+
+	pathlen = strlen(dir) + strlen(newfile) + 2;
+	relpath = calloc(1, pathlen);
+	if (!relpath)
+		goto mem_err;
+
+	snprintf(relpath, pathlen, "%s/%s", dir, newfile);
+
+	free(tmpcopy);
+	return relpath;
+mem_err:
+	if (tmpcopy)
+		free(tmpcopy);
+	return NULL;
+}
+
+
 /*
  * Parse the line LINE of SZ bytes.  Skip Comments, recognize section
  * headers and feed tag-value pairs into our configuration database.
  */
 static void
-conf_parse_line(int trans, char *line, int lineno, char **section, char **subsection)
+conf_parse_line(int trans, char *line, const char *filename, int lineno, char **section, char **subsection)
 {
 	char *val, *ptr;
 
@@ -366,27 +404,34 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 
 	if (strcasecmp(line, "include")==0) {
 		/* load and parse subordinate config files */
-		char * subconf = conf_readfile(val);
+		char *inc_section = NULL;
+		char *inc_subsection = NULL;
+		char *relpath = relative_path(filename, val);
+		char *subconf = conf_readfile(relpath);
 		if (subconf == NULL) {
-			xlog_warn("config file error: line %d: "
-			"error loading included config", lineno);
+			xlog_warn("%s:%d: error loading included config",
+				  filename, lineno);
+			if (relpath)
+				free(relpath);
 			return;
 		}
 
 		/* copy the section data so the included file can inherit it
 		 * without accidentally changing it for us */
-		char * inc_section = NULL;
-		char * inc_subsection = NULL;
 		if (*section != NULL) {
 			inc_section = strdup(*section);
 			if (*subsection != NULL)
 				inc_subsection = strdup(*subsection);
 		}
 
-		conf_parse(trans, subconf, &inc_section, &inc_subsection);
+		conf_parse(trans, subconf, &inc_section, &inc_subsection, relpath);
 
-		if (inc_section) free(inc_section);
-		if (inc_subsection) free(inc_subsection);
+		if (inc_section)
+			free(inc_section);
+		if (inc_subsection)
+			free(inc_subsection);
+		if (relpath)
+			free(relpath);
 		free(subconf);
 	} else {
 		/* XXX Perhaps should we not ignore errors?  */
@@ -396,7 +441,7 @@ conf_parse_line(int trans, char *line, int lineno, char **section, char **subsec
 
 /* Parse the mapped configuration file.  */
 static void
-conf_parse(int trans, char *buf, char **section, char **subsection)
+conf_parse(int trans, char *buf, char **section, char **subsection, const char *filename)
 {
 	char *cp = buf;
 	char *bufend = NULL;
@@ -413,7 +458,7 @@ conf_parse(int trans, char *buf, char **section, char **subsection)
 			else {
 				*cp = '\0';
 				lineno++;
-				conf_parse_line(trans, line, lineno, section, subsection);
+				conf_parse_line(trans, line, filename, lineno, section, subsection);
 				line = cp + 1;
 			}
 		}
@@ -434,6 +479,11 @@ static char *
 conf_readfile(const char *path)
 {
 	struct stat sb;
+	if (!path) {
+		xlog_err("conf_readfile: no path given");
+		return NULL;
+	}
+
 	if ((stat (path, &sb) == 0) || (errno != ENOENT)) {
 		char *new_conf_addr = NULL;
 		size_t sz = sb.st_size;
@@ -508,7 +558,7 @@ conf_load_file(const char *conf_file)
 	/* Parse config contents into the transaction queue */
 	char *section = NULL;
 	char *subsection = NULL;
-	conf_parse(trans, conf_data, &section, &subsection);
+	conf_parse(trans, conf_data, &section, &subsection, conf_file);
 	if (section) free(section);
 	if (subsection) free(subsection);
 	free(conf_data);
-- 
1.8.3.1

[PATCH v3 3/8] nfs-utils: Use config file name in error messages

[Justin Mitchell]

Improve readability of error configuration error messages
by including the filename currently being read.

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/nfs/conffile.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 95dda99..5fb58ea 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -285,8 +285,9 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		/* find the closing ] */
 		ptr = strchr(line, ']');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
- 				"non-matched ']', ignoring until next section", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "non-matched ']', ignoring until next section",
+				  filename, lineno);
 			return;
 		}
 
@@ -311,7 +312,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		/* copy the section name */
 		*section = strdup(line);
 		if (!*section) {
-			xlog_warn("conf_parse_line: %d: malloc failed", lineno);
+			xlog_warn("config error at %s:%d:"
+				  "malloc failed", filename, lineno);
 			return;
 		}
 
@@ -321,14 +323,16 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		/* check for the closing " */
 		ptr = strchr(val, '"');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
- 				"non-matched '\"', ignoring until next section", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "non-matched '\"', ignoring until next section",
+				filename, lineno);
 			return;
 		}
 		*ptr = '\0';
 		*subsection = strdup(val);
 		if (!*subsection)
-			xlog_warn("conf_parse_line: %d: malloc arg failed", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "malloc failed", filename, lineno);
 		return;
 	}
 
@@ -339,15 +343,17 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 	if (ptr == NULL) {
 		/* Other non-empty lines are weird.  */
 		if (line[strspn(line, " \t")])
-			xlog_warn("config file error: line %d: "
-				"line not empty and not an assignment", lineno);
+			xlog_warn("config error at %s:%d: "
+				"line not empty and not an assignment",
+				filename, lineno);
 		return;
 	}
 
 	/* If no section, we are ignoring the line.  */
 	if (!*section) {
-		xlog_warn("config file error: line %d: "
-			"ignoring line due to no section", lineno);
+		xlog_warn("config error at %s:%d: "
+			  "ignoring line not in a section",
+			  filename, lineno);
 		return;
 	}
 
@@ -364,8 +370,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		val++;
 		ptr = strchr(val, '"');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
-				"unmatched quotes", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "unmatched quotes",filename, lineno);
 			return;
 		}
 		*ptr = '\0';
@@ -374,8 +380,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		val++;
 		ptr = strchr(val, '\'');
 		if (ptr == NULL) {
-			xlog_warn("config file error: line %d: "
-				"unmatched quotes", lineno);
+			xlog_warn("config error at %s:%d: "
+				  "unmatched quotes", filename, lineno);
 			return;
 		}
 		*ptr = '\0';
@@ -392,13 +398,13 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 	}
 
 	if (*line == '\0') {
-		xlog_warn("config file error: line %d: "
-			"missing tag in assignment", lineno);
+		xlog_warn("config error at %s:%d: "
+			  "missing tag in assignment", filename, lineno);
 		return;
 	}
 	if (*val == '\0') {
-		xlog_warn("config file error: line %d: "
-			"missing value in assignment", lineno);
+		xlog_warn("config error at %s:%d: "
+			  "missing value in assignment", filename, lineno);
 		return;
 	}
 
@@ -409,7 +415,8 @@ conf_parse_line(int trans, char *line, const char *filename, int lineno, char **
 		char *relpath = relative_path(filename, val);
 		char *subconf = conf_readfile(relpath);
 		if (subconf == NULL) {
-			xlog_warn("%s:%d: error loading included config",
+			xlog_warn("config error at %s:%d: "
+				  "error loading included config",
 				  filename, lineno);
 			if (relpath)
 				free(relpath);
-- 
1.8.3.1

[PATCH v3 4/8] nfs-utils: Indicate if config file was missing

[Justin Mitchell]

Return an indication that the config file could not be loaded
for processes that want to differentiate this from empty config

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/include/conffile.h |  2 +-
 support/nfs/conffile.c     | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/support/include/conffile.h b/support/include/conffile.h
index ad20067..6baaf9a 100644
--- a/support/include/conffile.h
+++ b/support/include/conffile.h
@@ -60,7 +60,7 @@ extern _Bool    conf_get_bool(const char *, const char *, _Bool);
 extern char    *conf_get_str(const char *, const char *);
 extern char    *conf_get_str_with_def(const char *, const char *, char *);
 extern char    *conf_get_section(const char *, const char *, const char *);
-extern void     conf_init_file(const char *);
+extern int      conf_init_file(const char *);
 extern void     conf_cleanup(void);
 extern int      conf_match_num(const char *, const char *, int);
 extern int      conf_remove(int, const char *, const char *);
diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 5fb58ea..e65caaf 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -547,7 +547,7 @@ static void conf_free_bindings(void)
 }
 
 /* Open the config file and map it into our address space, then parse it.  */
-static void
+static int
 conf_load_file(const char *conf_file)
 {
 	int trans;
@@ -557,7 +557,7 @@ conf_load_file(const char *conf_file)
 	conf_data = conf_readfile(conf_file);
 
 	if (conf_data == NULL)
-		return;
+		return 1;
 
 	/* Load default configuration values.  */
 	conf_load_defaults();
@@ -575,10 +575,10 @@ conf_load_file(const char *conf_file)
 
 	/* Apply the new configuration values */
 	conf_end(trans, 1);
-	return;
+	return 0;
 }
 
-void
+int
 conf_init_file(const char *conf_file)
 {
 	unsigned int i;
@@ -589,7 +589,7 @@ conf_init_file(const char *conf_file)
 	TAILQ_INIT (&conf_trans_queue);
 
 	if (conf_file == NULL) conf_file=NFS_CONFFILE;
-	conf_load_file(conf_file);
+	return conf_load_file(conf_file);
 }
 
 /*
-- 
1.8.3.1

[PATCH v3 5/8] nfs-utils: tidy up output of conf_report

[Justin Mitchell]

Previously unused conf_report() function is modified to output
an alphabetically sorted version of the current configuration
to the nominated FILE handle.

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 support/include/conffile.h |   3 +-
 support/nfs/conffile.c     | 293 +++++++++++++++++++++++++++++++--------------
 2 files changed, 207 insertions(+), 89 deletions(-)

diff --git a/support/include/conffile.h b/support/include/conffile.h
index 6baaf9a..bc2d61f 100644
--- a/support/include/conffile.h
+++ b/support/include/conffile.h
@@ -37,6 +37,7 @@
 #include <ctype.h>
 #include <stdint.h>
 #include <stdbool.h>
+#include <stdio.h>
 
 struct conf_list_node {
 	TAILQ_ENTRY(conf_list_node) link;
@@ -65,7 +66,7 @@ extern void     conf_cleanup(void);
 extern int      conf_match_num(const char *, const char *, int);
 extern int      conf_remove(int, const char *, const char *);
 extern int      conf_remove_section(int, const char *);
-extern void     conf_report(void);
+extern void     conf_report(FILE *);
 
 /*
  * Convert letter from upper case to lower case
diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index e65caaf..1e8034a 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -1080,117 +1080,234 @@ conf_end(int transaction, int commit)
  * Configuration is "stored in reverse order", so reverse it again.
  */
 struct dumper {
-	char *s, *v;
+	char *section;
+	char *arg;
+	char *tag;
+	char *value;
 	struct dumper *next;
 };
 
+/*
+ * Test if two nodes belong to the same (sub)sections
+ */
+static int
+dumper_section_compare(const struct dumper *nodea, const struct dumper *nodeb)
+{
+	int ret;
+
+	/* missing node, shouldnt happen */
+	if (!nodea || !nodeb)
+		return -1;
+
+	/* no section names at all, they are equal */
+	if (!nodea->section && !nodeb->section)
+		return 0;
+
+	/* if only one has a section name, the blank one goes first */
+	if (!nodea->section && nodeb->section)
+		return -1;
+
+	if (nodea->section && !nodeb->section)
+		return 1;
+
+	/* both have section names, but do they match */
+	ret = strcmp(nodea->section, nodeb->section);
+
+	/* section names differ, that was easy */
+	if (ret != 0)
+		return ret;
+
+	/* sections matched, but how about sub-sections,
+	 * again, if only one has a value the blank goes first
+	 */
+	if (!nodea->arg && nodeb->arg)
+		return -1;
+
+	if (nodea->arg && !nodeb->arg)
+		return  1;
+
+	/* both have sub-section args and they differ */
+	if (nodea->arg && nodeb->arg
+	&& (ret=strcmp(nodea->arg, nodeb->arg))!=0)
+		return ret;
+
+	return 0;
+}
+
+/* If a string starts or ends with a space it should be quoted */
+static bool
+should_escape(const char *text)
+{
+	int len;
+
+	/* no string, no escaping needed */
+	if (!text)
+		return false;
+
+	/* first character is a space */
+	if (isspace(text[0]))
+		return true;
+
+	/* last character is a space */
+	len = strlen(text);
+	if (isspace(text[len-1]))
+		return true;
+
+	return false;
+}
+
 static void
-conf_report_dump(struct dumper *node)
+conf_report_dump_text(struct dumper *head, FILE *ff)
 {
-	/* Recursive, cleanup when we're done.  */
-	if (node->next)
-		conf_report_dump(node->next);
-
-	if (node->v)
-		xlog(LOG_INFO, "%s=\t%s", node->s, node->v);
-	else if (node->s) {
-		xlog(LOG_INFO, "%s", node->s);
-		if (strlen(node->s) > 0)
-			free(node->s);
+	const struct dumper *node = head;
+	const struct dumper *last = NULL;
+
+	for (node=head; node!=NULL; node=node->next) {
+		/* starting a new section, print the section header */
+		if (dumper_section_compare(last, node)!=0) {
+			if (node != head)
+				fprintf(ff, "\n");
+			if (node->arg)
+				fprintf(ff, "[%s \"%s\"]\n", node->section, node->arg);
+			else
+				fprintf(ff, "[%s]\n", node->section);
+		}
+
+		/* now print the tag and its value */
+		fprintf(ff, " %s", node->tag);
+		if (node->value) {
+			if (should_escape(node->value))
+				fprintf(ff, " = \"%s\"", node->value);
+			else
+				fprintf(ff, " = %s", node->value);
+		}
+		fprintf(ff, "\n");
+
+		last = node;
 	}
+}
 
-	free (node);
+/* sort by tag compare function */
+static int
+dumper_compare(const void *a, const void *b)
+{
+	const struct dumper *nodea = *(struct dumper **)a;
+	const struct dumper *nodeb = *(struct dumper **)b;
+	int ret;
+
+	/* missing node, shouldnt happen */
+	if (!nodea || !nodeb)
+		return -1;
+
+	/* are the two nodes in different (sub)sections */
+	ret = dumper_section_compare(nodea, nodeb);
+	if (ret != 0)
+		return ret;
+
+	/* sub-sections match (both blank, or both same)
+	 * so we compare the tag names
+	 */
+
+	/* blank tags shouldnt happen, but paranoia */
+	if (!nodea->tag && !nodeb->tag)
+		return  0;
+
+	/* still shouldnt happen, but use the blank-goes-first logic */
+	if (!nodea->tag &&  nodeb->tag)
+		return -1;
+	if ( nodea->tag && !nodeb->tag)
+		return  1;
+
+	/* last test, compare the tags directly */
+	ret = strcmp(nodea->tag, nodeb->tag);
+	return ret;
 }
 
-void
-conf_report (void)
+/* sort all of the report nodes */
+static struct dumper *
+conf_report_sort(struct dumper *start)
 {
-	struct conf_binding *cb, *last = 0;
-	unsigned int i, len, diff_arg = 0;
-	char *current_section = (char *)0;
-	char *current_arg = (char *)0;
-	struct dumper *dumper, *dnode;
+	struct dumper **list;
+	struct dumper *node;
+	unsigned int count = 0;
+	unsigned int i=0;
+
+	/* how long is this list */
+	for (node=start; node!=NULL; node=node->next)
+		count++;
+
+	/* no need to sort a list with less than 2 items */
+	if (count < 2)
+		return start;
+
+	/* build an array of all the nodes */
+	list = calloc(count, sizeof(struct dumper *));
+	if (!list)
+		goto mem_err;
+
+	for (node=start,i=0; node!=NULL; node=node->next) {
+		list[i++] = node;
+	}
+
+	/* sort the array alphabetically by section and tag */
+	qsort(list, count, sizeof(struct dumper *), dumper_compare);
+
+	/* rebuild the linked list in sorted order */
+	for (i=0; i<count-1; i++) {
+		list[i]->next = list[i+1];
+	}
+	list[count-1]->next = NULL;
+
+	/* remember the new head of list and discard the sorting array */
+	node = list[0];
+	free(list);
 
-	dumper = dnode = (struct dumper *)calloc(1, sizeof *dumper);
-	if (!dumper)
-		goto mem_fail;
+	/* return the new head of list */
+	return node;
+
+mem_err:
+	free(list);
+	return NULL;
+}
+
+/* Output a copy of the current configuration to file */
+void
+conf_report(FILE *outfile)
+{
+	struct conf_binding *cb = NULL;
+	unsigned int i;
+	struct dumper *dumper = NULL, *dnode = NULL;
 
 	xlog(LOG_INFO, "conf_report: dumping running configuration");
 
-	for (i = 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++)
+	/* build a linked list of all the config nodes */
+	for (i = 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++) {
 		for (cb = LIST_FIRST(&conf_bindings[i]); cb; cb = LIST_NEXT(cb, link)) {
-			if (!cb->is_default) {
-				/* Make sure the Section arugment is the same */
-				if (current_arg && current_section && cb->arg) {
-					if (strcmp(cb->section, current_section) == 0 &&
-						strcmp(cb->arg, current_arg) != 0)
-					diff_arg = 1;
-				}
-				/* Dump this entry.  */
-				if (!current_section || strcmp(cb->section, current_section)
-							|| diff_arg) {
-					if (current_section || diff_arg) {
-						len = strlen (current_section) + 3;
-						if (current_arg)
-							len += strlen(current_arg) + 3;
-						dnode->s = malloc(len);
-						if (!dnode->s)
-							goto mem_fail;
-
-						if (current_arg)
-							snprintf(dnode->s, len, "[%s \"%s\"]",
-								current_section, current_arg);
-						else
-							snprintf(dnode->s, len, "[%s]", current_section);
-
-						dnode->next =
-							(struct dumper *)calloc(1, sizeof (struct dumper));
-						dnode = dnode->next;
-						if (!dnode)
-							goto mem_fail;
-
-						dnode->s = "";
-						dnode->next =
-							(struct dumper *)calloc(1, sizeof (struct dumper));
-						dnode = dnode->next;
-						if (!dnode)
-						goto mem_fail;
-					}
-					current_section = cb->section;
-					current_arg = cb->arg;
-					diff_arg = 0;
-				}
-				dnode->s = cb->tag;
-				dnode->v = cb->value;
-				dnode->next = (struct dumper *)calloc (1, sizeof (struct dumper));
-				dnode = dnode->next;
-				if (!dnode)
-					goto mem_fail;
-				last = cb;
+			struct dumper *newnode = calloc(1, sizeof (struct dumper));
+			if (!newnode)
+				goto mem_fail;
+
+			newnode->next = dumper;
+			dumper = newnode;
+
+			newnode->section = cb->section;
+			newnode->arg = cb->arg;
+			newnode->tag = cb->tag;
+			newnode->value = cb->value;
 		}
 	}
 
-	if (last) {
-		len = strlen(last->section) + 3;
-		if (last->arg)
-			len += strlen(last->arg) + 3;
-		dnode->s = malloc(len);
-		if (!dnode->s)
-			goto mem_fail;
-		if (last->arg)
-			snprintf(dnode->s, len, "[%s \"%s\"]", last->section, last->arg);
-		else
-			snprintf(dnode->s, len, "[%s]", last->section);
-	}
-	conf_report_dump(dumper);
-	return;
+	/* sort the list then print it */
+	dumper = conf_report_sort(dumper);
+	conf_report_dump_text(dumper, outfile);
+	goto cleanup;
 
 mem_fail:
 	xlog_warn("conf_report: malloc/calloc failed");
+cleanup:
+	/* traverse the linked list freeing all the nodes */
 	while ((dnode = dumper) != 0) {
 		dumper = dumper->next;
-		if (dnode->s)
-			free(dnode->s);
 		free(dnode);
 	}
 	return;
-- 
1.8.3.1

[PATCH v3 6/8] nfs-utils: Add nfsconftool cli

[Justin Mitchell]

This tool uses the conffile facilities to allow commandline
querying of configuration settings and to dump the current
config for diagnosis and testing

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 Makefile.am                |   2 +-
 configure.ac               |   1 +
 tools/Makefile.am          |   2 +
 tools/nfsconf/Makefile.am  |   8 ++
 tools/nfsconf/nfsconfcli.c | 180 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 192 insertions(+), 1 deletion(-)
 create mode 100644 tools/nfsconf/Makefile.am
 create mode 100644 tools/nfsconf/nfsconfcli.c

diff --git a/Makefile.am b/Makefile.am
index e1f39aa..0022084 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,7 +2,7 @@
 
 AUTOMAKE_OPTIONS = foreign
 
-SUBDIRS = tools support utils linux-nfs tests systemd
+SUBDIRS = support tools utils linux-nfs tests systemd
 
 MAINTAINERCLEANFILES = Makefile.in
 
diff --git a/configure.ac b/configure.ac
index 5a11636..b925666 100644
--- a/configure.ac
+++ b/configure.ac
@@ -616,6 +616,7 @@ AC_CONFIG_FILES([
 	tools/rpcgen/Makefile
 	tools/mountstats/Makefile
 	tools/nfs-iostat/Makefile
+	tools/nfsconf/Makefile
 	utils/Makefile
 	utils/blkmapd/Makefile
 	utils/nfsdcltrack/Makefile
diff --git a/tools/Makefile.am b/tools/Makefile.am
index f2ce282..4266da4 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -6,6 +6,8 @@ if CONFIG_RPCGEN
 OPTDIRS += rpcgen
 endif
 
+OPTDIRS += nfsconf
+
 SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat $(OPTDIRS)
 
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/tools/nfsconf/Makefile.am b/tools/nfsconf/Makefile.am
new file mode 100644
index 0000000..0042db9
--- /dev/null
+++ b/tools/nfsconf/Makefile.am
@@ -0,0 +1,8 @@
+## Process this file with automake to produce Makefile.in
+
+bin_PROGRAMS = nfsconftool
+
+nfsconftool_SOURCES = nfsconfcli.c
+nfsconftool_LDADD = ../../support/nfs/libnfsconf.la
+
+MAINTAINERCLEANFILES = Makefile.in
diff --git a/tools/nfsconf/nfsconfcli.c b/tools/nfsconf/nfsconfcli.c
new file mode 100644
index 0000000..034ec51
--- /dev/null
+++ b/tools/nfsconf/nfsconfcli.c
@@ -0,0 +1,180 @@
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <getopt.h>
+#include <string.h>
+#include <errno.h>
+#include "config.h"
+#include "conffile.h"
+#include "xlog.h"
+
+typedef enum {
+	MODE_NONE,
+	MODE_GET,
+	MODE_ISSET,
+	MODE_DUMP
+} confmode_t;
+
+static void usage(const char *name)
+{
+	fprintf(stderr, "Usage: %s [-v] [--file filename.conf] ...\n", name);
+	fprintf(stderr, "Options:\n");
+	fprintf(stderr, " -v			Increase Verbosity\n");
+	fprintf(stderr, " --file filename.conf	Load this config file\n");
+	fprintf(stderr, "     (Default config file: " NFS_CONFFILE "\n");
+	fprintf(stderr, "Modes:\n");
+	fprintf(stderr, "  --dump [outputfile]\n");
+	fprintf(stderr, "      Outputs the configuration to the named file\n");
+	fprintf(stderr, "  --get [--arg subsection] {section} {tag}\n");
+	fprintf(stderr, "      Output one specific config value\n");
+	fprintf(stderr, "  --isset [--arg subsection] {section} {tag}\n");
+	fprintf(stderr, "      Return code indicates if config value is present\n");
+}
+
+int main(int argc, char **argv)
+{
+	const char * val;
+	char * confpath = NFS_CONFFILE;
+	char * arg = NULL;
+	int verbose=0;
+	int ret = 0;
+	char * dumpfile = NULL;
+
+	confmode_t mode = MODE_NONE;
+
+	while (1) {
+		int c;
+		int index = 0;
+		struct option long_options[] = {
+			{"get",		no_argument, 0, 'g' },
+			{"arg",	  required_argument, 0, 'a' },
+			{"isset", 	no_argument, 0, 'i' },
+			{"dump",  optional_argument, 0, 'd' },
+			{"file",  required_argument, 0, 'f' },
+			{"verbose",	no_argument, 0, 'v' },
+			{NULL,			  0, 0, 0 }
+		};
+
+		c = getopt_long(argc, argv, "ga:id::f:v", long_options, &index);
+		if (c == -1) break;
+
+		switch (c) {
+			case 0:
+				break;
+			case 'f':
+				/* user specified souce path for config */
+				confpath = optarg;
+				break;
+			case 'a':
+				/* user supplied a sub-section name */
+				arg = optarg;
+				break;
+			case 'v':
+				verbose++;
+				break;
+			case 'g':
+				mode = MODE_GET;
+				break;
+			case 'i':
+				mode = MODE_ISSET;
+				break;
+			case 'd':
+				/* check if user supplied a filename for dump */
+				if (optarg == NULL && argv[optind] != NULL
+				    && argv[optind][0] != '-')
+					optarg = argv[optind++];
+				mode = MODE_DUMP;
+				dumpfile = optarg;
+				break;
+			default:
+				usage(argv[0]);
+				return 1;
+		}
+	}
+
+	/* configure the logging that conffile.c uses */
+	if (verbose)
+		xlog_config(D_ALL, 1);
+	xlog_stderr(1);
+	xlog_syslog(0);
+	xlog_open("nfsconftool");
+
+	if (mode == MODE_NONE) {
+		fprintf(stderr, "Error: No MODE selected.\n");
+		usage(argv[0]);
+		return 1;
+	}
+
+	if (conf_init_file(confpath)) {
+		/* config file was missing or had an error, warn about it */
+		if (verbose || mode != MODE_ISSET)
+			fprintf(stderr, "Error loading config file %s\n",
+				confpath);
+		/* this isnt fatal for --isset */
+		if (mode != MODE_ISSET)
+			return 1;
+	}
+
+	/* --dump mode, output the current configuration */
+	if (mode == MODE_DUMP) {
+		/* default to writing it to stdout */
+		FILE *out = stdout;
+
+		/* user specified a file to write to instead */
+		if (dumpfile) {
+			out = fopen(dumpfile, "w");
+			if (out == NULL) {
+				fprintf(stderr, "Error opening dumpfile %s: %s\n",
+					dumpfile, strerror(errno));
+				ret = 2;
+				goto cleanup;
+			}
+			if (verbose)
+				printf("Dumping config to %s\n", dumpfile);
+		}
+
+		/* output the configuration */
+		conf_report(out);
+
+		/* close that user specified file */
+		if (dumpfile)
+			fclose(out);
+	} else
+	/* --iset and --get share a lot of code */
+	if (mode == MODE_GET || mode == MODE_ISSET) {
+		char * section = NULL;
+		char * tag = NULL;
+
+		/* test they supplied section and tag names */
+		if (optind+1 >= argc) {
+			fprintf(stderr, "Error: insufficient arguments for mode\n");
+			usage(argv[0]);
+			ret = 2;
+			goto cleanup;
+		}
+
+		/* now we have a section and tag name */
+		section = argv[optind++];
+		tag = argv[optind++];
+
+		/* retrieve the specified tags value */
+		val = conf_get_section(section, arg, tag);
+		if (val != NULL) {
+			/* ret=0, success, mode --get wants to output the value as well */
+			if (mode == MODE_GET)
+				printf("%s\n", val);
+		} else {
+			/* ret=1, no value found, tell the user if they asked */
+			if (mode == MODE_GET && verbose)
+				fprintf(stderr, "Tag '%s' not found\n", tag);
+			ret = 1;
+		}
+	} else {
+		fprintf(stderr, "Mode not yet implimented.\n");
+		ret = 2;
+	}
+
+cleanup:
+	conf_cleanup();
+	return ret;
+}
-- 
1.8.3.1

Re: [PATCH v3 6/8] nfs-utils: Add nfsconftool cli

[Steve Dickson]

Hello,

On 05/15/2018 11:54 AM, Justin Mitchell wrote:
> This tool uses the conffile facilities to allow commandline
> querying of configuration settings and to dump the current
> config for diagnosis and testing
Justin, would mind if I change he name of this tool from
nfsconftool to just nfsconf? 

steved.

> 
> Signed-off-by: Justin Mitchell <jumitche@redhat.com>
> ---
>  Makefile.am                |   2 +-
>  configure.ac               |   1 +
>  tools/Makefile.am          |   2 +
>  tools/nfsconf/Makefile.am  |   8 ++
>  tools/nfsconf/nfsconfcli.c | 180 +++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 192 insertions(+), 1 deletion(-)
>  create mode 100644 tools/nfsconf/Makefile.am
>  create mode 100644 tools/nfsconf/nfsconfcli.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index e1f39aa..0022084 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -2,7 +2,7 @@
>  
>  AUTOMAKE_OPTIONS = foreign
>  
> -SUBDIRS = tools support utils linux-nfs tests systemd
> +SUBDIRS = support tools utils linux-nfs tests systemd
>  
>  MAINTAINERCLEANFILES = Makefile.in
>  
> diff --git a/configure.ac b/configure.ac
> index 5a11636..b925666 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -616,6 +616,7 @@ AC_CONFIG_FILES([
>  	tools/rpcgen/Makefile
>  	tools/mountstats/Makefile
>  	tools/nfs-iostat/Makefile
> +	tools/nfsconf/Makefile
>  	utils/Makefile
>  	utils/blkmapd/Makefile
>  	utils/nfsdcltrack/Makefile
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index f2ce282..4266da4 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -6,6 +6,8 @@ if CONFIG_RPCGEN
>  OPTDIRS += rpcgen
>  endif
>  
> +OPTDIRS += nfsconf
> +
>  SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat $(OPTDIRS)
>  
>  MAINTAINERCLEANFILES = Makefile.in
> diff --git a/tools/nfsconf/Makefile.am b/tools/nfsconf/Makefile.am
> new file mode 100644
> index 0000000..0042db9
> --- /dev/null
> +++ b/tools/nfsconf/Makefile.am
> @@ -0,0 +1,8 @@
> +## Process this file with automake to produce Makefile.in
> +
> +bin_PROGRAMS = nfsconftool
> +
> +nfsconftool_SOURCES = nfsconfcli.c
> +nfsconftool_LDADD = ../../support/nfs/libnfsconf.la
> +
> +MAINTAINERCLEANFILES = Makefile.in
> diff --git a/tools/nfsconf/nfsconfcli.c b/tools/nfsconf/nfsconfcli.c
> new file mode 100644
> index 0000000..034ec51
> --- /dev/null
> +++ b/tools/nfsconf/nfsconfcli.c
> @@ -0,0 +1,180 @@
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <getopt.h>
> +#include <string.h>
> +#include <errno.h>
> +#include "config.h"
> +#include "conffile.h"
> +#include "xlog.h"
> +
> +typedef enum {
> +	MODE_NONE,
> +	MODE_GET,
> +	MODE_ISSET,
> +	MODE_DUMP
> +} confmode_t;
> +
> +static void usage(const char *name)
> +{
> +	fprintf(stderr, "Usage: %s [-v] [--file filename.conf] ...\n", name);
> +	fprintf(stderr, "Options:\n");
> +	fprintf(stderr, " -v			Increase Verbosity\n");
> +	fprintf(stderr, " --file filename.conf	Load this config file\n");
> +	fprintf(stderr, "     (Default config file: " NFS_CONFFILE "\n");
> +	fprintf(stderr, "Modes:\n");
> +	fprintf(stderr, "  --dump [outputfile]\n");
> +	fprintf(stderr, "      Outputs the configuration to the named file\n");
> +	fprintf(stderr, "  --get [--arg subsection] {section} {tag}\n");
> +	fprintf(stderr, "      Output one specific config value\n");
> +	fprintf(stderr, "  --isset [--arg subsection] {section} {tag}\n");
> +	fprintf(stderr, "      Return code indicates if config value is present\n");
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	const char * val;
> +	char * confpath = NFS_CONFFILE;
> +	char * arg = NULL;
> +	int verbose=0;
> +	int ret = 0;
> +	char * dumpfile = NULL;
> +
> +	confmode_t mode = MODE_NONE;
> +
> +	while (1) {
> +		int c;
> +		int index = 0;
> +		struct option long_options[] = {
> +			{"get",		no_argument, 0, 'g' },
> +			{"arg",	  required_argument, 0, 'a' },
> +			{"isset", 	no_argument, 0, 'i' },
> +			{"dump",  optional_argument, 0, 'd' },
> +			{"file",  required_argument, 0, 'f' },
> +			{"verbose",	no_argument, 0, 'v' },
> +			{NULL,			  0, 0, 0 }
> +		};
> +
> +		c = getopt_long(argc, argv, "ga:id::f:v", long_options, &index);
> +		if (c == -1) break;
> +
> +		switch (c) {
> +			case 0:
> +				break;
> +			case 'f':
> +				/* user specified souce path for config */
> +				confpath = optarg;
> +				break;
> +			case 'a':
> +				/* user supplied a sub-section name */
> +				arg = optarg;
> +				break;
> +			case 'v':
> +				verbose++;
> +				break;
> +			case 'g':
> +				mode = MODE_GET;
> +				break;
> +			case 'i':
> +				mode = MODE_ISSET;
> +				break;
> +			case 'd':
> +				/* check if user supplied a filename for dump */
> +				if (optarg == NULL && argv[optind] != NULL
> +				    && argv[optind][0] != '-')
> +					optarg = argv[optind++];
> +				mode = MODE_DUMP;
> +				dumpfile = optarg;
> +				break;
> +			default:
> +				usage(argv[0]);
> +				return 1;
> +		}
> +	}
> +
> +	/* configure the logging that conffile.c uses */
> +	if (verbose)
> +		xlog_config(D_ALL, 1);
> +	xlog_stderr(1);
> +	xlog_syslog(0);
> +	xlog_open("nfsconftool");
> +
> +	if (mode == MODE_NONE) {
> +		fprintf(stderr, "Error: No MODE selected.\n");
> +		usage(argv[0]);
> +		return 1;
> +	}
> +
> +	if (conf_init_file(confpath)) {
> +		/* config file was missing or had an error, warn about it */
> +		if (verbose || mode != MODE_ISSET)
> +			fprintf(stderr, "Error loading config file %s\n",
> +				confpath);
> +		/* this isnt fatal for --isset */
> +		if (mode != MODE_ISSET)
> +			return 1;
> +	}
> +
> +	/* --dump mode, output the current configuration */
> +	if (mode == MODE_DUMP) {
> +		/* default to writing it to stdout */
> +		FILE *out = stdout;
> +
> +		/* user specified a file to write to instead */
> +		if (dumpfile) {
> +			out = fopen(dumpfile, "w");
> +			if (out == NULL) {
> +				fprintf(stderr, "Error opening dumpfile %s: %s\n",
> +					dumpfile, strerror(errno));
> +				ret = 2;
> +				goto cleanup;
> +			}
> +			if (verbose)
> +				printf("Dumping config to %s\n", dumpfile);
> +		}
> +
> +		/* output the configuration */
> +		conf_report(out);
> +
> +		/* close that user specified file */
> +		if (dumpfile)
> +			fclose(out);
> +	} else
> +	/* --iset and --get share a lot of code */
> +	if (mode == MODE_GET || mode == MODE_ISSET) {
> +		char * section = NULL;
> +		char * tag = NULL;
> +
> +		/* test they supplied section and tag names */
> +		if (optind+1 >= argc) {
> +			fprintf(stderr, "Error: insufficient arguments for mode\n");
> +			usage(argv[0]);
> +			ret = 2;
> +			goto cleanup;
> +		}
> +
> +		/* now we have a section and tag name */
> +		section = argv[optind++];
> +		tag = argv[optind++];
> +
> +		/* retrieve the specified tags value */
> +		val = conf_get_section(section, arg, tag);
> +		if (val != NULL) {
> +			/* ret=0, success, mode --get wants to output the value as well */
> +			if (mode == MODE_GET)
> +				printf("%s\n", val);
> +		} else {
> +			/* ret=1, no value found, tell the user if they asked */
> +			if (mode == MODE_GET && verbose)
> +				fprintf(stderr, "Tag '%s' not found\n", tag);
> +			ret = 1;
> +		}
> +	} else {
> +		fprintf(stderr, "Mode not yet implimented.\n");
> +		ret = 2;
> +	}
> +
> +cleanup:
> +	conf_cleanup();
> +	return ret;
> +}
> 

Re: [PATCH v3 6/8] nfs-utils: Add nfsconftool cli

[Justin Mitchell]

On Mon, 2018-05-21 at 11:31 -0400, Steve Dickson wrote:
> Hello,
> 
> On 05/15/2018 11:54 AM, Justin Mitchell wrote:
> > This tool uses the conffile facilities to allow commandline
> > querying of configuration settings and to dump the current
> > config for diagnosis and testing
> Justin, would mind if I change he name of this tool from
> nfsconftool to just nfsconf? 

I do not mind what the name is, as a tool for querying (and eventually
manipulating) nfs config files it seemed suitably self explanatory, if a
little long winded.

So sure, change it unless anyone else has a better suggestion ?


> 
> steved.
> 
> > 
> > Signed-off-by: Justin Mitchell <jumitche@redhat.com>
> > ---
> >  Makefile.am                |   2 +-
> >  configure.ac               |   1 +
> >  tools/Makefile.am          |   2 +
> >  tools/nfsconf/Makefile.am  |   8 ++
> >  tools/nfsconf/nfsconfcli.c | 180 +++++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 192 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/nfsconf/Makefile.am
> >  create mode 100644 tools/nfsconf/nfsconfcli.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index e1f39aa..0022084 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -2,7 +2,7 @@
> >  
> >  AUTOMAKE_OPTIONS = foreign
> >  
> > -SUBDIRS = tools support utils linux-nfs tests systemd
> > +SUBDIRS = support tools utils linux-nfs tests systemd
> >  
> >  MAINTAINERCLEANFILES = Makefile.in
> >  
> > diff --git a/configure.ac b/configure.ac
> > index 5a11636..b925666 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -616,6 +616,7 @@ AC_CONFIG_FILES([
> >  	tools/rpcgen/Makefile
> >  	tools/mountstats/Makefile
> >  	tools/nfs-iostat/Makefile
> > +	tools/nfsconf/Makefile
> >  	utils/Makefile
> >  	utils/blkmapd/Makefile
> >  	utils/nfsdcltrack/Makefile
> > diff --git a/tools/Makefile.am b/tools/Makefile.am
> > index f2ce282..4266da4 100644
> > --- a/tools/Makefile.am
> > +++ b/tools/Makefile.am
> > @@ -6,6 +6,8 @@ if CONFIG_RPCGEN
> >  OPTDIRS += rpcgen
> >  endif
> >  
> > +OPTDIRS += nfsconf
> > +
> >  SUBDIRS = locktest rpcdebug nlmtest mountstats nfs-iostat $(OPTDIRS)
> >  
> >  MAINTAINERCLEANFILES = Makefile.in
> > diff --git a/tools/nfsconf/Makefile.am b/tools/nfsconf/Makefile.am
> > new file mode 100644
> > index 0000000..0042db9
> > --- /dev/null
> > +++ b/tools/nfsconf/Makefile.am
> > @@ -0,0 +1,8 @@
> > +## Process this file with automake to produce Makefile.in
> > +
> > +bin_PROGRAMS = nfsconftool
> > +
> > +nfsconftool_SOURCES = nfsconfcli.c
> > +nfsconftool_LDADD = ../../support/nfs/libnfsconf.la
> > +
> > +MAINTAINERCLEANFILES = Makefile.in
> > diff --git a/tools/nfsconf/nfsconfcli.c b/tools/nfsconf/nfsconfcli.c
> > new file mode 100644
> > index 0000000..034ec51
> > --- /dev/null
> > +++ b/tools/nfsconf/nfsconfcli.c
> > @@ -0,0 +1,180 @@
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <stdio.h>
> > +#include <getopt.h>
> > +#include <string.h>
> > +#include <errno.h>
> > +#include "config.h"
> > +#include "conffile.h"
> > +#include "xlog.h"
> > +
> > +typedef enum {
> > +	MODE_NONE,
> > +	MODE_GET,
> > +	MODE_ISSET,
> > +	MODE_DUMP
> > +} confmode_t;
> > +
> > +static void usage(const char *name)
> > +{
> > +	fprintf(stderr, "Usage: %s [-v] [--file filename.conf] ...\n", name);
> > +	fprintf(stderr, "Options:\n");
> > +	fprintf(stderr, " -v			Increase Verbosity\n");
> > +	fprintf(stderr, " --file filename.conf	Load this config file\n");
> > +	fprintf(stderr, "     (Default config file: " NFS_CONFFILE "\n");
> > +	fprintf(stderr, "Modes:\n");
> > +	fprintf(stderr, "  --dump [outputfile]\n");
> > +	fprintf(stderr, "      Outputs the configuration to the named file\n");
> > +	fprintf(stderr, "  --get [--arg subsection] {section} {tag}\n");
> > +	fprintf(stderr, "      Output one specific config value\n");
> > +	fprintf(stderr, "  --isset [--arg subsection] {section} {tag}\n");
> > +	fprintf(stderr, "      Return code indicates if config value is present\n");
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	const char * val;
> > +	char * confpath = NFS_CONFFILE;
> > +	char * arg = NULL;
> > +	int verbose=0;
> > +	int ret = 0;
> > +	char * dumpfile = NULL;
> > +
> > +	confmode_t mode = MODE_NONE;
> > +
> > +	while (1) {
> > +		int c;
> > +		int index = 0;
> > +		struct option long_options[] = {
> > +			{"get",		no_argument, 0, 'g' },
> > +			{"arg",	  required_argument, 0, 'a' },
> > +			{"isset", 	no_argument, 0, 'i' },
> > +			{"dump",  optional_argument, 0, 'd' },
> > +			{"file",  required_argument, 0, 'f' },
> > +			{"verbose",	no_argument, 0, 'v' },
> > +			{NULL,			  0, 0, 0 }
> > +		};
> > +
> > +		c = getopt_long(argc, argv, "ga:id::f:v", long_options, &index);
> > +		if (c == -1) break;
> > +
> > +		switch (c) {
> > +			case 0:
> > +				break;
> > +			case 'f':
> > +				/* user specified souce path for config */
> > +				confpath = optarg;
> > +				break;
> > +			case 'a':
> > +				/* user supplied a sub-section name */
> > +				arg = optarg;
> > +				break;
> > +			case 'v':
> > +				verbose++;
> > +				break;
> > +			case 'g':
> > +				mode = MODE_GET;
> > +				break;
> > +			case 'i':
> > +				mode = MODE_ISSET;
> > +				break;
> > +			case 'd':
> > +				/* check if user supplied a filename for dump */
> > +				if (optarg == NULL && argv[optind] != NULL
> > +				    && argv[optind][0] != '-')
> > +					optarg = argv[optind++];
> > +				mode = MODE_DUMP;
> > +				dumpfile = optarg;
> > +				break;
> > +			default:
> > +				usage(argv[0]);
> > +				return 1;
> > +		}
> > +	}
> > +
> > +	/* configure the logging that conffile.c uses */
> > +	if (verbose)
> > +		xlog_config(D_ALL, 1);
> > +	xlog_stderr(1);
> > +	xlog_syslog(0);
> > +	xlog_open("nfsconftool");
> > +
> > +	if (mode == MODE_NONE) {
> > +		fprintf(stderr, "Error: No MODE selected.\n");
> > +		usage(argv[0]);
> > +		return 1;
> > +	}
> > +
> > +	if (conf_init_file(confpath)) {
> > +		/* config file was missing or had an error, warn about it */
> > +		if (verbose || mode != MODE_ISSET)
> > +			fprintf(stderr, "Error loading config file %s\n",
> > +				confpath);
> > +		/* this isnt fatal for --isset */
> > +		if (mode != MODE_ISSET)
> > +			return 1;
> > +	}
> > +
> > +	/* --dump mode, output the current configuration */
> > +	if (mode == MODE_DUMP) {
> > +		/* default to writing it to stdout */
> > +		FILE *out = stdout;
> > +
> > +		/* user specified a file to write to instead */
> > +		if (dumpfile) {
> > +			out = fopen(dumpfile, "w");
> > +			if (out == NULL) {
> > +				fprintf(stderr, "Error opening dumpfile %s: %s\n",
> > +					dumpfile, strerror(errno));
> > +				ret = 2;
> > +				goto cleanup;
> > +			}
> > +			if (verbose)
> > +				printf("Dumping config to %s\n", dumpfile);
> > +		}
> > +
> > +		/* output the configuration */
> > +		conf_report(out);
> > +
> > +		/* close that user specified file */
> > +		if (dumpfile)
> > +			fclose(out);
> > +	} else
> > +	/* --iset and --get share a lot of code */
> > +	if (mode == MODE_GET || mode == MODE_ISSET) {
> > +		char * section = NULL;
> > +		char * tag = NULL;
> > +
> > +		/* test they supplied section and tag names */
> > +		if (optind+1 >= argc) {
> > +			fprintf(stderr, "Error: insufficient arguments for mode\n");
> > +			usage(argv[0]);
> > +			ret = 2;
> > +			goto cleanup;
> > +		}
> > +
> > +		/* now we have a section and tag name */
> > +		section = argv[optind++];
> > +		tag = argv[optind++];
> > +
> > +		/* retrieve the specified tags value */
> > +		val = conf_get_section(section, arg, tag);
> > +		if (val != NULL) {
> > +			/* ret=0, success, mode --get wants to output the value as well */
> > +			if (mode == MODE_GET)
> > +				printf("%s\n", val);
> > +		} else {
> > +			/* ret=1, no value found, tell the user if they asked */
> > +			if (mode == MODE_GET && verbose)
> > +				fprintf(stderr, "Tag '%s' not found\n", tag);
> > +			ret = 1;
> > +		}
> > +	} else {
> > +		fprintf(stderr, "Mode not yet implimented.\n");
> > +		ret = 2;
> > +	}
> > +
> > +cleanup:
> > +	conf_cleanup();
> > +	return ret;
> > +}
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 7/8] nfs-utils: use nfsconftool cli to test library function

[Justin Mitchell]

Uses the nfsconftool cli to test that the configuration
library functions are operating as expected

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 tests/nfsconf/01-errors.conf | 20 ++++++++++++++++++++
 tests/nfsconf/01-errors.exp  | 14 ++++++++++++++
 tests/nfsconf/02-valid.conf  | 25 +++++++++++++++++++++++++
 tests/nfsconf/02-valid.exp   | 26 ++++++++++++++++++++++++++
 tests/nfsconf/02-valid.sub   |  7 +++++++
 tests/t0002-nfsconf.sh       | 38 ++++++++++++++++++++++++++++++++++++++
 6 files changed, 130 insertions(+)
 create mode 100644 tests/nfsconf/01-errors.conf
 create mode 100644 tests/nfsconf/01-errors.exp
 create mode 100644 tests/nfsconf/02-valid.conf
 create mode 100644 tests/nfsconf/02-valid.exp
 create mode 100644 tests/nfsconf/02-valid.sub
 create mode 100755 tests/t0002-nfsconf.sh

diff --git a/tests/nfsconf/01-errors.conf b/tests/nfsconf/01-errors.conf
new file mode 100644
index 0000000..ca64d2c
--- /dev/null
+++ b/tests/nfsconf/01-errors.conf
@@ -0,0 +1,20 @@
+# file of deliberate errors
+[default]
+
+
+[ one
+
+[ two " foo ]
+aa = foo
+
+[ three 
+val = none
+
+[four]
+one
+ = two
+three =
+four = foo = bar
+five = " nothing
+six = normal
+
diff --git a/tests/nfsconf/01-errors.exp b/tests/nfsconf/01-errors.exp
new file mode 100644
index 0000000..2bf1b8c
--- /dev/null
+++ b/tests/nfsconf/01-errors.exp
@@ -0,0 +1,14 @@
+nfsconf: config error at 01-errors.conf:5: non-matched ']', ignoring until next section
+nfsconf: config error at 01-errors.conf:7: non-matched '"', ignoring until next section
+nfsconf: config error at 01-errors.conf:10: non-matched ']', ignoring until next section
+nfsconf: config error at 01-errors.conf:11: ignoring line not in a section
+nfsconf: config error at 01-errors.conf:14: line not empty and not an assignment
+nfsconf: config error at 01-errors.conf:15: missing tag in assignment
+nfsconf: config error at 01-errors.conf:16: missing value in assignment
+nfsconf: config error at 01-errors.conf:18: unmatched quotes
+[four]
+ four = foo = bar
+ six = normal
+
+[two]
+ aa = foo
diff --git a/tests/nfsconf/02-valid.conf b/tests/nfsconf/02-valid.conf
new file mode 100644
index 0000000..ca8ccab
--- /dev/null
+++ b/tests/nfsconf/02-valid.conf
@@ -0,0 +1,25 @@
+[environment]
+one = 1
+two = 2
+three = 3
+
+[section_one]
+one = 11
+two = 22
+three = $three
+ four   =   "six  "
+ five six = seven eight
+
+[section_two "two"]
+one = Un
+two = Dau
+include = "02-valid.sub"
+
+[section_two "one"]
+one = Un
+two = Deux
+
+[section_two "two"]
+four = Pedwar
+   five   =    " Pump "
+
diff --git a/tests/nfsconf/02-valid.exp b/tests/nfsconf/02-valid.exp
new file mode 100644
index 0000000..379d7a4
--- /dev/null
+++ b/tests/nfsconf/02-valid.exp
@@ -0,0 +1,26 @@
+[environment]
+ one = 1
+ three = 3
+ two = 2
+
+[extra_section]
+ bar = baz
+ foo = bar
+
+[section_one]
+ five six = seven eight
+ four = "six  "
+ one = 11
+ three = $three
+ two = 22
+
+[section_two "one"]
+ one = Un
+ two = Deux
+
+[section_two "two"]
+ five = " Pump "
+ four = Pedwar
+ one = Un
+ three = Tri
+ two = Dau
diff --git a/tests/nfsconf/02-valid.sub b/tests/nfsconf/02-valid.sub
new file mode 100644
index 0000000..ab7beda
--- /dev/null
+++ b/tests/nfsconf/02-valid.sub
@@ -0,0 +1,7 @@
+# Included configs don't need a section, it is inherited
+three=Tri
+
+# But if they do, that works also
+[extra_section]
+foo = bar
+bar = baz
diff --git a/tests/t0002-nfsconf.sh b/tests/t0002-nfsconf.sh
new file mode 100755
index 0000000..511d248
--- /dev/null
+++ b/tests/t0002-nfsconf.sh
@@ -0,0 +1,38 @@
+#!/bin/bash
+
+TESTFILES=nfsconf
+DUMPER=`realpath ../tools/nfsconf/nfsconftool`
+
+BASEDIR=`dirname "$0"`
+pushd $BASEDIR/$TESTFILES
+
+rm -f *.out
+
+TCOUNT=0
+TPASS=0
+
+for i in *.conf
+do
+TNAME=`basename "$i" .conf`
+
+echo "Running test $TNAME"
+TCOUNT=$((TCOUNT + 1))
+
+if ! $DUMPER --file "$i" --dump - > "$TNAME.out" 2>&1
+then
+echo "Error running test $TNAME"
+elif ! diff -u "$TNAME.exp" "$TNAME.out"
+then
+echo "FAIL differences detected in test $TNAME"
+else
+echo "PASS $TNAME"
+TPASS=$((TPASS + 1))
+fi
+
+done
+
+echo "nfsconf tests complete. $TPASS of $TCOUNT tests passed"
+
+if [ $TPASS -lt $TCOUNT ]; then
+exit 1
+fi
-- 
1.8.3.1

[PATCH v3 8/8] nfs-utils: Add man page for nfsconftool cli

[Justin Mitchell]

Provide a man page for nfsconftool

Signed-off-by: Justin Mitchell <jumitche@redhat.com>
---
 tools/nfsconf/Makefile.am     |  3 ++
 tools/nfsconf/nfsconftool.man | 81 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+)
 create mode 100644 tools/nfsconf/nfsconftool.man

diff --git a/tools/nfsconf/Makefile.am b/tools/nfsconf/Makefile.am
index 0042db9..3e6923b 100644
--- a/tools/nfsconf/Makefile.am
+++ b/tools/nfsconf/Makefile.am
@@ -1,5 +1,8 @@
 ## Process this file with automake to produce Makefile.in
 
+man8_MANS	= nfsconftool.man
+EXTRA_DIST	= $(man8_MANS)
+
 bin_PROGRAMS = nfsconftool
 
 nfsconftool_SOURCES = nfsconfcli.c
diff --git a/tools/nfsconf/nfsconftool.man b/tools/nfsconf/nfsconftool.man
new file mode 100644
index 0000000..b70ec38
--- /dev/null
+++ b/tools/nfsconf/nfsconftool.man
@@ -0,0 +1,81 @@
+.\"
+.\" nfsconftool(8)
+.\"
+.TH nfsconftool 8 "2 May 2018"
+.SH NAME
+nfsconftool \- Query various NFS configuration settings
+.SH SYNOPSIS
+.B nfsconftool \-\-dump
+.RB [ \-v | \-\-verbose ]
+.RB [ \-f | \-\-file
+.IR infile.conf ]
+.RI [ outfile ]
+.P
+.B nfsconftool \-\-get
+.RB [ \-v | \-\-verbose ]
+.RB [ \-f | \-\-file
+.IR infile.conf ]
+.RB [ \-a | \-\-arg
+.IR subsection ]
+.IR section
+.IR tag
+.P
+.B nfsconftool \-\-isset
+.RB [ \-v | \-\-verbose ]
+.RB [ \-f | \-\-file
+.IR infile.conf ]
+.RB [ \-a | \-\-arg
+.IR subsection ]
+.IR section
+.IR tag
+.SH DESCRIPTION
+The
+.B nfsconftool
+command can be used to test for and retrieve configuration settings
+from a range of nfs-utils configuration files.
+.SS Modes
+The following modes are available:
+.IP "\fB\-d, \-\-dump\fP"
+Output an alphabetically sorted dump of the current configuration in conf file format. Accepts an optional filename in which to write the output.
+.IP "\fB\-i, \-\-isset\fP"
+Test if a specific tag has a value set.
+.IP "\fB\-g, \-\-get\fP"
+Output the current value of the specified tag.
+.SH OPTIONS
+.SS Options valid in all modes
+.TP
+.B \-v, \-\-verbose
+Increase verbosity and print debugging information.
+.TP
+.B \-f, \-\-file \fIinfile\fR
+Select a different config file to operate upon, default is
+.I /etc/nfs.conf
+.SS Options only valid in \fB\-\-get\fR and \fB\-\-isset\fR modes.
+.TP
+.B \-a, \-\-arg \fIsubsection\fR
+Select a specific sub-section
+.SH EXIT STATUS
+.SS \fB\-\-isset\fR mode
+In this mode the command will return success (0) if the selected tag has a value, any other exit code indicates the value is not set, or some other error has occured.
+.SS all other modes
+Success is indicated by an exit status of zero, any other status indicates an error. Error messages are output on stderr, and increasing verbosity will give more detailed explanations if any are available.
+.SH EXAMPLES
+.TP
+.B nfsconftool -v --dump --file /tmp/testconf.conf  sorted.conf
+Check a new config file for syntax errors and output a sorted version for ease of comparison with existing settings.
+.TP
+.B if ! nfsconftool --isset gssd preferred-realm ; then echo 'No prefered realm configured for gss'; fi
+The tool allows for easy testing of configuration values from shell scripts, here we test if a specific value has been set.
+.TP
+.B nfsconftool --file /etc/nfsmount.conf --get --arg /home MountPoint background
+Show default value for \fIbackground\fR option for NFS mounts of the \fI/home\fR path.
+.SH FILES
+.TP
+.B /etc/nfs.conf
+.SH SEE ALSO
+.BR nfsd (8),
+.BR exportfs (8),
+.BR idmapd (8),
+.BR statd (8)
+.SH AUTHOR
+Justin Mitchell <jumitche@redhat.com>
-- 
1.8.3.1

Re: [PATCH v3 0/8] nfs-utils: nfsconf cli tool and code tests

[Steve Dickson]

On 05/15/2018 11:50 AM, Justin Mitchell wrote:
> Apologies for the noise, I missed some files out when I last
> reposted, so here is an updated and tested version.
> 
> v3
> Include files that were missed out of v2
> Add the man page for nfsconftool
> 
> v2
> Updated style and commenting to make the code clearer
> 
> v1
> This series introduces a new cli tool to nfs-utils which provides
> for the dumping of the current configuration, querying of individual
> settings in a scripting friendly manner and by extention the 
> scripted testing and verification of nfsconf functionality.
> 
> The previously unreferenced conf_report() is modified to produce
> a sorted output, which allows you to --dump the current config in
> a way thats easier to diff
> 
> As well as some improved error messages a script is included which
> tests the current config parsing code by supplying various example
> config files and comparing the results of the --dump output.
> 
> The cli tool also allows a simple test to check if a config item
> has been set, and to retrieve the current value in a way that is
> easy to call from a shell script.
> 
> As the next stage I hope to impliment config setting via the cli 
> tool in order to provide an api for automation and management tools
> 
> Signed-off-by: Justin Mitchell <jumitche@redhat.com>
> 
> 
The series was committed with the name change... Thank you!

steved.