[PATCH 0/7 nfs-utils] Assorted improvements to handling nfsmount.conf

[PATCH 0/7 nfs-utils] Assorted improvements to handling nfsmount.conf

[NeilBrown]

The handling of version specifiers in mount.nfs, and even in the kernel,
is somewhat ideosyncratic when multiple versions are listed.

For example,  "-o vers=4.1,nfsvers=3" will result in both versions being
passed to the kernel, and the kernel complaining because version 3
doesn't support minor version numbers.
Conversly, "-o nfsvers=4.1,vers=3" will result in the "nfsvers=4.1"
being stripped off and vers=3 being used.

Further, version settings found in /etc/nfsmount.conf are sometimes
ignored if a version is given on the command line, and sometimes not.
If "nfsvers=3" is in the config file, then the presense of "-o vers=4.1"
will cause it to be ignored, the presense of "-o nfsvers=4.1" will too, but
mainly because of sloppy code.  However "-o v4.1" won't cause the config
file setting to be ignored.

This series cleans up all of this and some related issues, and updates
the man page.

With other options, the last option listed on the command line wins.
I have not tried to provide that for version options.  Instead, if there
are multiple version options listed, and error is reported.

Thanks,
NeilBrown

---

NeilBrown (7):
      mount: configfile: remove whitesspace from end of lines
      mount: report error if multiple version specifiers are given.
      Revert "mount.nfs: merge in vers= and nfsvers= options"
      mount: convert configfile.c to use parse_opt.c
      mount: options in config file shouldn't over-ride command-line options.
      mount: don't add config-file protcol version options when already present.
      mount: update nfsmount.conf man page


 utils/mount/configfile.c      | 230 +++++++++++-----------------------
 utils/mount/network.c         |  36 +++---
 utils/mount/nfsmount.conf.man | 110 ++++++++++------
 utils/mount/parse_opt.c       |  12 +-
 utils/mount/parse_opt.h       |   3 +-
 5 files changed, 174 insertions(+), 217 deletions(-)

--
Signature

[PATCH 2/7] mount: report error if multiple version specifiers are given.

[NeilBrown]

The NFS version can be requested with multiple different options:
  v2 v3 v4 v4.x vers=x nfsvers=

If multiple versions are given with different options, the choice of
which wins is quite ideosyncratic.  It certainly isn't simple "last one
wins" as with some other options.

Rather than providing a coherent rule, simply make multiple version
specifiers illegal.

This requires enhancing po_contains_prefix() to be able to look beyond
the first match, it see if there are multiple matches with the same
prefix, as well as checking all prefixes to see if more than one
matches.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 utils/mount/network.c   |   36 ++++++++++++++++++++++--------------
 utils/mount/parse_opt.c |   12 +++++++++---
 utils/mount/parse_opt.h |    3 ++-
 3 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index d9c0b513101d..e803dbbe5a2c 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1269,27 +1269,31 @@ int
 nfs_nfs_version(char *type, struct mount_options *options, struct nfs_version *version)
 {
 	char *version_key, *version_val = NULL, *cptr;
-	int i, found = 0;
+	int i, found = -1;
 
 	version->v_mode = V_DEFAULT;
 
 	for (i = 0; nfs_version_opttbl[i]; i++) {
 		if (po_contains_prefix(options, nfs_version_opttbl[i],
-				       &version_key) == PO_FOUND) {
-			found++;
-			break;
+				       &version_key, 0) == PO_FOUND) {
+			if (found >= 0)
+				goto ret_error_multiple;
+			if (po_contains_prefix(options, nfs_version_opttbl[i],
+					       NULL, 1) == PO_FOUND)
+				goto ret_error_multiple;
+			found = i;
 		}
 	}
 
-	if (!found && strcmp(type, "nfs4") == 0)
+	if (found < 0 && strcmp(type, "nfs4") == 0)
 		version_val = type + 3;
-	else if (!found)
+	else if (found < 0)
 		return 1;
-	else if (i <= 2 ) {
+	else if (found <= 2 ) {
 		/* v2, v3, v4 */
 		version_val = version_key + 1;
 		version->v_mode = V_SPECIFIC;
-	} else if (i > 2 ) {
+	} else if (found > 2 ) {
 		/* vers=, nfsvers= */
 		version_val = po_get(options, version_key);
 	}
@@ -1303,7 +1307,7 @@ nfs_nfs_version(char *type, struct mount_options *options, struct nfs_version *v
 	if (version->major == 4 && *cptr != '.' &&
 	    (version_val = po_get(options, "minorversion")) != NULL) {
 		version->minor = strtol(version_val, &cptr, 10);
-		i = -1;
+		found = -1;
 		if (*cptr)
 			goto ret_error;
 		version->v_mode = V_SPECIFIC;
@@ -1319,7 +1323,7 @@ nfs_nfs_version(char *type, struct mount_options *options, struct nfs_version *v
 		if (version_val != NULL) {
 			version->minor = strtol(version_val, &cptr, 10);
 			version->v_mode = V_SPECIFIC;
-		} else 
+		} else
 			version->v_mode = V_GENERAL;
 	}
 	if (*cptr != '\0')
@@ -1327,17 +1331,21 @@ nfs_nfs_version(char *type, struct mount_options *options, struct nfs_version *v
 
 	return 1;
 
+ret_error_multiple:
+	nfs_error(_("%s: multiple version options not permitted"),
+		  progname);
+	found = 10; /* avoid other errors */
 ret_error:
-	if (i < 0) {
+	if (found < 0) {
 		nfs_error(_("%s: parsing error on 'minorversion=' option"),
 			progname);
-	} else if (i <= 2 ) {
+	} else if (found <= 2 ) {
 		nfs_error(_("%s: parsing error on 'v' option"),
 			progname);
-	} else if (i == 3 ) {
+	} else if (found == 3 ) {
 		nfs_error(_("%s: parsing error on 'vers=' option"),
 			progname);
-	} else if (i == 4) {
+	} else if (found == 4) {
 		nfs_error(_("%s: parsing error on 'nfsvers=' option"),
 			progname);
 	}
diff --git a/utils/mount/parse_opt.c b/utils/mount/parse_opt.c
index 7ba61c4e52d8..b6065cad2888 100644
--- a/utils/mount/parse_opt.c
+++ b/utils/mount/parse_opt.c
@@ -414,19 +414,25 @@ po_found_t po_contains(struct mount_options *options, char *keyword)
  * @options: pointer to mount options
  * @prefix: pointer to prefix to match against a keyword
  * @keyword: pointer to a C string containing the option keyword if found
+ * @n: number of instances to skip, so '0' returns the first.
  *
  * On success, *keyword contains the pointer of the matching option's keyword.
  */
 po_found_t po_contains_prefix(struct mount_options *options,
-								const char *prefix, char **keyword)
+			      const char *prefix, char **keyword, int n)
 {
 	struct mount_option *option;
 
 	if (options && prefix) {
 		for (option = options->head; option; option = option->next)
 			if (strncmp(option->keyword, prefix, strlen(prefix)) == 0) {
-				*keyword = option->keyword;
-				return PO_FOUND;
+				if (n > 0) {
+					n -= 1;
+				} else {
+					if (keyword)
+						*keyword = option->keyword;
+					return PO_FOUND;
+				}
 			}
 	}
 
diff --git a/utils/mount/parse_opt.h b/utils/mount/parse_opt.h
index 0745e0f0833e..0a153768d434 100644
--- a/utils/mount/parse_opt.h
+++ b/utils/mount/parse_opt.h
@@ -46,7 +46,8 @@ po_return_t		po_join(struct mount_options *, char **);
 po_return_t		po_append(struct mount_options *, char *);
 po_found_t		po_contains(struct mount_options *, char *);
 po_found_t		po_contains_prefix(struct mount_options *options,
-						const char *prefix, char **keyword);
+					   const char *prefix, char **keyword,
+					   int n);
 char *			po_get(struct mount_options *, char *);
 po_found_t		po_get_numeric(struct mount_options *,
 					char *, long *);

[PATCH 1/7] mount: configfile: remove whitesspace from end of lines

[NeilBrown]

While space at end of line is ugly..  especially when your editor is
configured to show it in RED.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 utils/mount/configfile.c |   67 +++++++++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
index 93fe500bc7a2..2470bc6a8bf6 100644
--- a/utils/mount/configfile.c
+++ b/utils/mount/configfile.c
@@ -1,5 +1,5 @@
 /*
- * configfile.c -- mount configuration file manipulation 
+ * configfile.c -- mount configuration file manipulation
  * Copyright (C) 2008 Red Hat, Inc <nfs@redhat.com>
  *
  * - Routines use to create mount options from the mount
@@ -77,10 +77,10 @@ int mnt_alias_sz = (sizeof(mnt_alias_tab)/sizeof(mnt_alias_tab[0]));
 static int strict;
 
 /*
- * See if the option is an alias, if so return the 
+ * See if the option is an alias, if so return the
  * real mount option along with the argument type.
  */
-inline static 
+inline static
 char *mountopts_alias(char *opt, int *argtype)
 {
 	int i;
@@ -99,7 +99,7 @@ char *mountopts_alias(char *opt, int *argtype)
 }
 /*
  * Convert numeric strings that end with 'k', 'm' or 'g'
- * into numeric strings with the real value. 
+ * into numeric strings with the real value.
  * Meaning '8k' becomes '8094'.
  */
 char *mountopts_convert(char *value)
@@ -146,26 +146,26 @@ static int list_size;
 /*
  * Add option to the link list
  */
-inline static void 
+inline static void
 add_entry(char *opt)
 {
 	struct entry *entry;
 
 	entry = calloc(1, sizeof(struct entry));
 	if (entry == NULL) {
-		xlog_warn("Unable calloc memory for mount configs"); 
+		xlog_warn("Unable calloc memory for mount configs");
 		return;
 	}
 	entry->opt = strdup(opt);
 	if (entry->opt == NULL) {
-		xlog_warn("Unable calloc memory for mount opts"); 
+		xlog_warn("Unable calloc memory for mount opts");
 		free(entry);
 		return;
 	}
 	SLIST_INSERT_HEAD(&head, entry, entries);
 }
 /*
- * Check the alias list to see if the given 
+ * Check the alias list to see if the given
  * opt is a alias
  */
 char *is_alias(char *opt)
@@ -174,7 +174,7 @@ char *is_alias(char *opt)
 
 	for (i=0; i < mnt_alias_sz; i++) {
 		if (strcasecmp(opt, mnt_alias_tab[i].alias) == 0)
-			return mnt_alias_tab[i].opt; 
+			return mnt_alias_tab[i].opt;
 	}
 	return NULL;
 }
@@ -182,7 +182,7 @@ char *is_alias(char *opt)
  * See if the given entry exists if the link list,
  * if so return that entry
  */
-inline static 
+inline static
 char *lookup_entry(char *opt)
 {
 	struct entry *entry;
@@ -217,7 +217,7 @@ char *lookup_entry(char *opt)
 /*
  * Free all entries on the link list
  */
-inline static 
+inline static
 void free_all(void)
 {
 	struct entry *entry;
@@ -236,10 +236,10 @@ extern sa_family_t config_default_family;
 
 /*
  * Check to see if a default value is being set.
- * If so, set the appropriate global value which will 
+ * If so, set the appropriate global value which will
  * be used as the initial value in the server negation.
  */
-static int 
+static int
 default_value(char *mopt)
 {
 	struct mount_options *options = NULL;
@@ -253,11 +253,11 @@ default_value(char *mopt)
 	if (strncasecmp(field, "proto", strlen("proto")) == 0) {
 		if ((options = po_split(field)) != NULL) {
 			if (!nfs_nfs_protocol(options, &config_default_proto)) {
-				xlog_warn("Unable to set default protocol : %s", 
+				xlog_warn("Unable to set default protocol : %s",
 					strerror(errno));
 			}
 			if (!nfs_nfs_proto_family(options, &config_default_family)) {
-				xlog_warn("Unable to set default family : %s", 
+				xlog_warn("Unable to set default family : %s",
 					strerror(errno));
 			}
 		} else {
@@ -266,14 +266,13 @@ default_value(char *mopt)
 	} else if (strncasecmp(field, "vers", strlen("vers")) == 0) {
 		if ((options = po_split(field)) != NULL) {
 			if (!nfs_nfs_version("nfs", options, &config_default_vers)) {
-				xlog_warn("Unable to set default version: %s", 
+				xlog_warn("Unable to set default version: %s",
 					strerror(errno));
-				
 			}
 		} else {
 			xlog_warn("Unable to alloc memory for default version");
 		}
-	} else 
+	} else
 		xlog_warn("Invalid default setting: '%s'", mopt);
 
 	if (options)
@@ -282,11 +281,11 @@ default_value(char *mopt)
 	return 1;
 }
 /*
- * Parse the given section of the configuration 
+ * Parse the given section of the configuration
  * file to if there are any mount options set.
  * If so, added them to link list.
  */
-static void 
+static void
 conf_parse_mntopts(char *section, char *arg, char *opts)
 {
 	struct conf_list *list;
@@ -300,7 +299,7 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
 		/* check first if this is an alias for another option */
 		field = mountopts_alias(node->field, &argtype);
 		/*
-		 * Do not overwrite options if already exists 
+		 * Do not overwrite options if already exists
 		 */
 		snprintf(buf, BUFSIZ, "%s=", field);
 		if (opts && strcasestr(opts, buf) != NULL)
@@ -333,8 +332,8 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
 		}
 		if (buf[0] == '\0')
 			continue;
-		/* 
-		 * Keep a running tally of the list size adding 
+		/*
+		 * Keep a running tally of the list size adding
 		 * one for the ',' that will be appened later
 		 */
 		list_size += strlen(buf) + 1;
@@ -344,14 +343,14 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
 }
 
 /*
- * Concatenate options from the configuration file with the 
+ * Concatenate options from the configuration file with the
  * given options by building a link list of options from the
- * different sections in the conf file. Options that exists 
- * in the either the given options or link list are not 
+ * different sections in the conf file. Options that exists
+ * in the either the given options or link list are not
  * overwritten so it matter which when each section is
- * parsed. 
+ * parsed.
  */
-char *conf_get_mntopts(char *spec, char *mount_point, 
+char *conf_get_mntopts(char *spec, char *mount_point,
 	char *mount_opts)
 {
 	struct entry *entry;
@@ -362,18 +361,18 @@ char *conf_get_mntopts(char *spec, char *mount_point,
 	SLIST_INIT(&head);
 	list_size = 0;
 	/*
-	 * First see if there are any mount options relative 
+	 * First see if there are any mount options relative
 	 * to the mount point.
 	 */
 	conf_parse_mntopts(NFSMOUNT_MOUNTPOINT, mount_point, mount_opts);
 
-	/* 
+	/*
 	 * Next, see if there are any mount options relative
 	 * to the server
 	 */
 	server = strdup(spec);
 	if (server == NULL) {
-		xlog_warn("conf_get_mountops: Unable calloc memory for server"); 
+		xlog_warn("conf_get_mountops: Unable calloc memory for server");
 		free_all();
 		return mount_opts;
 	}
@@ -383,7 +382,7 @@ char *conf_get_mntopts(char *spec, char *mount_point,
 	free(server);
 
 	/*
-	 * Finally process all the global mount options. 
+	 * Finally process all the global mount options.
 	 */
 	conf_parse_mntopts(NFSMOUNT_GLOBAL_OPTS, NULL, mount_opts);
 
@@ -396,7 +395,7 @@ char *conf_get_mntopts(char *spec, char *mount_point,
 
 	/*
 	 * Found options in the configuration file. So
-	 * concatenate the configuration options with the 
+	 * concatenate the configuration options with the
 	 * options that were passed in
 	 */
 	if (mount_opts)
@@ -405,7 +404,7 @@ char *conf_get_mntopts(char *spec, char *mount_point,
 	/* list_size + optlen + ',' + '\0' */
 	config_opts = calloc(1, (list_size+optlen+2));
 	if (config_opts == NULL) {
-		xlog_warn("conf_get_mountops: Unable calloc memory for config_opts"); 
+		xlog_warn("conf_get_mountops: Unable calloc memory for config_opts");
 		free_all();
 		return mount_opts;
 	}

[PATCH 3/7] Revert "mount.nfs: merge in vers= and nfsvers= options"

[NeilBrown]

This reverts commit 8110103404b35d9e86057ef0764f8aa87585f455.

Using mnt_alias_tab[] to handle options which are synonyms isn't really
a good fit.  This sort-of works, but in part only because 'strstr()'
is used for matching so "vers=" is found when "nfsvers=" is present.
This doesn't handle other version-setting options like v2, v3, v4.x.

So remove this commit to make room for a better solution.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 utils/mount/configfile.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
index 2470bc6a8bf6..e20aa73739fc 100644
--- a/utils/mount/configfile.c
+++ b/utils/mount/configfile.c
@@ -70,7 +70,6 @@ struct mnt_alias {
 	{"background", "bg", MNT_NOARG},
 	{"foreground", "fg", MNT_NOARG},
 	{"sloppy", "sloppy", MNT_NOARG},
-	{"nfsvers", "vers", MNT_UNSET},
 };
 int mnt_alias_sz = (sizeof(mnt_alias_tab)/sizeof(mnt_alias_tab[0]));
 
@@ -296,21 +295,20 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
 
 	list = conf_get_tag_list(section, arg);
 	TAILQ_FOREACH(node, &list->fields, link) {
-		/* check first if this is an alias for another option */
-		field = mountopts_alias(node->field, &argtype);
 		/*
 		 * Do not overwrite options if already exists
 		 */
-		snprintf(buf, BUFSIZ, "%s=", field);
+		snprintf(buf, BUFSIZ, "%s=", node->field);
 		if (opts && strcasestr(opts, buf) != NULL)
 			continue;
 
-		if (lookup_entry(field) != NULL)
+		if (lookup_entry(node->field) != NULL)
 			continue;
 		buf[0] = '\0';
 		value = conf_get_section(section, arg, node->field);
 		if (value == NULL)
 			continue;
+		field = mountopts_alias(node->field, &argtype);
 		if (strcasecmp(value, "false") == 0) {
 			if (argtype != MNT_NOARG)
 				snprintf(buf, BUFSIZ, "no%s", field);

[PATCH 5/7] mount: options in config file shouldn't over-ride command-line options.

[NeilBrown]

When reading from the config file, we already ignore options that exist
on the command line, or that were already found earlier in the config
file.  However this only works for exact matches of options.

e.g. if "noac" is on the command line and "ac=true" is in the config file,
then "ac" will be added, and this will be used.

Add tests for the "no" prefix, and also for "fg" vs "bg", so that if
"fg" is set on the command line, a "bg" or "background" setting in the
config file does not over-ride it.

Note that this *doesn't* handle the different protocol version
specifiers.  That will come later.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 utils/mount/configfile.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
index 8c68ff2c1323..40378ab247fc 100644
--- a/utils/mount/configfile.c
+++ b/utils/mount/configfile.c
@@ -204,6 +204,27 @@ conf_parse_mntopts(char *section, char *arg, struct mount_options *options)
 		field = mountopts_alias(node->field, &argtype);
 		if (po_contains(options, field) == PO_FOUND)
 			continue;
+		/* Some options can be inverted by a "no" prefix.
+		 * Check for these.
+		 * "no" prefixes are unlikely in the config file as
+		 * "option=false" is preferred, but still possible.
+		 */
+		if (strncmp(field, "no", 2) == 0 &&
+		    po_contains(options, field+2) == PO_FOUND)
+			continue;
+		if (strlen(field) < BUFSIZ-3) {
+			strcat(strcpy(buf, "no"), field);
+			if (po_contains(options, buf) == PO_FOUND)
+				continue;
+		}
+
+		/* If fg or bg already present, ignore bg or fg */
+		if (strcmp(field, "fg") == 0 &&
+		    po_contains(options, "bg") == PO_FOUND)
+			continue;
+		if (strcmp(field, "bg") == 0 &&
+		    po_contains(options, "fg") == PO_FOUND)
+			continue;
 
 		buf[0] = '\0';
 		value = conf_get_section(section, arg, node->field);

[PATCH 4/7] mount: convert configfile.c to use parse_opt.c

[NeilBrown]

configfile.c contains some ad-hoc parsing of the comma-separated option
list, which uses a simple 'strstr' and can find options that don't
really match the searched-for option (the found options might have a
prefix).

It also has a list of options which duplicates the functionality in
parse_opt.

This can be simplified by using parse_opt directly.  We split the
original arguments, optionally append new arguments if they don't
already exist, then recombine.

"defaultfoo" config options require special handling.  The
default_value() call is now made as soon as the option has been parsed.
It is left on the options list so that new instances of the value are
ignored.  Then all "defaultfoo" options are remove from the list at the
end.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 utils/mount/configfile.c |  183 ++++++----------------------------------------
 1 file changed, 25 insertions(+), 158 deletions(-)

diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
index e20aa73739fc..8c68ff2c1323 100644
--- a/utils/mount/configfile.c
+++ b/utils/mount/configfile.c
@@ -34,10 +34,7 @@
 #include "parse_opt.h"
 #include "network.h"
 #include "conffile.h"
-
-char *mountopts_convert(char *value);
-char *is_alias(char *opt);
-char *conf_get_mntopts(char *spec, char *mount_point, char *mount_opts);
+#include "mount_config.h"
 
 #define KBYTES(x)     ((x) * (1024))
 #define MEGABYTES(x)  ((x) * (1048576))
@@ -101,7 +98,7 @@ char *mountopts_alias(char *opt, int *argtype)
  * into numeric strings with the real value.
  * Meaning '8k' becomes '8094'.
  */
-char *mountopts_convert(char *value)
+static char *mountopts_convert(char *value)
 {
 	unsigned long long factor, num;
 	static char buf[64];
@@ -135,100 +132,6 @@ char *mountopts_convert(char *value)
 	return buf;
 }
 
-struct entry {
-	SLIST_ENTRY(entry) entries;
-	char *opt;
-};
-static SLIST_HEAD(shead, entry) head = SLIST_HEAD_INITIALIZER(head);
-static int list_size;
-
-/*
- * Add option to the link list
- */
-inline static void
-add_entry(char *opt)
-{
-	struct entry *entry;
-
-	entry = calloc(1, sizeof(struct entry));
-	if (entry == NULL) {
-		xlog_warn("Unable calloc memory for mount configs");
-		return;
-	}
-	entry->opt = strdup(opt);
-	if (entry->opt == NULL) {
-		xlog_warn("Unable calloc memory for mount opts");
-		free(entry);
-		return;
-	}
-	SLIST_INSERT_HEAD(&head, entry, entries);
-}
-/*
- * Check the alias list to see if the given
- * opt is a alias
- */
-char *is_alias(char *opt)
-{
-	int i;
-
-	for (i=0; i < mnt_alias_sz; i++) {
-		if (strcasecmp(opt, mnt_alias_tab[i].alias) == 0)
-			return mnt_alias_tab[i].opt;
-	}
-	return NULL;
-}
-/*
- * See if the given entry exists if the link list,
- * if so return that entry
- */
-inline static
-char *lookup_entry(char *opt)
-{
-	struct entry *entry;
-	char *alias = is_alias(opt);
-	char *ptr;
-
-	SLIST_FOREACH(entry, &head, entries) {
-		/*
-		 * Only check the left side or options that use '='
-		 */
-		if ((ptr = strchr(entry->opt, '=')) != 0) {
-			int len = (int) (ptr - entry->opt);
-
-			if (strncasecmp(entry->opt, opt, len) == 0)
-				return opt;
-		}
-		if (strcasecmp(entry->opt, opt) == 0)
-			return opt;
-		if (alias && strcasecmp(entry->opt, alias) == 0)
-			return opt;
-		if (alias && strcasecmp(alias, "fg") == 0) {
-			if (strcasecmp(entry->opt, "bg") == 0)
-				return opt;
-		}
-		if (alias && strcasecmp(alias, "bg") == 0) {
-			if (strcasecmp(entry->opt, "fg") == 0)
-				return opt;
-		}
-	}
-	return NULL;
-}
-/*
- * Free all entries on the link list
- */
-inline static
-void free_all(void)
-{
-	struct entry *entry;
-
-	while (!SLIST_EMPTY(&head)) {
-		entry = SLIST_FIRST(&head);
-		SLIST_REMOVE_HEAD(&head, entries);
-		free(entry->opt);
-		free(entry);
-	}
-}
-
 struct nfs_version config_default_vers;
 unsigned long config_default_proto;
 extern sa_family_t config_default_family;
@@ -285,7 +188,7 @@ default_value(char *mopt)
  * If so, added them to link list.
  */
 static void
-conf_parse_mntopts(char *section, char *arg, char *opts)
+conf_parse_mntopts(char *section, char *arg, struct mount_options *options)
 {
 	struct conf_list *list;
 	struct conf_list_node *node;
@@ -298,17 +201,14 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
 		/*
 		 * Do not overwrite options if already exists
 		 */
-		snprintf(buf, BUFSIZ, "%s=", node->field);
-		if (opts && strcasestr(opts, buf) != NULL)
+		field = mountopts_alias(node->field, &argtype);
+		if (po_contains(options, field) == PO_FOUND)
 			continue;
 
-		if (lookup_entry(node->field) != NULL)
-			continue;
 		buf[0] = '\0';
 		value = conf_get_section(section, arg, node->field);
 		if (value == NULL)
 			continue;
-		field = mountopts_alias(node->field, &argtype);
 		if (strcasecmp(value, "false") == 0) {
 			if (argtype != MNT_NOARG)
 				snprintf(buf, BUFSIZ, "no%s", field);
@@ -330,12 +230,8 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
 		}
 		if (buf[0] == '\0')
 			continue;
-		/*
-		 * Keep a running tally of the list size adding
-		 * one for the ',' that will be appened later
-		 */
-		list_size += strlen(buf) + 1;
-		add_entry(buf);
+		po_append(options, buf);
+		default_value(buf);
 	}
 	conf_free_list(list);
 }
@@ -349,20 +245,22 @@ conf_parse_mntopts(char *section, char *arg, char *opts)
  * parsed.
  */
 char *conf_get_mntopts(char *spec, char *mount_point,
-	char *mount_opts)
+			      char *mount_opts)
 {
-	struct entry *entry;
-	char *ptr, *server, *config_opts;
-	int optlen = 0;
+	struct mount_options *options;
+	char *ptr, *server;
 
 	strict = 0;
-	SLIST_INIT(&head);
-	list_size = 0;
+	options = po_split(mount_opts);
+	if (!options) {
+		xlog_warn("conf_get_mountops: Unable calloc memory for options");
+		return mount_opts;
+	}
 	/*
 	 * First see if there are any mount options relative
 	 * to the mount point.
 	 */
-	conf_parse_mntopts(NFSMOUNT_MOUNTPOINT, mount_point, mount_opts);
+	conf_parse_mntopts(NFSMOUNT_MOUNTPOINT, mount_point, options);
 
 	/*
 	 * Next, see if there are any mount options relative
@@ -371,58 +269,27 @@ char *conf_get_mntopts(char *spec, char *mount_point,
 	server = strdup(spec);
 	if (server == NULL) {
 		xlog_warn("conf_get_mountops: Unable calloc memory for server");
-		free_all();
+		po_destroy(options);
 		return mount_opts;
 	}
 	if ((ptr = strchr(server, ':')) != NULL)
 		*ptr='\0';
-	conf_parse_mntopts(NFSMOUNT_SERVER, server, mount_opts);
+	conf_parse_mntopts(NFSMOUNT_SERVER, server, options);
 	free(server);
 
 	/*
 	 * Finally process all the global mount options.
 	 */
-	conf_parse_mntopts(NFSMOUNT_GLOBAL_OPTS, NULL, mount_opts);
-
-	/*
-	 * If no mount options were found in the configuration file
-	 * just return what was passed in .
-	 */
-	if (SLIST_EMPTY(&head))
-		return mount_opts;
+	conf_parse_mntopts(NFSMOUNT_GLOBAL_OPTS, NULL, options);
 
 	/*
-	 * Found options in the configuration file. So
-	 * concatenate the configuration options with the
-	 * options that were passed in
+	 * Strip out defaults, which have already been handled,
+	 * then join the rest and return.
 	 */
-	if (mount_opts)
-		optlen = strlen(mount_opts);
-
-	/* list_size + optlen + ',' + '\0' */
-	config_opts = calloc(1, (list_size+optlen+2));
-	if (config_opts == NULL) {
-		xlog_warn("conf_get_mountops: Unable calloc memory for config_opts");
-		free_all();
-		return mount_opts;
-	}
-
-	if (mount_opts) {
-		strcpy(config_opts, mount_opts);
-		strcat(config_opts, ",");
-	}
-	SLIST_FOREACH(entry, &head, entries) {
-		if (default_value(entry->opt))
-			continue;
-		strcat(config_opts, entry->opt);
-		strcat(config_opts, ",");
-	}
-	if ((ptr = strrchr(config_opts, ',')) != NULL)
-		*ptr = '\0';
+	po_remove_all(options, "default");
 
-	free_all();
-	if (mount_opts)
-		free(mount_opts);
+	po_join(options, &mount_opts);
+	po_destroy(options);
 
-	return config_opts;
+	return mount_opts;
 }

[PATCH 6/7] mount: don't add config-file protcol version options when already present.

[NeilBrown]

If there is already an option specifying the protocol version, whether
from the command line or from an earlier config section, don't add new
version options.

There are multiple different version options, so they need to be handled
differently from other options.  There could in the future be more
options that start "v4.", e.g.  "v4.3" might happen one day.  So rather
than list possible "v4.x" options, handle "v4." separately.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 utils/mount/configfile.c |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
index 40378ab247fc..7934f4f625d9 100644
--- a/utils/mount/configfile.c
+++ b/utils/mount/configfile.c
@@ -70,8 +70,23 @@ struct mnt_alias {
 };
 int mnt_alias_sz = (sizeof(mnt_alias_tab)/sizeof(mnt_alias_tab[0]));
 
+static const char *version_keys[] = {
+	"v2", "v3", "v4", "vers", "nfsvers", "minorversion", NULL
+};
+
 static int strict;
 
+static int is_version(const char *field)
+{
+	int i;
+	for (i = 0; version_keys[i] ; i++)
+		if (strcmp(version_keys[i], field) == 0)
+			return 1;
+	if (strncmp(field, "v4.", 3) == 0)
+		return 1;
+	return 0;
+}
+
 /*
  * See if the option is an alias, if so return the
  * real mount option along with the argument type.
@@ -195,6 +210,11 @@ conf_parse_mntopts(char *section, char *arg, struct mount_options *options)
 	char buf[BUFSIZ], *value, *field;
 	char *nvalue, *ptr;
 	int argtype;
+	int have_version = 0;
+
+	if (po_rightmost(options, version_keys) >= 0 ||
+	    po_contains_prefix(options, "v4.", NULL, 0) == PO_FOUND)
+		have_version = 1;
 
 	list = conf_get_tag_list(section, arg);
 	TAILQ_FOREACH(node, &list->fields, link) {
@@ -226,6 +246,12 @@ conf_parse_mntopts(char *section, char *arg, struct mount_options *options)
 		    po_contains(options, "fg") == PO_FOUND)
 			continue;
 
+		if (is_version(field)) {
+			if (have_version)
+				continue;
+			have_version = 1;
+		}
+
 		buf[0] = '\0';
 		value = conf_get_section(section, arg, node->field);
 		if (value == NULL)

[PATCH 7/7] mount: update nfsmount.conf man page

[NeilBrown]

Multiple changes including:
- using \[dq] for double quotes rather than \(lq and \(rq.
  In almost every case, a regular ASCII double quote is being
  referred to, so that is what we should use.
- clean up indenting in examples.
- be explicit about case-insensitive matching.
- give more details about permitted options, including the
  need to use =true and =false for flags
- explain Backgroud, Forground and Sloppy
- remain trailing white space

Signed-off-by: NeilBrown <neilb@suse.de>
---
 utils/mount/nfsmount.conf.man |  110 ++++++++++++++++++++++++++---------------
 1 file changed, 70 insertions(+), 40 deletions(-)

diff --git a/utils/mount/nfsmount.conf.man b/utils/mount/nfsmount.conf.man
index 4f8f351addf4..73c3e1188541 100644
--- a/utils/mount/nfsmount.conf.man
+++ b/utils/mount/nfsmount.conf.man
@@ -1,53 +1,84 @@
-.\"@(#)nfsmount.conf.5"
-.TH NFSMOUNT.CONF 5 "9 October 2012"
+."@(#)nfsmount.conf.5"
+.TH NFSMOUNT.CONF 5 "16 December 2020"
 .SH NAME
 nfsmount.conf - Configuration file for NFS mounts
 .SH SYNOPSIS
 Configuration file for NFS mounts that allows options
 to be set globally, per server or per mount point.
 .SH DESCRIPTION
-The configuration file is made up of multiple sections 
-followed by variables associated with that section.
-A section is defined by a string enclosed by 
+The configuration file is made up of multiple section headers
+followed by variable assignments associated with that section.
+A section header is defined by a string enclosed by
 .BR [
-and 
+and
 .BR ]
-branches.
-Variables are assignment statements that assign values 
-to particular variables using the  
-.BR = 
-operator, as in 
+brackets.
+Variable assignments are assignment statements that assign values
+to particular variables using the
+.BR =
+operator, as in
 .BR Proto=Tcp .
-The variables that can be assigned are exactly the set of NFS specific
+The variables that can be assigned are the set of NFS specific
 mount options listed in
-.BR nfs (5).
+.BR nfs (5)
+together with the filesystem-independant mount options listed in
+.BR mount (8)
+and three additions:
+.B Sloppy=True
+has the same effect as the
+.B -s
+option to
+.IR mount ,
+and
+.B Foreground=True
+and
+.B Background=True
+have the same effect as
+.B bg
+and
+.BR fg .
+.PP
+Options in the config file may be given in upper, lower, or mixed case
+and will be shifted to lower case before being passed to the filesystem.
+.PP
+Boolean mount options which do not need an equals sign must be given as
+.RI \[dq] option =True".
+Instead of preceeding such an option with
+.RB \[dq] no \[dq]
+its negation must be given as
+.RI \[dq] option =False".
 .PP
 Sections are broken up into three basic categories:
 Global options, Server options and Mount Point options.
 .HP
 .B [ NFSMount_Global_Options ]
 - This statically named section
-defines all of the global mount options that can be 
+defines all of the global mount options that can be
 applied to every NFS mount.
 .HP
-.B [ Server \(lqServer_Name\(rq ] 
-- This section defines all the mount options that should 
-be used on mounts to a particular NFS server. The 
-.I \(lqServer_Name\(rq
-strings needs to be surrounded by '\(lq' and 
-be an exact match of the server name used in the 
+.B [ Server \[dq]Server_Name\[dq] ]
+- This section defines all the mount options that should
+be used on mounts to a particular NFS server. The
+.I \[dq]Server_Name\[dq]
+strings needs to be surrounded by '\[dq]' and be an exact match
+(ignoring case) of the server name used in the
 .B mount
-command. 
+command.
 .HP
-.B [ MountPoint \(lqMount_Point\(rq ]
-- This section defines all the mount options that 
+.B [ MountPoint \[dq]Mount_Point\[dq] ]
+- This section defines all the mount options that
 should be used on a particular mount point.
-The 
-.I \(lqMount_Point\(rq
-string needs to be surrounded by '\(lq' and be an 
-exact match of the mount point used in the 
-.BR mount 
-command.
+The
+.I \[dq]Mount_Point\[dq]
+string needs to be surrounded by '\[dq]' and be an
+exact match of the mount point used in the
+.BR mount
+command.  Though path names are usually case-sensitive, the Mount_Point
+name is matched insensitive to case.
+.PP
+The sections are processed in the reverse of the order listed above, and
+any options already seen, either in a previous section or on the
+command line, will be ignored when seen again.
 .SH EXAMPLES
 .PP
 These are some example lines of how sections and variables
@@ -57,43 +88,42 @@ are defined in the configuration file.
 .br
     Proto=Tcp
 .RS
-.HP
+.PP
 The TCP/IPv4 protocol will be used on every NFS mount.
-.HP
 .RE
-[ Server \(lqnfsserver.foo.com\(rq ]
+.PP
+[ Server \[dq]nfsserver.foo.com\[dq] ]
 .br
     rsize=32k
 .br
     wsize=32k
 .br
     proto=udp6
-.HP
 .RS
+.PP
 A 32k (32768 bytes) block size will be used as the read and write
 size on all mounts to the 'nfsserver.foo.com' server.  UDP/IPv6
 is the protocol to be used.
-.HP
 .RE
-.BR 
-[ MountPoint \(lq/export/home\(rq ]
+.PP
+[ MountPoint \[dq]/export/home\[dq] ]
 .br
     Background=True
 .RS
-.HP
+.PP
 All mounts to the '/export/home' export will be performed in
 the background (i.e. done asynchronously).
-.HP
+.RE
 .SH FILES
 .TP 10n
 .I /etc/nfsmount.conf
 Default NFS mount configuration file
 .TP 10n
 .I /etc/nfsmount.conf.d
-When this directory exists and files ending 
+When this directory exists and files ending
 with ".conf" exist, those files will be
 used to set configuration variables. These
-files will override variables set 
+files will override variables set
 in /etc/nfsmount.conf
 .PD
 .SH SEE ALSO

Re: [PATCH 0/7 nfs-utils] Assorted improvements to handling nfsmount.conf

[Steve Dickson]

On 12/15/20 11:43 PM, NeilBrown wrote:
> The handling of version specifiers in mount.nfs, and even in the kernel,
> is somewhat ideosyncratic when multiple versions are listed.
> 
> For example,  "-o vers=4.1,nfsvers=3" will result in both versions being
> passed to the kernel, and the kernel complaining because version 3
> doesn't support minor version numbers.
> Conversly, "-o nfsvers=4.1,vers=3" will result in the "nfsvers=4.1"
> being stripped off and vers=3 being used.
> 
> Further, version settings found in /etc/nfsmount.conf are sometimes
> ignored if a version is given on the command line, and sometimes not.
> If "nfsvers=3" is in the config file, then the presense of "-o vers=4.1"
> will cause it to be ignored, the presense of "-o nfsvers=4.1" will too, but
> mainly because of sloppy code.  However "-o v4.1" won't cause the config
> file setting to be ignored.
> 
> This series cleans up all of this and some related issues, and updates
> the man page.
> 
> With other options, the last option listed on the command line wins.
> I have not tried to provide that for version options.  Instead, if there
> are multiple version options listed, and error is reported.
> 
> Thanks,
> NeilBrown
The series is Committed! (tag:  nfs-utils-2-5-3-rc3)

steved.
 
> 
> ---
> 
> NeilBrown (7):
>       mount: configfile: remove whitesspace from end of lines
>       mount: report error if multiple version specifiers are given.
>       Revert "mount.nfs: merge in vers= and nfsvers= options"
>       mount: convert configfile.c to use parse_opt.c
>       mount: options in config file shouldn't over-ride command-line options.
>       mount: don't add config-file protcol version options when already present.
>       mount: update nfsmount.conf man page
> 
> 
>  utils/mount/configfile.c      | 230 +++++++++++-----------------------
>  utils/mount/network.c         |  36 +++---
>  utils/mount/nfsmount.conf.man | 110 ++++++++++------
>  utils/mount/parse_opt.c       |  12 +-
>  utils/mount/parse_opt.h       |   3 +-
>  5 files changed, 174 insertions(+), 217 deletions(-)
> 
> --
> Signature
>