All of lore.kernel.org
 help / color / mirror / Atom feed
* [Patch 0/9] NFS Mount Configuration File
@ 2009-03-09 20:44 Steve Dickson
  2009-03-09 20:47 ` [Patch 1/9] Make idmapd's Configuration Parsing Code Available Steve Dickson
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 20:44 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

Hello,

The following patch set introduces a configuration file where 
mount options can be define. The options in the file can be 
applied globally or per server or per mount point. 

The patch set reuses the configuration parsing code that
idmapd uses. I did added a couple enhancements like ignoring
blanks in certain definitions as well as at the beginning of 
assignment statements.

There are man pages include in the patch set but in a
nut shell, the configuration file has three basic types
of sections. A global section, server section and mount point
section. There can only be one global section and multiple 
server and mount point sections.

The mount command prioritize these sections in the following
way:
     * Options on the command line are always used.

     * Options defined in the mount point section are used
       as long a the options are not in the command line options.

     * Options defined in the server section are used as long as
       they are not defined on the command line or in the mount point
       section.

     * Options defined in the global section are used as long as the
       options are not on the command line or in the mount point section
       or in the server section.

This is the first step toward moving the default NFS version from 3 to 4
on the client. Having a configuration file which allows users to define
the maximum and minimum NFS versions that should be negotiated is the
right thing to do... IMHO.. Even though this patch does not does not 
do that, it does lay the ground work for that type of functionality.

Comments?

steved.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Patch 1/9] Make idmapd's Configuration Parsing Code Available
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
@ 2009-03-09 20:47 ` Steve Dickson
  2009-03-09 20:50 ` [Patch 2/9] Ignore blanks in section definitions and before assignment statements Steve Dickson
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 20:47 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit e550784ae2a96f5cd448c00d3c17669fa519e1ef
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 13:52:53 2009 -0400

    Move idmapd's configuration file parsing routines into
    the shared libnfs.a library, making them available to
    other daemons and programs.
    =

    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/support/include/Makefile.am b/support/include/Makefile.am
index 718abda..f5a77ec 100644
--- a/support/include/Makefile.am
+++ b/support/include/Makefile.am
@@ -14,6 +14,7 @@ noinst_HEADERS =3D \
 	xio.h \
 	xlog.h \
 	xmalloc.h \
-	xcommon.h
+	xcommon.h \
+	conffile.h
 =

 MAINTAINERCLEANFILES =3D Makefile.in
diff --git a/support/include/conffile.h b/support/include/conffile.h
new file mode 100644
index 0000000..3309788
--- /dev/null
+++ b/support/include/conffile.h
@@ -0,0 +1,67 @@
+/* $OpenBSD: conf.h,v 1.30 2004/06/25 20:25:34 hshoexer Exp $	 */
+/* $EOM: conf.h,v 1.13 2000/09/18 00:01:47 ho Exp $	 */
+
+/*
+ * Copyright (c) 1998, 1999, 2001 Niklas Hallqvist.  All rights reserved.
+ * Copyright (c) 2000, 2003 H=E5kan Olsson.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTI=
ES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF US=
E,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * This code was written under funding by Ericsson Radio Systems.
+ */
+
+#ifndef _CONFFILE_H_
+#define _CONFFILE_H_
+
+#include <sys/queue.h>
+
+struct conf_list_node {
+	TAILQ_ENTRY(conf_list_node) link;
+	char	*field;
+};
+
+struct conf_list {
+	size_t	cnt;
+	TAILQ_HEAD(conf_list_fields_head, conf_list_node) fields;
+};
+
+extern char    *conf_path;
+
+extern int      conf_begin(void);
+extern int      conf_decode_base64(u_int8_t *, u_int32_t *, u_char *);
+extern int      conf_end(int, int);
+extern void     conf_free_list(struct conf_list *);
+extern struct sockaddr *conf_get_address(char *, char *);
+extern struct conf_list *conf_get_list(char *, char *);
+extern struct conf_list *conf_get_tag_list(char *);
+extern int      conf_get_num(char *, char *, int);
+extern char    *conf_get_str(char *, char *);
+extern void     conf_init(void);
+extern int      conf_match_num(char *, char *, int);
+extern void     conf_reinit(void);
+extern int      conf_remove(int, char *, char *);
+extern int      conf_remove_section(int, char *);
+extern int      conf_set(int, char *, char *, char *, int, int);
+extern void     conf_report(void);
+
+#endif				/* _CONFFILE_H_ */
diff --git a/support/nfs/Makefile.am b/support/nfs/Makefile.am
index 86f52a1..63e1800 100644
--- a/support/nfs/Makefile.am
+++ b/support/nfs/Makefile.am
@@ -4,7 +4,7 @@ noinst_LIBRARIES =3D libnfs.a
 libnfs_a_SOURCES =3D exports.c rmtab.c xio.c rpcmisc.c rpcdispatch.c \
 		   xlog.c xcommon.c wildmat.c nfssvc.c nfsclient.c \
 		   nfsexport.c getfh.c nfsctl.c rpc_socket.c getport.c \
-		   svc_socket.c cacheio.c closeall.c nfs_mntent.c
+		   svc_socket.c cacheio.c closeall.c nfs_mntent.c conffile.c
 =

 MAINTAINERCLEANFILES =3D Makefile.in
 =

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
new file mode 100644
index 0000000..754bed9
--- /dev/null
+++ b/support/nfs/conffile.c
@@ -0,0 +1,850 @@
+/*	$OpenBSD: conf.c,v 1.55 2003/06/03 14:28:16 ho Exp $	*/
+/*	$EOM: conf.c,v 1.48 2000/12/04 02:04:29 angelos Exp $	*/
+
+/*
+ * Copyright (c) 1998, 1999, 2000, 2001 Niklas Hallqvist.  All rights rese=
rved.
+ * Copyright (c) 2000, 2001, 2002 H=E5kan Olsson.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
+ * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTI=
ES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF US=
E,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * This code was written under funding by Ericsson Radio Systems.
+ */
+
+#include <sys/param.h>
+#include <sys/mman.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <ctype.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+#include <err.h>
+#include <syslog.h>
+
+#include "conffile.h"
+#include "xlog.h"
+
+static void conf_load_defaults (int);
+#if 0
+static int conf_find_trans_xf (int, char *);
+#endif
+
+size_t  strlcpy(char *, const char *, size_t);
+
+struct conf_trans {
+	TAILQ_ENTRY (conf_trans) link;
+	int trans;
+	enum conf_op { CONF_SET, CONF_REMOVE, CONF_REMOVE_SECTION } op;
+	char *section;
+	char *tag;
+	char *value;
+	int override;
+	int is_default;
+};
+
+TAILQ_HEAD (conf_trans_head, conf_trans) conf_trans_queue;
+
+/*
+ * Radix-64 Encoding.
+ */
+static const u_int8_t bin2asc[]
+  =3D "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+
+static const u_int8_t asc2bin[] =3D
+{
+  255, 255, 255, 255, 255, 255, 255, 255,
+  255, 255, 255, 255, 255, 255, 255, 255,
+  255, 255, 255, 255, 255, 255, 255, 255,
+  255, 255, 255, 255, 255, 255, 255, 255,
+  255, 255, 255, 255, 255, 255, 255, 255,
+  255, 255, 255,  62, 255, 255, 255,  63,
+   52,  53,  54,  55,  56,  57,  58,  59,
+   60,  61, 255, 255, 255, 255, 255, 255,
+  255,   0,   1,   2,   3,   4,   5,   6,
+    7,   8,   9,  10,  11,  12,  13,  14,
+   15,  16,  17,  18,  19,  20,  21,  22,
+   23,  24,  25, 255, 255, 255, 255, 255,
+  255,  26,  27,  28,  29,  30,  31,  32,
+   33,  34,  35,  36,  37,  38,  39,  40,
+   41,  42,  43,  44,  45,  46,  47,  48,
+   49,  50,  51, 255, 255, 255, 255, 255
+};
+
+struct conf_binding {
+  LIST_ENTRY (conf_binding) link;
+  char *section;
+  char *tag;
+  char *value;
+  int is_default;
+};
+
+char *conf_path;
+LIST_HEAD (conf_bindings, conf_binding) conf_bindings[256];
+
+static char *conf_addr;
+
+static __inline__ u_int8_t
+conf_hash(char *s)
+{
+	u_int8_t hash =3D 0;
+
+	while (*s) {
+		hash =3D ((hash << 1) | (hash >> 7)) ^ tolower (*s);
+		s++;
+	}
+	return hash;
+}
+
+/*
+ * Insert a tag-value combination from LINE (the equal sign is at POS)
+ */
+static int
+conf_remove_now(char *section, char *tag)
+{
+	struct conf_binding *cb, *next;
+
+	cb =3D LIST_FIRST(&conf_bindings[conf_hash (section)]);
+	for (; cb; cb =3D next) {
+		next =3D LIST_NEXT(cb, link);
+		if (strcasecmp(cb->section, section) =3D=3D 0
+				&& strcasecmp(cb->tag, tag) =3D=3D 0) {
+			LIST_REMOVE(cb, link);
+			xlog(LOG_INFO,"[%s]:%s->%s removed", section, tag, cb->value);
+			free(cb->section);
+			free(cb->tag);
+			free(cb->value);
+			free(cb);
+			return 0;
+		}
+	}
+	return 1;
+}
+
+static int
+conf_remove_section_now(char *section)
+{
+  struct conf_binding *cb, *next;
+  int unseen =3D 1;
+
+	cb =3D LIST_FIRST(&conf_bindings[conf_hash (section)]);
+	for (; cb; cb =3D next) {
+		next =3D LIST_NEXT(cb, link);
+		if (strcasecmp(cb->section, section) =3D=3D 0) {
+			unseen =3D 0;
+			LIST_REMOVE(cb, link);
+			xlog(LOG_INFO, "[%s]:%s->%s removed", section, cb->tag, cb->value);
+			free(cb->section);
+			free(cb->tag);
+			free(cb->value);
+			free(cb);
+			}
+		}
+	return unseen;
+}
+
+/*
+ * Insert a tag-value combination from LINE (the equal sign is at POS)
+ * into SECTION of our configuration database.
+ */
+static int
+conf_set_now(char *section, char *tag, char *value, int override,
+	      int is_default)
+{
+	struct conf_binding *node =3D 0;
+
+	if (override)
+		conf_remove_now(section, tag);
+	else if (conf_get_str(section, tag)) {
+		if (!is_default) {
+			xlog(LOG_INFO, "conf_set: duplicate tag [%s]:%s, ignoring...\n", =

+				section, tag);
+		}
+		return 1;
+	}
+
+	node =3D calloc(1, sizeof *node);
+	if (!node) {
+		xlog_warn("conf_set: calloc (1, %lu) failed", (unsigned long)sizeof *nod=
e);
+		return 1;
+	}
+	node->section =3D strdup(section);
+	node->tag =3D strdup(tag);
+	node->value =3D strdup(value);
+	node->is_default =3D is_default;
+
+	LIST_INSERT_HEAD(&conf_bindings[conf_hash (section)], node, link);
+	return 0;
+}
+
+/*
+ * 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, size_t sz)
+{
+	char *val;
+	size_t i;
+	int j;
+	static char *section =3D 0;
+	static int ln =3D 0;
+
+	/* Lines starting with '#' or ';' are comments.  */
+	ln++;
+	if (*line =3D=3D '#' || *line =3D=3D ';')
+		return;
+
+	/* '[section]' parsing...  */
+	if (*line =3D=3D '[') {
+		for (i =3D 1; i < sz; i++)
+			if (line[i] =3D=3D ']')
+				break;
+		if (section)
+			free (section);
+		if (i =3D=3D sz) {
+			xlog_warn("conf_parse_line: %d:"
+ 				"non-matched ']', ignoring until next section", ln);
+			section =3D 0;
+			return;
+		}
+		section =3D malloc(i);
+		if (!section) {
+			xlog_warn("conf_parse_line: %d: malloc (%lu) failed", ln,
+						(unsigned long)i);
+			return;
+		}
+		strlcpy(section, line + 1, i);
+		return;
+	}
+
+	/* Deal with assignments.  */
+	for (i =3D 0; i < sz; i++) {
+		if (line[i] =3D=3D '=3D') {
+			/* If no section, we are ignoring the lines.  */
+			if (!section) {
+				xlog_warn("conf_parse_line: %d: ignoring line due to no section", =

+					ln);
+				return;
+			}
+			line[strcspn (line, " \t=3D")] =3D '\0';
+			val =3D line + i + 1 + strspn (line + i + 1, " \t");
+			/* Skip trailing whitespace, if any */
+			for (j =3D sz - (val - line) - 1; j > 0 && isspace(val[j]); j--)
+				val[j] =3D '\0';
+			/* XXX Perhaps should we not ignore errors?  */
+			conf_set(trans, section, line, val, 0, 0);
+			return;
+		}
+	}
+	/* Other non-empty lines are weird.  */
+	i =3D strspn(line, " \t");
+	if (line[i])
+		xlog_warn("conf_parse_line: %d: syntax error", ln);
+
+	return;
+}
+
+/* Parse the mapped configuration file.  */
+static void
+conf_parse(int trans, char *buf, size_t sz)
+{
+	char *cp =3D buf;
+	char *bufend =3D buf + sz;
+	char *line;
+
+	line =3D cp;
+	while (cp < bufend) {
+		if (*cp =3D=3D '\n') {
+			/* Check for escaped newlines.  */
+			if (cp > buf && *(cp - 1) =3D=3D '\\')
+				*(cp - 1) =3D *cp =3D ' ';
+			else {
+				*cp =3D '\0';
+				conf_parse_line(trans, line, cp - line);
+				line =3D cp + 1;
+			}
+		}
+		cp++;
+	}
+	if (cp !=3D line)
+		xlog_warn("conf_parse: last line non-terminated, ignored.");
+}
+
+static void
+conf_load_defaults(int tr)
+{
+	/* No defaults */
+	return;
+}
+
+void
+conf_init (void)
+{
+	unsigned int i;
+
+	for (i =3D 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++)
+		LIST_INIT (&conf_bindings[i]);
+
+	TAILQ_INIT (&conf_trans_queue);
+	conf_reinit();
+}
+
+/* Open the config file and map it into our address space, then parse it. =
 */
+void
+conf_reinit(void)
+{
+	struct conf_binding *cb =3D 0;
+	int fd, trans;
+	unsigned int i;
+	size_t sz;
+	char *new_conf_addr =3D 0;
+	struct stat sb;
+
+	if ((stat (conf_path, &sb) =3D=3D 0) || (errno !=3D ENOENT)) {
+		sz =3D sb.st_size;
+		fd =3D open (conf_path, O_RDONLY, 0);
+		if (fd =3D=3D -1) {
+			xlog_warn("conf_reinit: open (\"%s\", O_RDONLY) failed", conf_path);
+			return;
+		}
+
+		new_conf_addr =3D malloc(sz);
+		if (!new_conf_addr) {
+			xlog_warn("conf_reinit: malloc (%lu) failed", (unsigned long)sz);
+			goto fail;
+		}
+
+		/* XXX I assume short reads won't happen here.  */
+		if (read (fd, new_conf_addr, sz) !=3D (int)sz) {
+			xlog_warn("conf_reinit: read (%d, %p, %lu) failed",
+   				fd, new_conf_addr, (unsigned long)sz);
+			goto fail;
+		}
+		close(fd);
+
+		trans =3D conf_begin();
+		/* XXX Should we not care about errors and rollback?  */
+		conf_parse(trans, new_conf_addr, sz);
+	}
+	else
+		trans =3D conf_begin();
+
+	/* Load default configuration values.  */
+	conf_load_defaults(trans);
+
+	/* Free potential existing configuration.  */
+	if (conf_addr) {
+		for (i =3D 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++) {
+			cb =3D LIST_FIRST (&conf_bindings[i]);
+			for (; cb; cb =3D LIST_FIRST (&conf_bindings[i]))
+				conf_remove_now(cb->section, cb->tag);
+		}
+		free (conf_addr);
+	}
+
+	conf_end(trans, 1);
+	conf_addr =3D new_conf_addr;
+	return;
+
+fail:
+	if (new_conf_addr)
+		free(new_conf_addr);
+	close (fd);
+}
+
+/*
+ * Return the numeric value denoted by TAG in section SECTION or DEF
+ * if that tag does not exist.
+ */
+int
+conf_get_num(char *section, char *tag, int def)
+{
+	char *value =3D conf_get_str(section, tag);
+
+	if (value)
+		return atoi(value);
+
+	return def;
+}
+
+/* Validate X according to the range denoted by TAG in section SECTION.  */
+int
+conf_match_num(char *section, char *tag, int x)
+{
+	char *value =3D conf_get_str (section, tag);
+	int val, min, max, n;
+
+	if (!value)
+		return 0;
+	n =3D sscanf (value, "%d,%d:%d", &val, &min, &max);
+	switch (n) {
+	case 1:
+		xlog(LOG_INFO, "conf_match_num: %s:%s %d=3D=3D%d?", section, tag, val, x=
);
+		return x =3D=3D val;
+	case 3:
+		xlog(LOG_INFO, "conf_match_num: %s:%s %d<=3D%d<=3D%d?", section, =

+			tag, min, x, max);
+		return min <=3D x && max >=3D x;
+	default:
+		xlog(LOG_INFO, "conf_match_num: section %s tag %s: invalid number spec %=
s",
+			section, tag, value);
+	}
+	return 0;
+}
+
+/* Return the string value denoted by TAG in section SECTION.  */
+char *
+conf_get_str(char *section, char *tag)
+{
+	struct conf_binding *cb;
+
+	cb =3D LIST_FIRST (&conf_bindings[conf_hash (section)]);
+	for (; cb; cb =3D LIST_NEXT (cb, link)) {
+		if (strcasecmp (section, cb->section) =3D=3D 0
+				&& strcasecmp (tag, cb->tag) =3D=3D 0)
+			return cb->value;
+	}
+	return 0;
+}
+
+/*
+ * Build a list of string values out of the comma separated value denoted =
by
+ * TAG in SECTION.
+ */
+struct conf_list *
+conf_get_list(char *section, char *tag)
+{
+	char *liststr =3D 0, *p, *field, *t;
+	struct conf_list *list =3D 0;
+	struct conf_list_node *node;
+
+	list =3D malloc (sizeof *list);
+	if (!list)
+		goto cleanup;
+	TAILQ_INIT (&list->fields);
+	list->cnt =3D 0;
+	liststr =3D conf_get_str(section, tag);
+	if (!liststr)
+		goto cleanup;
+	liststr =3D strdup (liststr);
+	if (!liststr)
+		goto cleanup;
+	p =3D liststr;
+	while ((field =3D strsep (&p, ",")) !=3D NULL) {
+		/* Skip leading whitespace */
+		while (isspace (*field))
+			field++;
+		/* Skip trailing whitespace */
+		if (p) {
+			for (t =3D p - 1; t > field && isspace (*t); t--)
+				*t =3D '\0';
+		}
+		if (*field =3D=3D '\0') {
+			xlog(LOG_INFO, "conf_get_list: empty field, ignoring...");
+			continue;
+		}
+		list->cnt++;
+		node =3D calloc (1, sizeof *node);
+		if (!node)
+			goto cleanup;
+		node->field =3D strdup (field);
+		if (!node->field) {
+			free(node);
+			goto cleanup;
+		}
+		TAILQ_INSERT_TAIL (&list->fields, node, link);
+	}
+	free (liststr);
+	return list;
+
+cleanup:
+	if (list)
+		conf_free_list(list);
+	if (liststr)
+		free(liststr);
+	return 0;
+}
+
+struct conf_list *
+conf_get_tag_list (char *section)
+{
+	struct conf_list *list =3D 0;
+	struct conf_list_node *node;
+	struct conf_binding *cb;
+
+	list =3D malloc(sizeof *list);
+	if (!list)
+		goto cleanup;
+	TAILQ_INIT(&list->fields);
+	list->cnt =3D 0;
+	cb =3D LIST_FIRST(&conf_bindings[conf_hash (section)]);
+	for (; cb; cb =3D LIST_NEXT(cb, link)) {
+		if (strcasecmp (section, cb->section) =3D=3D 0) {
+			list->cnt++;
+			node =3D calloc (1, sizeof *node);
+			if (!node)
+				goto cleanup;
+			node->field =3D strdup(cb->tag);
+			if (!node->field) {
+				free(node);
+				goto cleanup;
+			}
+			TAILQ_INSERT_TAIL(&list->fields, node, link);
+		}
+	}
+	return list;
+
+cleanup:
+	if (list)
+		conf_free_list(list);
+	return 0;
+}
+
+/* Decode a PEM encoded buffer.  */
+int
+conf_decode_base64 (u_int8_t *out, u_int32_t *len, u_char *buf)
+{
+	u_int32_t c =3D 0;
+	u_int8_t c1, c2, c3, c4;
+
+	while (*buf) {
+		if (*buf > 127 || (c1 =3D asc2bin[*buf]) =3D=3D 255)
+			return 0;
+
+		buf++;
+		if (*buf > 127 || (c2 =3D asc2bin[*buf]) =3D=3D 255)
+			return 0;
+
+		buf++;
+		if (*buf =3D=3D '=3D') {
+			c3 =3D c4 =3D 0;
+			c++;
+
+			/* Check last four bit */
+			if (c2 & 0xF)
+				return 0;
+
+			if (strcmp((char *)buf, "=3D=3D") =3D=3D 0)
+				buf++;
+			else
+				return 0;
+		} else if (*buf > 127 || (c3 =3D asc2bin[*buf]) =3D=3D 255)
+			return 0;
+		else {
+			if (*++buf =3D=3D '=3D') {
+				c4 =3D 0;
+				c +=3D 2;
+
+				/* Check last two bit */
+				if (c3 & 3)
+					return 0;
+
+			if (strcmp((char *)buf, "=3D"))
+				return 0;
+			} else if (*buf > 127 || (c4 =3D asc2bin[*buf]) =3D=3D 255)
+				return 0;
+			else
+				c +=3D 3;
+		}
+
+		buf++;
+		*out++ =3D (c1 << 2) | (c2 >> 4);
+		*out++ =3D (c2 << 4) | (c3 >> 2);
+		*out++ =3D (c3 << 6) | c4;
+	}
+
+	*len =3D c;
+	return 1;
+}
+
+void
+conf_free_list(struct conf_list *list)
+{
+	struct conf_list_node *node =3D TAILQ_FIRST(&list->fields);
+
+	while (node) {
+		TAILQ_REMOVE(&list->fields, node, link);
+		if (node->field)
+			free(node->field);
+		free (node);
+		node =3D TAILQ_FIRST(&list->fields);
+	}
+	free (list);
+}
+
+int
+conf_begin (void)
+{
+  static int seq =3D 0;
+
+  return ++seq;
+}
+
+static struct conf_trans *
+conf_trans_node (int transaction, enum conf_op op)
+{
+	struct conf_trans *node;
+
+	node =3D calloc (1, sizeof *node);
+	if (!node) {
+		xlog_warn("conf_trans_node: calloc (1, %lu) failed",
+		(unsigned long)sizeof *node);
+		return 0;
+	}
+	node->trans =3D transaction;
+	node->op =3D op;
+	TAILQ_INSERT_TAIL (&conf_trans_queue, node, link);
+	return node;
+}
+
+/* Queue a set operation.  */
+int
+conf_set (int transaction, char *section, char *tag, =

+	char *value, int override, int is_default)
+{
+	struct conf_trans *node;
+
+	node =3D conf_trans_node(transaction, CONF_SET);
+	if (!node)
+		return 1;
+	node->section =3D strdup(section);
+	if (!node->section) {
+		xlog_warn("conf_set: strdup(\"%s\") failed", section);
+		goto fail;
+	}
+	node->tag =3D strdup(tag);
+	if (!node->tag) {
+		xlog_warn("conf_set: strdup(\"%s\") failed", tag);
+		goto fail;
+	}
+	node->value =3D strdup(value);
+	if (!node->value) {
+		xlog_warn("conf_set: strdup(\"%s\") failed", value);
+		goto fail;
+	}
+	node->override =3D override;
+	node->is_default =3D is_default;
+	return 0;
+
+fail:
+	if (node->tag)
+		free(node->tag);
+	if (node->section)
+		free(node->section);
+	if (node)
+		free(node);
+	return 1;
+}
+
+/* Queue a remove operation.  */
+int
+conf_remove(int transaction, char *section, char *tag)
+{
+	struct conf_trans *node;
+
+	node =3D conf_trans_node(transaction, CONF_REMOVE);
+	if (!node)
+		goto fail;
+	node->section =3D strdup(section);
+	if (!node->section) {
+		xlog_warn("conf_remove: strdup(\"%s\") failed", section);
+		goto fail;
+	}
+	node->tag =3D strdup(tag);
+	if (!node->tag) {
+		xlog_warn("conf_remove: strdup(\"%s\") failed", tag);
+		goto fail;
+	}
+	return 0;
+
+fail:
+	if (node && node->section)
+		free (node->section);
+	if (node)
+		free (node);
+	return 1;
+}
+
+/* Queue a remove section operation.  */
+int
+conf_remove_section(int transaction, char *section)
+{
+	struct conf_trans *node;
+
+	node =3D conf_trans_node(transaction, CONF_REMOVE_SECTION);
+	if (!node)
+		goto fail;
+	node->section =3D strdup(section);
+	if (!node->section) {
+		xlog_warn("conf_remove_section: strdup(\"%s\") failed", section);
+		goto fail;
+	}
+	return 0;
+
+fail:
+	if (node)
+		free(node);
+	return 1;
+}
+
+/* Execute all queued operations for this transaction.  Cleanup.  */
+int
+conf_end(int transaction, int commit)
+{
+	struct conf_trans *node, *next;
+
+	for (node =3D TAILQ_FIRST(&conf_trans_queue); node; node =3D next) {
+		next =3D TAILQ_NEXT(node, link);
+		if (node->trans =3D=3D transaction) {
+			if (commit) {
+				switch (node->op) {
+				case CONF_SET:
+					conf_set_now(node->section, node->tag, node->value,
+					node->override, node->is_default);
+					break;
+				case CONF_REMOVE:
+					conf_remove_now(node->section, node->tag);
+					break;
+				case CONF_REMOVE_SECTION:
+					conf_remove_section_now(node->section);
+					break;
+				default:
+					xlog(LOG_INFO, "conf_end: unknown operation: %d", node->op);
+				}
+			}
+			TAILQ_REMOVE (&conf_trans_queue, node, link);
+			if (node->section)
+				free(node->section);
+			if (node->tag)
+				free(node->tag);
+			if (node->value)
+				free(node->value);
+			free (node);
+		}
+	}
+	return 0;
+}
+
+/*
+ * Dump running configuration upon SIGUSR1.
+ * Configuration is "stored in reverse order", so reverse it again.
+ */
+struct dumper {
+	char *s, *v;
+	struct dumper *next;
+};
+
+static void
+conf_report_dump(struct dumper *node)
+{
+	/* Recursive, cleanup when we're done.  */
+	if (node->next)
+		conf_report_dump(node->next);
+
+	if (node->v)
+		xlog(LOG_INFO, "%s=3D\t%s", node->s, node->v);
+	else if (node->s) {
+		xlog(LOG_INFO, "%s", node->s);
+		if (strlen(node->s) > 0)
+			free(node->s);
+	}
+
+	free (node);
+}
+
+void
+conf_report (void)
+{
+	struct conf_binding *cb, *last =3D 0;
+	unsigned int i, len;
+	char *current_section =3D (char *)0;
+	struct dumper *dumper, *dnode;
+
+	dumper =3D dnode =3D (struct dumper *)calloc(1, sizeof *dumper);
+	if (!dumper)
+		goto mem_fail;
+
+	xlog(LOG_INFO, "conf_report: dumping running configuration");
+
+	for (i =3D 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++)
+		for (cb =3D LIST_FIRST(&conf_bindings[i]); cb; cb =3D LIST_NEXT(cb, link=
)) {
+			if (!cb->is_default) {
+				/* Dump this entry.  */
+				if (!current_section || strcmp(cb->section, current_section)) {
+					if (current_section) {
+						len =3D strlen (current_section) + 3;
+						dnode->s =3D malloc(len);
+						if (!dnode->s)
+							goto mem_fail;
+
+						snprintf(dnode->s, len, "[%s]", current_section);
+						dnode->next =3D =

+							(struct dumper *)calloc(1, sizeof (struct dumper));
+						dnode =3D dnode->next;
+						if (!dnode)
+							goto mem_fail;
+
+						dnode->s =3D "";
+						dnode->next =3D =

+							(struct dumper *)calloc(1, sizeof (struct dumper));
+						dnode =3D dnode->next;
+						if (!dnode)
+						goto mem_fail;
+					}
+					current_section =3D cb->section;
+				}
+				dnode->s =3D cb->tag;
+				dnode->v =3D cb->value;
+				dnode->next =3D (struct dumper *)calloc (1, sizeof (struct dumper));
+				dnode =3D dnode->next;
+				if (!dnode)
+					goto mem_fail;
+				last =3D cb;
+		}
+	}
+
+	if (last) {
+		len =3D strlen(last->section) + 3;
+		dnode->s =3D malloc(len);
+		if (!dnode->s)
+			goto mem_fail;
+		snprintf(dnode->s, len, "[%s]", last->section);
+	}
+	conf_report_dump(dumper);
+	return;
+
+mem_fail:
+	xlog_warn("conf_report: malloc/calloc failed");
+	while ((dnode =3D dumper) !=3D 0) {
+		dumper =3D dumper->next;
+		if (dnode->s)
+			free(dnode->s);
+		free(dnode);
+	}
+	return;
+}
diff --git a/utils/idmapd/Makefile.am b/utils/idmapd/Makefile.am
index eb393df..4dabb3d 100644
--- a/utils/idmapd/Makefile.am
+++ b/utils/idmapd/Makefile.am
@@ -14,7 +14,6 @@ EXTRA_DIST =3D \
 =

 idmapd_SOURCES =3D \
 	atomicio.c \
-	cfg.c \
 	idmapd.c \
 	strlcat.c \
 	strlcpy.c \
diff --git a/utils/idmapd/cfg.c b/utils/idmapd/cfg.c
deleted file mode 100644
index 16d392a..0000000
--- a/utils/idmapd/cfg.c
+++ /dev/null
@@ -1,893 +0,0 @@
-/*	$OpenBSD: conf.c,v 1.55 2003/06/03 14:28:16 ho Exp $	*/
-/*	$EOM: conf.c,v 1.48 2000/12/04 02:04:29 angelos Exp $	*/
-
-/*
- * Copyright (c) 1998, 1999, 2000, 2001 Niklas Hallqvist.  All rights rese=
rved.
- * Copyright (c) 2000, 2001, 2002 H=E5kan Olsson.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTI=
ES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF US=
E,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/*
- * This code was written under funding by Ericsson Radio Systems.
- */
-
-#include <sys/param.h>
-#include <sys/mman.h>
-#include <sys/socket.h>
-#include <sys/stat.h>
-#include <netinet/in.h>
-#include <arpa/inet.h>
-#include <ctype.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include <errno.h>
-#include <err.h>
-
-#include "cfg.h"
-
-static void conf_load_defaults (int);
-#if 0
-static int conf_find_trans_xf (int, char *);
-#endif
-
-size_t  strlcpy(char *, const char *, size_t);
-
-struct conf_trans {
-  TAILQ_ENTRY (conf_trans) link;
-  int trans;
-  enum conf_op { CONF_SET, CONF_REMOVE, CONF_REMOVE_SECTION } op;
-  char *section;
-  char *tag;
-  char *value;
-  int override;
-  int is_default;
-};
-
-TAILQ_HEAD (conf_trans_head, conf_trans) conf_trans_queue;
-
-/*
- * Radix-64 Encoding.
- */
-const u_int8_t bin2asc[]
-  =3D "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
-
-const u_int8_t asc2bin[] =3D
-{
-  255, 255, 255, 255, 255, 255, 255, 255,
-  255, 255, 255, 255, 255, 255, 255, 255,
-  255, 255, 255, 255, 255, 255, 255, 255,
-  255, 255, 255, 255, 255, 255, 255, 255,
-  255, 255, 255, 255, 255, 255, 255, 255,
-  255, 255, 255,  62, 255, 255, 255,  63,
-   52,  53,  54,  55,  56,  57,  58,  59,
-   60,  61, 255, 255, 255, 255, 255, 255,
-  255,   0,   1,   2,   3,   4,   5,   6,
-    7,   8,   9,  10,  11,  12,  13,  14,
-   15,  16,  17,  18,  19,  20,  21,  22,
-   23,  24,  25, 255, 255, 255, 255, 255,
-  255,  26,  27,  28,  29,  30,  31,  32,
-   33,  34,  35,  36,  37,  38,  39,  40,
-   41,  42,  43,  44,  45,  46,  47,  48,
-   49,  50,  51, 255, 255, 255, 255, 255
-};
-
-struct conf_binding {
-  LIST_ENTRY (conf_binding) link;
-  char *section;
-  char *tag;
-  char *value;
-  int is_default;
-};
-
-char *conf_path;
-LIST_HEAD (conf_bindings, conf_binding) conf_bindings[256];
-
-static char *conf_addr;
-
-static __inline__ u_int8_t
-conf_hash (char *s)
-{
-  u_int8_t hash =3D 0;
-
-  while (*s)
-    {
-      hash =3D ((hash << 1) | (hash >> 7)) ^ tolower (*s);
-      s++;
-    }
-  return hash;
-}
-
-/*
- * Insert a tag-value combination from LINE (the equal sign is at POS)
- */
-static int
-conf_remove_now (char *section, char *tag)
-{
-  struct conf_binding *cb, *next;
-
-  for (cb =3D LIST_FIRST (&conf_bindings[conf_hash (section)]); cb; cb =3D=
 next)
-    {
-      next =3D LIST_NEXT (cb, link);
-      if (strcasecmp (cb->section, section) =3D=3D 0
-	  && strcasecmp (cb->tag, tag) =3D=3D 0)
-	{
-	  LIST_REMOVE (cb, link);
-	  warnx("[%s]:%s->%s removed", section, tag, cb->value);
-	  free (cb->section);
-	  free (cb->tag);
-	  free (cb->value);
-	  free (cb);
-	  return 0;
-	}
-    }
-  return 1;
-}
-
-static int
-conf_remove_section_now (char *section)
-{
-  struct conf_binding *cb, *next;
-  int unseen =3D 1;
-
-  for (cb =3D LIST_FIRST (&conf_bindings[conf_hash (section)]); cb; cb =3D=
 next)
-    {
-      next =3D LIST_NEXT (cb, link);
-      if (strcasecmp (cb->section, section) =3D=3D 0)
-	{
-	  unseen =3D 0;
-	  LIST_REMOVE (cb, link);
-	  warnx("[%s]:%s->%s removed", section, cb->tag, cb->value);
-	  free (cb->section);
-	  free (cb->tag);
-	  free (cb->value);
-	  free (cb);
-	}
-    }
-  return unseen;
-}
-
-/*
- * Insert a tag-value combination from LINE (the equal sign is at POS)
- * into SECTION of our configuration database.
- */
-static int
-conf_set_now (char *section, char *tag, char *value, int override,
-	      int is_default)
-{
-  struct conf_binding *node =3D 0;
-
-  if (override)
-    conf_remove_now (section, tag);
-  else if (conf_get_str (section, tag))
-    {
-      if (!is_default)
-	warnx("conf_set: duplicate tag [%s]:%s, ignoring...\n", section, tag);
-      return 1;
-    }
-
-  node =3D calloc (1, sizeof *node);
-  if (!node)
-    {
-      warnx("conf_set: calloc (1, %lu) failed", (unsigned long)sizeof *nod=
e);
-      return 1;
-    }
-  node->section =3D strdup (section);
-  node->tag =3D strdup (tag);
-  node->value =3D strdup (value);
-  node->is_default =3D is_default;
-
-  LIST_INSERT_HEAD (&conf_bindings[conf_hash (section)], node, link);
-  return 0;
-}
-
-/*
- * 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, size_t sz)
-{
-  char *val;
-  size_t i;
-  int j;
-  static char *section =3D 0;
-  static int ln =3D 0;
-
-  ln++;
-
-  /* Lines starting with '#' or ';' are comments.  */
-  if (*line =3D=3D '#' || *line =3D=3D ';')
-    return;
-
-  /* '[section]' parsing...  */
-  if (*line =3D=3D '[')
-    {
-      for (i =3D 1; i < sz; i++)
-	if (line[i] =3D=3D ']')
-	  break;
-      if (section)
-	free (section);
-      if (i =3D=3D sz)
-	{
-	  warnx("conf_parse_line: %d:"
-		     "non-matched ']', ignoring until next section", ln);
-	  section =3D 0;
-	  return;
-	}
-      section =3D malloc (i);
-      if (!section)
-	{
-	  warnx("conf_parse_line: %d: malloc (%lu) failed", ln,
-		(unsigned long)i);
-	  return;
-	}
-      strlcpy (section, line + 1, i);
-      return;
-    }
-
-  /* Deal with assignments.  */
-  for (i =3D 0; i < sz; i++)
-    if (line[i] =3D=3D '=3D')
-      {
-	/* If no section, we are ignoring the lines.  */
-	if (!section)
-	  {
-	    warnx("conf_parse_line: %d: ignoring line due to no section", ln);
-	    return;
-	  }
-	line[strcspn (line, " \t=3D")] =3D '\0';
-	val =3D line + i + 1 + strspn (line + i + 1, " \t");
-	/* Skip trailing whitespace, if any */
-	for (j =3D sz - (val - line) - 1; j > 0 && isspace (val[j]); j--)
-	  val[j] =3D '\0';
-	/* XXX Perhaps should we not ignore errors?  */
-	conf_set (trans, section, line, val, 0, 0);
-	return;
-      }
-
-  /* Other non-empty lines are weird.  */
-  i =3D strspn (line, " \t");
-  if (line[i])
-    warnx("conf_parse_line: %d: syntax error", ln);
-
-  return;
-}
-
-/* Parse the mapped configuration file.  */
-static void
-conf_parse (int trans, char *buf, size_t sz)
-{
-  char *cp =3D buf;
-  char *bufend =3D buf + sz;
-  char *line;
-
-  line =3D cp;
-  while (cp < bufend)
-    {
-      if (*cp =3D=3D '\n')
-	{
-	  /* Check for escaped newlines.  */
-	  if (cp > buf && *(cp - 1) =3D=3D '\\')
-	    *(cp - 1) =3D *cp =3D ' ';
-	  else
-	    {
-	      *cp =3D '\0';
-	      conf_parse_line (trans, line, cp - line);
-	      line =3D cp + 1;
-	    }
-	}
-      cp++;
-    }
-  if (cp !=3D line)
-    warnx("conf_parse: last line non-terminated, ignored.");
-}
-
-static void
-conf_load_defaults (int tr)
-{
-	/* No defaults */
-	return;
-}
-
-void
-conf_init (void)
-{
-  unsigned int i;
-
-  for (i =3D 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++)
-    LIST_INIT (&conf_bindings[i]);
-  TAILQ_INIT (&conf_trans_queue);
-  conf_reinit ();
-}
-
-/* Open the config file and map it into our address space, then parse it. =
 */
-void
-conf_reinit (void)
-{
-  struct conf_binding *cb =3D 0;
-  int fd, trans;
-  unsigned int i;
-  size_t sz;
-  char *new_conf_addr =3D 0;
-  struct stat sb;
-
-  if ((stat (conf_path, &sb) =3D=3D 0) || (errno !=3D ENOENT))
-    {
-      sz =3D sb.st_size;
-      fd =3D open (conf_path, O_RDONLY, 0);
-      if (fd =3D=3D -1)
-        {
-	  warnx("conf_reinit: open (\"%s\", O_RDONLY) failed", conf_path);
-	  return;
-	}
-
-      new_conf_addr =3D malloc (sz);
-      if (!new_conf_addr)
-        {
-	  warnx("conf_reinit: malloc (%lu) failed", (unsigned long)sz);
-	  goto fail;
-	}
-
-      /* XXX I assume short reads won't happen here.  */
-      if (read (fd, new_conf_addr, sz) !=3D (int)sz)
-        {
-	    warnx("conf_reinit: read (%d, %p, %lu) failed",
-		       fd, new_conf_addr, (unsigned long)sz);
-	    goto fail;
-	}
-      close (fd);
-
-      trans =3D conf_begin ();
-
-      /* XXX Should we not care about errors and rollback?  */
-      conf_parse (trans, new_conf_addr, sz);
-    }
-  else
-    trans =3D conf_begin ();
-
-  /* Load default configuration values.  */
-  conf_load_defaults (trans);
-
-  /* Free potential existing configuration.  */
-  if (conf_addr)
-    {
-      for (i =3D 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i+=
+)
-	for (cb =3D LIST_FIRST (&conf_bindings[i]); cb;
-	     cb =3D LIST_FIRST (&conf_bindings[i]))
-	  conf_remove_now (cb->section, cb->tag);
-      free (conf_addr);
-    }
-
-  conf_end (trans, 1);
-  conf_addr =3D new_conf_addr;
-  return;
-
- fail:
-  if (new_conf_addr)
-    free (new_conf_addr);
-  close (fd);
-}
-
-/*
- * Return the numeric value denoted by TAG in section SECTION or DEF
- * if that tag does not exist.
- */
-int
-conf_get_num (char *section, char *tag, int def)
-{
-  char *value =3D conf_get_str (section, tag);
-
-  if (value)
-      return atoi (value);
-  return def;
-}
-
-/* Validate X according to the range denoted by TAG in section SECTION.  */
-int
-conf_match_num (char *section, char *tag, int x)
-{
-  char *value =3D conf_get_str (section, tag);
-  int val, min, max, n;
-
-  if (!value)
-    return 0;
-  n =3D sscanf (value, "%d,%d:%d", &val, &min, &max);
-  switch (n)
-    {
-    case 1:
-      warnx("conf_match_num: %s:%s %d=3D=3D%d?", section, tag, val, x);
-      return x =3D=3D val;
-    case 3:
-      warnx("conf_match_num: %s:%s %d<=3D%d<=3D%d?", section, tag, min, x,=
 max);
-      return min <=3D x && max >=3D x;
-    default:
-      warnx("conf_match_num: section %s tag %s: invalid number spec %s",
-		 section, tag, value);
-    }
-  return 0;
-}
-
-/* Return the string value denoted by TAG in section SECTION.  */
-char *
-conf_get_str (char *section, char *tag)
-{
-  struct conf_binding *cb;
-
-  for (cb =3D LIST_FIRST (&conf_bindings[conf_hash (section)]); cb;
-       cb =3D LIST_NEXT (cb, link))
-    if (strcasecmp (section, cb->section) =3D=3D 0
-	&& strcasecmp (tag, cb->tag) =3D=3D 0)
-      {
-	return cb->value;
-      }
-  return 0;
-}
-
-/*
- * Build a list of string values out of the comma separated value denoted =
by
- * TAG in SECTION.
- */
-struct conf_list *
-conf_get_list (char *section, char *tag)
-{
-  char *liststr =3D 0, *p, *field, *t;
-  struct conf_list *list =3D 0;
-  struct conf_list_node *node;
-
-  list =3D malloc (sizeof *list);
-  if (!list)
-    goto cleanup;
-  TAILQ_INIT (&list->fields);
-  list->cnt =3D 0;
-  liststr =3D conf_get_str (section, tag);
-  if (!liststr)
-    goto cleanup;
-  liststr =3D strdup (liststr);
-  if (!liststr)
-    goto cleanup;
-  p =3D liststr;
-  while ((field =3D strsep (&p, ",")) !=3D NULL)
-    {
-      /* Skip leading whitespace */
-      while (isspace (*field))
-	field++;
-      /* Skip trailing whitespace */
-      if (p)
-	for (t =3D p - 1; t > field && isspace (*t); t--)
-	  *t =3D '\0';
-      if (*field =3D=3D '\0')
-	{
-	  warnx("conf_get_list: empty field, ignoring...");
-	  continue;
-	}
-      list->cnt++;
-      node =3D calloc (1, sizeof *node);
-      if (!node)
-	goto cleanup;
-      node->field =3D strdup (field);
-      if (!node->field) {
-	free(node);
-	goto cleanup;
-      }
-      TAILQ_INSERT_TAIL (&list->fields, node, link);
-    }
-  free (liststr);
-  return list;
-
- cleanup:
-  if (list)
-    conf_free_list (list);
-  if (liststr)
-    free (liststr);
-  return 0;
-}
-
-struct conf_list *
-conf_get_tag_list (char *section)
-{
-  struct conf_list *list =3D 0;
-  struct conf_list_node *node;
-  struct conf_binding *cb;
-
-  list =3D malloc (sizeof *list);
-  if (!list)
-    goto cleanup;
-  TAILQ_INIT (&list->fields);
-  list->cnt =3D 0;
-  for (cb =3D LIST_FIRST (&conf_bindings[conf_hash (section)]); cb;
-       cb =3D LIST_NEXT (cb, link))
-    if (strcasecmp (section, cb->section) =3D=3D 0)
-      {
-	list->cnt++;
-	node =3D calloc (1, sizeof *node);
-	if (!node)
-	  goto cleanup;
-	node->field =3D strdup (cb->tag);
-	if (!node->field) {
-	  free(node);
-	  goto cleanup;
-	}
-	TAILQ_INSERT_TAIL (&list->fields, node, link);
-      }
-  return list;
-
- cleanup:
-  if (list)
-    conf_free_list (list);
-  return 0;
-}
-
-/* Decode a PEM encoded buffer.  */
-int
-conf_decode_base64 (u_int8_t *out, u_int32_t *len, u_char *buf)
-{
-  u_int32_t c =3D 0;
-  u_int8_t c1, c2, c3, c4;
-
-  while (*buf)
-    {
-      if (*buf > 127 || (c1 =3D asc2bin[*buf]) =3D=3D 255)
-	return 0;
-      buf++;
-
-      if (*buf > 127 || (c2 =3D asc2bin[*buf]) =3D=3D 255)
-	return 0;
-      buf++;
-
-      if (*buf =3D=3D '=3D')
-	{
-	  c3 =3D c4 =3D 0;
-	  c++;
-
-	  /* Check last four bit */
-	  if (c2 & 0xF)
-	    return 0;
-
-	  if (strcmp ((char *)buf, "=3D=3D") =3D=3D 0)
-	    buf++;
-	  else
-	    return 0;
-	}
-      else if (*buf > 127 || (c3 =3D asc2bin[*buf]) =3D=3D 255)
-	return 0;
-      else
-	{
-	  if (*++buf =3D=3D '=3D')
-	    {
-	      c4 =3D 0;
-	      c +=3D 2;
-
-	      /* Check last two bit */
-	      if (c3 & 3)
-		return 0;
-
-	      if (strcmp ((char *)buf, "=3D"))
-		return 0;
-
-	    }
-	  else if (*buf > 127 || (c4 =3D asc2bin[*buf]) =3D=3D 255)
-	      return 0;
-	  else
-	      c +=3D 3;
-	}
-
-      buf++;
-      *out++ =3D (c1 << 2) | (c2 >> 4);
-      *out++ =3D (c2 << 4) | (c3 >> 2);
-      *out++ =3D (c3 << 6) | c4;
-    }
-
-  *len =3D c;
-  return 1;
-
-}
-
-void
-conf_free_list (struct conf_list *list)
-{
-  struct conf_list_node *node =3D TAILQ_FIRST (&list->fields);
-
-  while (node)
-    {
-      TAILQ_REMOVE (&list->fields, node, link);
-      if (node->field)
-	free (node->field);
-      free (node);
-      node =3D TAILQ_FIRST (&list->fields);
-    }
-  free (list);
-}
-
-int
-conf_begin (void)
-{
-  static int seq =3D 0;
-
-  return ++seq;
-}
-
-static struct conf_trans *
-conf_trans_node (int transaction, enum conf_op op)
-{
-  struct conf_trans *node;
-
-  node =3D calloc (1, sizeof *node);
-  if (!node)
-    {
-      warnx("conf_trans_node: calloc (1, %lu) failed",
-	(unsigned long)sizeof *node);
-      return 0;
-    }
-  node->trans =3D transaction;
-  node->op =3D op;
-  TAILQ_INSERT_TAIL (&conf_trans_queue, node, link);
-  return node;
-}
-
-/* Queue a set operation.  */
-int
-conf_set (int transaction, char *section, char *tag, char *value, int over=
ride,
-	  int is_default)
-{
-  struct conf_trans *node;
-
-  node =3D conf_trans_node (transaction, CONF_SET);
-  if (!node)
-    return 1;
-  node->section =3D strdup (section);
-  if (!node->section)
-    {
-      warnx("conf_set: strdup (\"%s\") failed", section);
-      goto fail;
-    }
-  node->tag =3D strdup (tag);
-  if (!node->tag)
-    {
-      warnx("conf_set: strdup (\"%s\") failed", tag);
-      goto fail;
-    }
-  node->value =3D strdup (value);
-  if (!node->value)
-    {
-      warnx("conf_set: strdup (\"%s\") failed", value);
-      goto fail;
-    }
-  node->override =3D override;
-  node->is_default =3D is_default;
-  return 0;
-
- fail:
-  if (node->tag)
-    free (node->tag);
-  if (node->section)
-    free (node->section);
-  if (node)
-    free (node);
-  return 1;
-}
-
-/* Queue a remove operation.  */
-int
-conf_remove (int transaction, char *section, char *tag)
-{
-  struct conf_trans *node;
-
-  node =3D conf_trans_node (transaction, CONF_REMOVE);
-  if (!node)
-    goto fail;
-  node->section =3D strdup (section);
-  if (!node->section)
-    {
-      warnx("conf_remove: strdup (\"%s\") failed", section);
-      goto fail;
-    }
-  node->tag =3D strdup (tag);
-  if (!node->tag)
-    {
-      warnx("conf_remove: strdup (\"%s\") failed", tag);
-      goto fail;
-    }
-  return 0;
-
- fail:
-  if (node && node->section)
-    free (node->section);
-  if (node)
-    free (node);
-  return 1;
-}
-
-/* Queue a remove section operation.  */
-int
-conf_remove_section (int transaction, char *section)
-{
-  struct conf_trans *node;
-
-  node =3D conf_trans_node (transaction, CONF_REMOVE_SECTION);
-  if (!node)
-    goto fail;
-  node->section =3D strdup (section);
-  if (!node->section)
-    {
-      warnx("conf_remove_section: strdup (\"%s\") failed", section);
-      goto fail;
-    }
-  return 0;
-
- fail:
-  if (node)
-    free (node);
-  return 1;
-}
-
-/* Execute all queued operations for this transaction.  Cleanup.  */
-int
-conf_end (int transaction, int commit)
-{
-  struct conf_trans *node, *next;
-
-  for (node =3D TAILQ_FIRST (&conf_trans_queue); node; node =3D next)
-    {
-      next =3D TAILQ_NEXT (node, link);
-      if (node->trans =3D=3D transaction)
-	{
-	  if (commit)
-	    switch (node->op)
-	      {
-	      case CONF_SET:
-		conf_set_now (node->section, node->tag, node->value,
-			      node->override, node->is_default);
-		break;
-	      case CONF_REMOVE:
-		conf_remove_now (node->section, node->tag);
-		break;
-	      case CONF_REMOVE_SECTION:
-		conf_remove_section_now (node->section);
-		break;
-	      default:
-		warnx("conf_end: unknown operation: %d", node->op);
-	      }
-	  TAILQ_REMOVE (&conf_trans_queue, node, link);
-	  if (node->section)
-	    free (node->section);
-	  if (node->tag)
-	    free (node->tag);
-	  if (node->value)
-	    free (node->value);
-	  free (node);
-	}
-    }
-  return 0;
-}
-
-/*
- * Dump running configuration upon SIGUSR1.
- * Configuration is "stored in reverse order", so reverse it again.
- */
-struct dumper {
-  char *s, *v;
-  struct dumper *next;
-};
-
-static void
-conf_report_dump (struct dumper *node)
-{
-  /* Recursive, cleanup when we're done.  */
-
-  if (node->next)
-    conf_report_dump (node->next);
-
-  if (node->v)
-    warnx("%s=3D\t%s", node->s, node->v);
-  else if (node->s)
-    {
-      warnx("%s", node->s);
-      if (strlen (node->s) > 0)
-	free (node->s);
-    }
-
-  free (node);
-}
-
-void
-conf_report (void)
-{
-  struct conf_binding *cb, *last =3D 0;
-  unsigned int i, len;
-  char *current_section =3D (char *)0;
-  struct dumper *dumper, *dnode;
-
-  dumper =3D dnode =3D (struct dumper *)calloc (1, sizeof *dumper);
-  if (!dumper)
-    goto mem_fail;
-
-  warnx("conf_report: dumping running configuration");
-
-  for (i =3D 0; i < sizeof conf_bindings / sizeof conf_bindings[0]; i++)
-    for (cb =3D LIST_FIRST (&conf_bindings[i]); cb;
-	 cb =3D LIST_NEXT (cb, link))
-      {
-	if (!cb->is_default)
-	  {
-	    /* Dump this entry.  */
-	    if (!current_section || strcmp (cb->section, current_section))
-	      {
-		if (current_section)
-		  {
-		    len =3D strlen (current_section) + 3;
-		    dnode->s =3D malloc (len);
-		    if (!dnode->s)
-		      goto mem_fail;
-
-		    snprintf (dnode->s, len, "[%s]", current_section);
-		    dnode->next
-		      =3D (struct dumper *)calloc (1, sizeof (struct dumper));
-		    dnode =3D dnode->next;
-		    if (!dnode)
-		      goto mem_fail;
-
-		    dnode->s =3D "";
-		    dnode->next
-		      =3D (struct dumper *)calloc (1, sizeof (struct dumper));
-		    dnode =3D dnode->next;
-		    if (!dnode)
-		      goto mem_fail;
-		  }
-		current_section =3D cb->section;
-	      }
-	    dnode->s =3D cb->tag;
-	    dnode->v =3D cb->value;
-	    dnode->next =3D (struct dumper *)calloc (1, sizeof (struct dumper));
-	    dnode =3D dnode->next;
-	    if (!dnode)
-	      goto mem_fail;
-	    last =3D cb;
-	  }
-      }
-
-  if (last)
-    {
-      len =3D strlen (last->section) + 3;
-      dnode->s =3D malloc (len);
-      if (!dnode->s)
-	goto mem_fail;
-      snprintf (dnode->s, len, "[%s]", last->section);
-    }
-
-  conf_report_dump (dumper);
-
-  return;
-
- mem_fail:
-  warnx("conf_report: malloc/calloc failed");
-  while ((dnode =3D dumper) !=3D 0)
-    {
-      dumper =3D dumper->next;
-      if (dnode->s)
-	free (dnode->s);
-      free (dnode);
-    }
-  return;
-}
diff --git a/utils/idmapd/cfg.h b/utils/idmapd/cfg.h
deleted file mode 100644
index c1ca940..0000000
--- a/utils/idmapd/cfg.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/* $OpenBSD: conf.h,v 1.30 2004/06/25 20:25:34 hshoexer Exp $	 */
-/* $EOM: conf.h,v 1.13 2000/09/18 00:01:47 ho Exp $	 */
-
-/*
- * Copyright (c) 1998, 1999, 2001 Niklas Hallqvist.  All rights reserved.
- * Copyright (c) 2000, 2003 H=E5kan Olsson.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTI=
ES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF US=
E,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/*
- * This code was written under funding by Ericsson Radio Systems.
- */
-
-#ifndef _CONF_H_
-#define _CONF_H_
-
-#include "queue.h"
-
-struct conf_list_node {
-	TAILQ_ENTRY(conf_list_node) link;
-	char	*field;
-};
-
-struct conf_list {
-	size_t	cnt;
-	TAILQ_HEAD(conf_list_fields_head, conf_list_node) fields;
-};
-
-extern char    *conf_path;
-
-extern int      conf_begin(void);
-extern int      conf_decode_base64(u_int8_t *, u_int32_t *, u_char *);
-extern int      conf_end(int, int);
-extern void     conf_free_list(struct conf_list *);
-extern struct sockaddr *conf_get_address(char *, char *);
-extern struct conf_list *conf_get_list(char *, char *);
-extern struct conf_list *conf_get_tag_list(char *);
-extern int      conf_get_num(char *, char *, int);
-extern char    *conf_get_str(char *, char *);
-extern void     conf_init(void);
-extern int      conf_match_num(char *, char *, int);
-extern void     conf_reinit(void);
-extern int      conf_remove(int, char *, char *);
-extern int      conf_remove_section(int, char *);
-extern int      conf_set(int, char *, char *, char *, int, int);
-extern void     conf_report(void);
-
-#endif				/* _CONF_H_ */
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index c1cf4eb..c769b9c 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -66,7 +66,7 @@
 #endif /* HAVE_CONFIG_H */
 =

 #include "xlog.h"
-#include "cfg.h"
+#include "conffile.h"
 #include "queue.h"
 #include "nfslib.h"
 =

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 2/9] Ignore blanks in section definitions and before assignment statements
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
  2009-03-09 20:47 ` [Patch 1/9] Make idmapd's Configuration Parsing Code Available Steve Dickson
@ 2009-03-09 20:50 ` Steve Dickson
  2009-03-09 20:52 ` [Patch 3/9] Ensure configuration values are stored in lower case Steve Dickson
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 20:50 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit 5974cd0fdc295fed36f6bc092d9569b41808d19e
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 13:55:25 2009 -0400

    Taught conf_parse_line() to ignore spaces in the
     '[section]' parsing and before the assignment statements
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 754bed9..1b8d098 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -50,11 +50,6 @@
 #include "xlog.h"
 
 static void conf_load_defaults (int);
-#if 0
-static int conf_find_trans_xf (int, char *);
-#endif
-
-size_t  strlcpy(char *, const char *, size_t);
 
 struct conf_trans {
 	TAILQ_ENTRY (conf_trans) link;
@@ -219,26 +214,48 @@ conf_parse_line(int trans, char *line, size_t sz)
 	if (*line == '#' || *line == ';')
 		return;
 
+	/* Ignore blank lines */
+	if (*line == '\0')
+		return;
+
+	/* Strip off any leading blanks */
+	while (isblank(*line)) 
+		line++;
+
 	/* '[section]' parsing...  */
 	if (*line == '[') {
-		for (i = 1; i < sz; i++)
-			if (line[i] == ']')
+		line++;
+		/* Strip off any blanks after '[' */
+		while (isblank(*line)) 
+			line++;
+
+		for (i = 0; i < sz; i++) {
+			if (line[i] == ']') {
 				break;
+			}
+		}
 		if (section)
-			free (section);
+			free(section);
 		if (i == sz) {
 			xlog_warn("conf_parse_line: %d:"
  				"non-matched ']', ignoring until next section", ln);
 			section = 0;
 			return;
 		}
+		/* Strip off any blanks before ']' */
+		val = line;
+		while (*val && !isblank(*val)) 
+			val++, j++;
+		if (*val)
+			i = j;
+
 		section = malloc(i);
 		if (!section) {
 			xlog_warn("conf_parse_line: %d: malloc (%lu) failed", ln,
 						(unsigned long)i);
 			return;
 		}
-		strlcpy(section, line + 1, i);
+		strncpy(section, line, i);
 		return;
 	}

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 3/9] Ensure configuration values are stored in lower case
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
  2009-03-09 20:47 ` [Patch 1/9] Make idmapd's Configuration Parsing Code Available Steve Dickson
  2009-03-09 20:50 ` [Patch 2/9] Ignore blanks in section definitions and before assignment statements Steve Dickson
@ 2009-03-09 20:52 ` Steve Dickson
  2009-03-09 20:58 ` [Patch 4/9] Mount support routines used to convert mount options Steve Dickson
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 20:52 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit 3b192f9d86e13ee0413c4dcc88229e6bc35cb5c9
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 13:57:10 2009 -0400

    Store values in lower case. This makes it easier to do string
    comparisons when reading lists from the configuration file.
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 1b8d098..a362700 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -114,7 +114,16 @@ conf_hash(char *s)
 	}
 	return hash;
 }
+/*
+ * Convert letter from upper case to lower case
+ */
+static inline upper2lower(char *str)
+{
+	char *ptr = str;
 
+	while (*ptr) 
+		*ptr++ = tolower(*ptr);
+}
 /*
  * Insert a tag-value combination from LINE (the equal sign is at POS)
  */
@@ -523,7 +532,7 @@ conf_get_tag_list (char *section)
 	for (; cb; cb = LIST_NEXT(cb, link)) {
 		if (strcasecmp (section, cb->section) == 0) {
 			list->cnt++;
-			node = calloc (1, sizeof *node);
+			node = calloc(1, sizeof *node);
 			if (!node)
 				goto cleanup;
 			node->field = strdup(cb->tag);
@@ -615,7 +624,7 @@ conf_free_list(struct conf_list *list)
 }
 
 int
-conf_begin (void)
+conf_begin(void)
 {
   static int seq = 0;
 
@@ -623,7 +632,7 @@ conf_begin (void)
 }
 
 static struct conf_trans *
-conf_trans_node (int transaction, enum conf_op op)
+conf_trans_node(int transaction, enum conf_op op)
 {
 	struct conf_trans *node;
 
@@ -639,9 +648,9 @@ conf_trans_node (int transaction, enum conf_op op)
 	return node;
 }
 
-/* Queue a set operation.  */
+/* Queue a set operation.  Store value in lower case */
 int
-conf_set (int transaction, char *section, char *tag, 
+conf_set(int transaction, char *section, char *tag, 
 	char *value, int override, int is_default)
 {
 	struct conf_trans *node;
@@ -654,16 +663,22 @@ conf_set (int transaction, char *section, char *tag,
 		xlog_warn("conf_set: strdup(\"%s\") failed", section);
 		goto fail;
 	}
+	upper2lower(node->section);
+
 	node->tag = strdup(tag);
 	if (!node->tag) {
 		xlog_warn("conf_set: strdup(\"%s\") failed", tag);
 		goto fail;
 	}
+	upper2lower(node->tag);
+
 	node->value = strdup(value);
 	if (!node->value) {
 		xlog_warn("conf_set: strdup(\"%s\") failed", value);
 		goto fail;
 	}
+	upper2lower(node->value);
+
 	node->override = override;
 	node->is_default = is_default;
 	return 0;

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 4/9] Mount support routines used to convert mount options
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
                   ` (2 preceding siblings ...)
  2009-03-09 20:52 ` [Patch 3/9] Ensure configuration values are stored in lower case Steve Dickson
@ 2009-03-09 20:58 ` Steve Dickson
  2009-03-09 21:03 ` [Patch 5/9] Hooks needs incorporate file configuration code Steve Dickson
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 20:58 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit 06dc48fe691d2147c9b3d505183266662ac17e35
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 14:00:34 2009 -0400

    Support routines used to read sections from the configuration file
    and parse them into comma separated mount options.
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/Makefile.am b/utils/mount/Makefile.am
index 459fa45..29e993d 100644
--- a/utils/mount/Makefile.am
+++ b/utils/mount/Makefile.am
@@ -12,7 +12,7 @@ EXTRA_DIST = nfsmount.x $(man8_MANS) $(man5_MANS)
 mount_nfs_SOURCES = mount.c error.c network.c fstab.c token.c \
 		    parse_opt.c parse_dev.c \
 		    nfsmount.c nfs4mount.c stropts.c\
-		    nfsumount.c \
+		    nfsumount.c configfile.c \
 		    mount_constants.h error.h network.h fstab.h token.h \
 		    parse_opt.h parse_dev.h \
 		    nfs4_mount.h nfs_mount4.h stropts.h version.h
diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
new file mode 100644
index 0000000..722eaba
--- /dev/null
+++ b/utils/mount/configfile.c
@@ -0,0 +1,299 @@
+/*
+ * configfile.c -- mount configuration file manipulation 
+ * Copyright (C) 2008 Red Hat <nfs-team@redhat.com>
+ *
+ * - Routines use to create mount options from the mount
+ *   configuration file.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+#include <sys/types.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+
+#include "xlog.h"
+#include "conffile.h"
+
+#define KBYTES(x)     ((x) * (1024))
+#define MEGABYTES(x)  ((x) * (1048576))
+#define GIGABYTES(x)  ((x) * (1073741824))
+
+#ifndef NFSMOUNT_GLOBAL_OPTS
+#define NFSMOUNT_GLOBAL_OPTS "NFSMount_Global_Options"
+#endif
+
+#ifndef MOUNTOPTS_CONFFILE
+#define MOUNTOPTS_CONFFILE "/etc/nfsmount.conf"
+#endif
+char *conf_path = MOUNTOPTS_CONFFILE;
+enum {
+	MNT_NOARG=0,
+	MNT_INTARG,
+	MNT_STRARG,
+	MNT_SPEC,
+	MNT_UNSET
+};
+struct mnt_alias {
+	char *alias;
+	char *opt;
+	int  argtype;
+} mnt_alias_tab[] = {
+	{"background", "bg", MNT_NOARG},
+	{"foreground", "fg", MNT_NOARG},
+	{"sloppy", "sloppy", MNT_NOARG},
+};
+int mnt_alias_sz = (sizeof(mnt_alias_tab)/sizeof(mnt_alias_tab[0]));
+
+/*
+ * See if the option is an alias, if so return the 
+ * real mount option along with the argument type.
+ */
+inline static 
+char *mountopts_alias(char *opt, int *argtype)
+{
+	int i;
+
+	*argtype = MNT_UNSET;
+	for (i=0; i < mnt_alias_sz; i++) {
+		if (strcasecmp(opt, mnt_alias_tab[i].alias) != 0)
+			continue;
+		*argtype = mnt_alias_tab[i].argtype;
+		return mnt_alias_tab[i].opt;
+	}
+	return opt;
+}
+/*
+ * Convert numeric strings that end with 'k', 'm' or 'g'
+ * into numeric strings with the real value. 
+ * Meaning '8k' becomes '8094'.
+ */
+char *mountopts_convert(char *value)
+{
+	unsigned long long factor, num;
+	static char buf[64];
+	char *ch;
+
+	ch = &value[strlen(value)-1];
+	switch (tolower(*ch)) {
+	case 'k':
+		factor = KBYTES(1);
+		break;
+	case 'm':
+		factor = MEGABYTES(1);
+		break;
+	case 'g':
+		factor = GIGABYTES(1);
+		break;
+	default:
+		return value;
+	}
+	*ch = '\0';
+	if (strncmp(value, "0x", 2) == 0) {
+		num = strtol(value, (char **)NULL, 16);
+	} else if (strncmp(value, "0", 1) == 0) {
+		num = strtol(value, (char **)NULL, 8);
+	} else {
+		num = strtol(value, (char **)NULL, 10);
+	}
+	num *= factor;
+	snprintf(buf, 64, "%lld", num);
+
+	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);
+}
+/*
+ * 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;
+
+	SLIST_FOREACH(entry, &head, entries) {
+		if (strcasecmp(entry->opt, opt) == 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);
+	}
+}
+/*
+ * 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 
+conf_parse_mntopts(char *section, char *opts)
+{
+	struct conf_list *list;
+	struct conf_list_node *node;
+	char buf[BUFSIZ], *value, *field;
+	int argtype;
+
+	list = conf_get_tag_list(section);
+	TAILQ_FOREACH(node, &list->fields, link) {
+		/*
+		 * Do not overwrite options if already exists 
+		 */
+		snprintf(buf, BUFSIZ, "%s=", node->field);
+		if (strstr(opts, buf) != NULL)
+			continue;
+		if (lookup_entry(node->field) != NULL)
+			continue;
+
+		buf[0] = '\0';
+		value = conf_get_str(section, node->field);
+		field = mountopts_alias(node->field, &argtype);
+		if (strcasecmp(value, "false") == 0) {
+			if (argtype != MNT_NOARG)
+				snprintf(buf, BUFSIZ, "no%s", field);
+		} else if (strcasecmp(value, "true") == 0) {
+			snprintf(buf, BUFSIZ, "%s", field);
+		} else {
+			value = mountopts_convert(value);
+			snprintf(buf, BUFSIZ, "%s=%s", field, value);
+		}
+		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);
+	}
+	conf_free_list(list);
+}
+
+/*
+ * 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 
+ * overwritten so it matter which when each section is
+ * parsed. 
+ */
+char *conf_get_mntopts(char *spec, char *mount_point, 
+	char *mount_opts)
+{
+	struct entry *entry;
+	char *ptr, *server, *config_opts;
+
+	SLIST_INIT(&head);
+	list_size = 0;
+
+	/*
+	 * First see if there are any mount options relative 
+	 * to the mount point.
+	 */
+	conf_parse_mntopts(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"); 
+		free_all();
+		return mount_opts;
+	}
+	if ((ptr = strchr(server, ':')) != NULL)
+		*ptr='\0';
+	conf_parse_mntopts(server, mount_opts);
+	free(server);
+
+	/*
+	 * Finally process all the global mount options. 
+	 */
+	conf_parse_mntopts(NFSMOUNT_GLOBAL_OPTS, 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;
+
+	/*
+	 * Found options in the configuration file. So
+	 * concatenate the configuration options with the 
+	 * options that were passed in
+	 */
+	config_opts = calloc(1, (list_size+strlen(mount_opts)+1));
+	if (server == NULL) {
+		xlog_warn("conf_get_mountops: Unable calloc memory for config_opts"); 
+		free_all();
+		return mount_opts;
+	}
+	strcat(config_opts, mount_opts);
+	strcat(config_opts, ",");
+
+	SLIST_FOREACH(entry, &head, entries) {
+		strcat(config_opts, entry->opt);
+		strcat(config_opts, ",");
+	}
+	*(strrchr(config_opts, ',')) = '\0';
+
+	free_all();
+	free(mount_opts);
+
+	return config_opts;
+}

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 5/9] Hooks needs incorporate file configuration code.
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
                   ` (3 preceding siblings ...)
  2009-03-09 20:58 ` [Patch 4/9] Mount support routines used to convert mount options Steve Dickson
@ 2009-03-09 21:03 ` Steve Dickson
  2009-03-09 21:04 ` [Patch 6/9] An example of an nfsmount.conf file Steve Dickson
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 21:03 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit 4623964850df9c49a5ad5d77dffbb7181ed96a92
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 14:01:38 2009 -0400

    Added hooks to the mount command that allow
    mount options to be set in a configuration file
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index 06e2804..0c38ea3 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -46,6 +46,9 @@
 #include "error.h"
 #include "stropts.h"
 #include "version.h"
+#include "conffile.h"
+
+extern char *conf_get_mntopts(char *, char *, char *);
 
 char *progname;
 int nfs_mount_data_version;
@@ -474,6 +477,11 @@ int main(int argc, char *argv[])
 	spec = argv[1];
 	mount_point = argv[2];
 
+	/*
+	 * Read the the default mount options
+	 */
+	conf_init();
+
 	argv[2] = argv[0]; /* so that getopt error messages are correct */
 	while ((c = getopt_long(argc - 2, argv + 2, "rvVwfno:hs",
 				longopts, NULL)) != -1) {
@@ -559,7 +567,10 @@ int main(int argc, char *argv[])
 		mnt_err = EX_USAGE;
 		goto out;
 	}
-
+	/*
+	 * Concatenate mount options from the configuration file
+	 */
+	mount_opts = conf_get_mntopts(spec, mount_point, mount_opts);
 	parse_opts(mount_opts, &flags, &extra_opts);
 
 	if (uid != 0) {

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 6/9] An example of an nfsmount.conf file
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
                   ` (4 preceding siblings ...)
  2009-03-09 21:03 ` [Patch 5/9] Hooks needs incorporate file configuration code Steve Dickson
@ 2009-03-09 21:04 ` Steve Dickson
  2009-03-09 21:06 ` [Patch 7/9] New nfsmount.conf(5) man page Steve Dickson
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 21:04 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit f35a86f80c7e82d86403d0cf4e01b725e556b0a9
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 14:04:20 2009 -0400

    The example nfsmount.conf file
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/Makefile.am b/utils/mount/Makefile.am
index 29e993d..de810a4 100644
--- a/utils/mount/Makefile.am
+++ b/utils/mount/Makefile.am
@@ -8,7 +8,7 @@ man8_MANS	= mount.nfs.man umount.nfs.man
 man5_MANS	= nfs.man
 
 sbin_PROGRAMS	= mount.nfs
-EXTRA_DIST = nfsmount.x $(man8_MANS) $(man5_MANS)
+EXTRA_DIST = nfsmount.x $(man8_MANS) $(man5_MANS) nfsmount.conf
 mount_nfs_SOURCES = mount.c error.c network.c fstab.c token.c \
 		    parse_opt.c parse_dev.c \
 		    nfsmount.c nfs4mount.c stropts.c\
diff --git a/utils/mount/nfsmount.conf b/utils/mount/nfsmount.conf
new file mode 100644
index 0000000..9272284
--- /dev/null
+++ b/utils/mount/nfsmount.conf
@@ -0,0 +1,120 @@
+#
+# /etc/nfsmount.conf - see nfsmount.conf(5) for details
+#
+# This is an NFS mount configuration file. This file can be broken
+# up into three different sections: Mount, Server and Global
+# 
+# [ <Mount_point> ] 
+# This section defines all the mount options that
+# should be used on a particular mount point. The '<Mount_Point>'
+# string need to be an exact match of the path in the mount 
+# command. Example:
+#     [/export/home]
+#       background=True
+# Would cause all mount to /export/home would be done in
+# the background
+#
+# [ <Server_Name> ]
+# This section defines all the mount options that
+# should be used on mounts to a particular NFS server. 
+# Example:
+#     [nfsserver.foo.com]
+#       rsize=32k
+#       wsize=32k
+# All reads and writes to the 'nfsserver.foo.com' server 
+# will be done with 32k (32768 bytes) block sizes.
+#
+#[ NFSMount_Global_Options ]
+# This statically named section defines global mount 
+# options that can be applied on all NFS mount.
+#
+# Protocol Version [2,3]
+# Nfsvers=3
+# Network Transport [Udp,Tcp,Rdma]
+# Proto=Tcp
+#
+# The number of times a request will be retired before 
+# generating a timeout 
+# Retrans=2
+#
+# The number of minutes that will retry mount
+# Retry=2
+#
+# The minimum time (in seconds) file attributes are cached
+# acregmin=30
+#
+# The Maximum time (in seconds) file attributes are cached
+# acregmin=60
+#
+# The minimum time (in seconds) directory attributes are cached
+# acregmin=30
+#
+# The Maximum time (in seconds) directory attributes are cached
+# acregmin=60
+#
+# Enable Access  Control  Lists
+# Acl=False
+#
+# Enable Attribute Caching
+# Ac=True
+#
+# Do mounts in background (i.e. asynchronously)
+# Background=False
+#
+# Close-To-Open cache coherence
+# Cto=True
+#
+# Do mounts in foreground (i.e. synchronously)
+# Foreground=True
+#
+# How to handle times out from servers (Hard is STRONGLY suggested)
+# Hard=True
+# Soft=False
+#
+# Enable File Locking
+# Lock=True
+#
+# Enable READDIRPLUS on NFS version 3 mounts
+# Rdirplus=True
+#
+# Maximum Read Size (in Bytes)
+# Rsize=8k
+#
+# Maximum Write Size (in Bytes)
+# Wsize=8k
+#
+# Maximum Server Block Size (in Bytes)
+# Bsize=8k
+#
+# Ignore unknown mount options
+# Sloppy=False
+#
+# Share Data and Attribute Caches
+# Sharecache=True
+#
+# The amount of time, in tenths of a seconds, the client
+# will wait for a response from the server before retransmitting
+# the request.
+# Timeo=600
+#
+# Sets all attributes times to the same time (in seconds)
+# actimeo=30
+#
+# Server Mountd port mountport
+# mountport=4001
+#
+# Server Mountd Protocol
+# mountproto=tcp
+#
+# Server Mountd Version
+# mounvers=3
+#
+# Server Mountd Host
+# mounthost=hostname
+#
+# Server Port
+# Port=2049
+#
+# RPCGSS security flavors 
+# [none, sys, krb5, krb5i, krb5p ]
+# Sec=sys

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 7/9] New nfsmount.conf(5) man page
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
                   ` (5 preceding siblings ...)
  2009-03-09 21:04 ` [Patch 6/9] An example of an nfsmount.conf file Steve Dickson
@ 2009-03-09 21:06 ` Steve Dickson
  2009-03-09 21:08 ` [Patch 8/9] Another way to define the configuration file Steve Dickson
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 21:06 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit 5232a0cbb7d7b1eb64dfb3f282b42d1ff5fd586a
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 14:08:57 2009 -0400

    The new nfsmount.conf(5) man page and the update to
    the nfs(5) man page
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/Makefile.am b/utils/mount/Makefile.am
index de810a4..a586edc 100644
--- a/utils/mount/Makefile.am
+++ b/utils/mount/Makefile.am
@@ -5,7 +5,7 @@
 sbindir = /sbin
 
 man8_MANS	= mount.nfs.man umount.nfs.man
-man5_MANS	= nfs.man
+man5_MANS	= nfs.man nfsmount.conf.man
 
 sbin_PROGRAMS	= mount.nfs
 EXTRA_DIST = nfsmount.x $(man8_MANS) $(man5_MANS) nfsmount.conf
diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
index 13de524..1700c5b 100644
--- a/utils/mount/nfs.man
+++ b/utils/mount/nfs.man
@@ -752,6 +752,36 @@ In the presence of multiple client network interfaces,
 special routing policies,
 or atypical network topologies,
 the exact address to use for callbacks may be nontrivial to determine. 
+.SH "MOUNT CONFIGURATION FILE"
+All of the mount options described the above section can also be set 
+in the 
+.B /etc/nfsmount.conf
+file. The configuration file is made up of three different 
+sections: Global, Server and MountPoint. See 
+.B nfsmount.conf(5) 
+for details.
+.PP
+The mount command parses section in a particular order and
+will not use options that have been set in previous sections.
+The order of precedence is as follows:
+is:
+.HP
+.B Command line option
+- options set on the command will always be used.
+.HP
+.B Mount Point options
+- options set in the '<Mount_Point>' section will be 
+used only if they are not specified on the command line.
+.HP
+.B Sever options
+- options set in the '<Server_Name>' section will be used if 
+they are not specified on the command line section or 
+the mount point section.
+.HP
+.B Global options
+- options set in the '[NFSMount_Global_Options]' will be used 
+if they are not specified on the command or in the mount 
+point section or the server section.
 .SH EXAMPLES
 To mount an export using NFS version 2,
 use the
diff --git a/utils/mount/nfsmount.conf.man b/utils/mount/nfsmount.conf.man
new file mode 100644
index 0000000..fddd018
--- /dev/null
+++ b/utils/mount/nfsmount.conf.man
@@ -0,0 +1,68 @@
+.\"@(#)nfsmount.conf.5"
+.TH NFSMOUNT.CONF 5 "9 Mar 2008"
+.SH NAME
+nfsmount.conf - Configuration file for NFS mounts
+.SH SYNOPSIS
+Configuration file for NFS mounts that allow mount 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 define by a string enclosed by '[' and ']'
+branches, similar to '[nfs.server.com]'. 
+Variables are assignment statements that assign values 
+to particular variables using a '=' sign,
+similar to 'Proto=Tcp'.
+Sections are broken up into three basic categories:
+Global options, Server options and Mount Point options.
+.HP
+.BR [NFSMount_Global_Options]
+- This statically named section
+defines all of the global mount options that can be 
+applied on every NFS mount.
+.HP
+.BR [<Server_Name>]
+- This section defines all the mount options that should 
+be used on mounts to a particular NFS server. The '<Sever_Name>'
+needs to be an exact match of server name in the 
+mount command. 
+.HP
+.BR [<Mount_Point>]
+- This section defines all the mount options that 
+should be used on a particular mount point.
+The '<Mount_Point>' string need to be an exact match of the
+path in the mount command.
+.SH EXAMPLES
+.PP
+These are some example lines of how sections and variables
+are define in the configuration file.
+.PP
+[NFSMount_Global_Options]
+.br
+Proto=Tcp
+.PP
+The TCP protocol will be used on every NFS mount.
+.PP
+[nfsserver.foo.com]
+.br
+rsize=32k
+.br
+wsize=32k
+.PP
+A 33k (32768 bytes) block size will be used as the read and write
+size on all mounts to the 'nfsserver.foo.com' server.
+.PP
+[/export/home]
+.br
+Background=True
+.PP
+All mounts to the '/export/home' export will put in
+background (i.e. done asynchronously).
+.SH FILES
+.TP 10n
+.I /etc/nfsmount.conf
+Default NFS mount configuration file
+.PD
+.SH SEE ALSO
+.BR nfs (5),
+.BR mount (8),

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 8/9] Another way to define the configuration file
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
                   ` (6 preceding siblings ...)
  2009-03-09 21:06 ` [Patch 7/9] New nfsmount.conf(5) man page Steve Dickson
@ 2009-03-09 21:08 ` Steve Dickson
  2009-03-09 21:10 ` [Patch 9/9] Fixed a couple nits Steve Dickson
  2009-03-09 21:49 ` [Patch 0/9] NFS Mount Configuration File Chuck Lever
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 21:08 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit e4a13e3b2f3d6c13b40425bcdbbf4689a3ceb6fa
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 15:11:04 2009 -0400

    Added '--with-mountconfig" configuration flag so the configuration
    file can be redefined during configuration.
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/configure.ac b/configure.ac
index 5db4417..51645f2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -278,8 +278,17 @@ if test "$enable_ipv6" = yes; then
   AC_CHECK_DECL([AI_ADDRCONFIG], ,
                AC_MSG_ERROR([full getaddrinfo(3) implementation needed for IPv6 support]),
                [ #include <netdb.h> ] )
+
 fi
 
+AC_ARG_WITH(mountconfig,
+	[AC_HELP_STRING([--with-mountconfig=filename],
+		[Using filename as the NFS mount options file [/etc/nfsmounts.conf]]
+	)],
+	mountconfig=$withval,
+	mountconfig=/etc/nfsmount.conf)
+	AC_SUBST(mountconfig)
+
 dnl *************************************************************
 dnl Check for headers
 dnl *************************************************************
@@ -344,6 +353,7 @@ dnl *************************************************************
 dnl Export some path names to config.h
 dnl *************************************************************
 AC_DEFINE_UNQUOTED(NFS_STATEDIR, "$statedir", [This defines the location of the NFS state files. Warning: this must match definitions in config.mk!])
+AC_DEFINE_UNQUOTED(MOUNTOPTS_CONFFILE, "$mountconfig", [This defines the location of the NFS mount configuration file])
 
 if test "x$cross_compiling" = "xno"; then
 	CFLAGS_FOR_BUILD=${CFLAGS_FOR_BUILD-"$CFLAGS"}

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [Patch 9/9] Fixed a couple nits
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
                   ` (7 preceding siblings ...)
  2009-03-09 21:08 ` [Patch 8/9] Another way to define the configuration file Steve Dickson
@ 2009-03-09 21:10 ` Steve Dickson
  2009-03-09 21:49 ` [Patch 0/9] NFS Mount Configuration File Chuck Lever
  9 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-09 21:10 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Linux NFSv4 mailing list

commit b329b584f72a7078c59d71afd238099e7e9b4dfa
Author: Steve Dickson <steved@redhat.com>
Date:   Mon Mar 9 15:39:07 2009 -0400

    General clean up
    
    Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index a362700..f5e0f79 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -517,7 +517,7 @@ cleanup:
 }
 
 struct conf_list *
-conf_get_tag_list (char *section)
+conf_get_tag_list(char *section)
 {
 	struct conf_list *list = 0;
 	struct conf_list_node *node;
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index c769b9c..28d87c4 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -156,7 +156,7 @@ static char *nobodyuser, *nobodygroup;
 static uid_t nobodyuid;
 static gid_t nobodygid;
 
-/* Used by cfg.c */
+/* Used by conffile.c in libnfs.a */
 char *conf_path;
 
 static int
diff --git a/utils/mount/configfile.c b/utils/mount/configfile.c
index 722eaba..d1dd1bf 100644
--- a/utils/mount/configfile.c
+++ b/utils/mount/configfile.c
@@ -1,6 +1,6 @@
 /*
  * configfile.c -- mount configuration file manipulation 
- * Copyright (C) 2008 Red Hat <nfs-team@redhat.com>
+ * Copyright (C) 2008 Red Hat <nfs@redhat.com>
  *
  * - Routines use to create mount options from the mount
  *   configuration file.

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
                   ` (8 preceding siblings ...)
  2009-03-09 21:10 ` [Patch 9/9] Fixed a couple nits Steve Dickson
@ 2009-03-09 21:49 ` Chuck Lever
  2009-03-17 19:44   ` Steve Dickson
  9 siblings, 1 reply; 21+ messages in thread
From: Chuck Lever @ 2009-03-09 21:49 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

Hi Steve-

On Mar 9, 2009, at Mar 9, 2009, 4:44 PM, Steve Dickson wrote:
> Hello,
>
> The following patch set introduces a configuration file where
> mount options can be define. The options in the file can be
> applied globally or per server or per mount point.
>
> The patch set reuses the configuration parsing code that
> idmapd uses. I did added a couple enhancements like ignoring
> blanks in certain definitions as well as at the beginning of
> assignment statements.
>
> There are man pages include in the patch set but in a
> nut shell, the configuration file has three basic types
> of sections. A global section, server section and mount point
> section. There can only be one global section and multiple
> server and mount point sections.
>
> The mount command prioritize these sections in the following
> way:
>     * Options on the command line are always used.
>
>     * Options defined in the mount point section are used
>       as long a the options are not in the command line options.
>
>     * Options defined in the server section are used as long as
>       they are not defined on the command line or in the mount point
>       section.
>
>     * Options defined in the global section are used as long as the
>       options are not on the command line or in the mount point  
> section
>       or in the server section.

This can become challenging to troubleshoot if there are these semi- 
hidden options (cf. selinux).

I don't get why the per-mount section is even needed -- currently, the  
mount options in /etc/fstab are used automatically if no options are  
specified on the command line.

Is there a specific use case you have in mind for the per-mount  
section?  You just want a user to say "-o noac" and she will get all  
the remaining options in the per-mount section too?  I guess that  
means you also need to know when to specify the opposite to turn off  
the options listed in this section (like using "-o ac" if noac is  
contained in this section).  So again, that doesn't seem like  
especially helpful behavior in some cases.

This scheme doesn't allow conditional options: "always use retrans=10  
if proto=udp was specified, but retrans=2 if proto=tcp was specified."

> This is the first step toward moving the default NFS version from 3  
> to 4
> on the client.

I would think that the _only_ step is to implement the version  
fallback logic; ie. if the server doesn't support NFSv4, then use  
NFSv3, then NFSv2.  Why can't an admin simply specify a fixed NFS  
version (nfsvers=3) if there is a problem with NFSv4?  It seems to me  
that if the admin hasn't specified nfsvers=, then she does not care  
which NFS version is used.

> Having a configuration file which allows users to define
> the maximum and minimum NFS versions that should be negotiated is the
> right thing to do... IMHO.. Even though this patch does not does not
> do that, it does lay the ground work for that type of functionality.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-09 21:49 ` [Patch 0/9] NFS Mount Configuration File Chuck Lever
@ 2009-03-17 19:44   ` Steve Dickson
  2009-03-17 20:17     ` Chuck Lever
  0 siblings, 1 reply; 21+ messages in thread
From: Steve Dickson @ 2009-03-17 19:44 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

Sorry for the delayed response... 

Chuck Lever wrote:
> Hi Steve-
> 
> On Mar 9, 2009, at Mar 9, 2009, 4:44 PM, Steve Dickson wrote:
>> Hello,
>>
>> The following patch set introduces a configuration file where
>> mount options can be define. The options in the file can be
>> applied globally or per server or per mount point.
>>
>> The patch set reuses the configuration parsing code that
>> idmapd uses. I did added a couple enhancements like ignoring
>> blanks in certain definitions as well as at the beginning of
>> assignment statements.
>>
>> There are man pages include in the patch set but in a
>> nut shell, the configuration file has three basic types
>> of sections. A global section, server section and mount point
>> section. There can only be one global section and multiple
>> server and mount point sections.
>>
>> The mount command prioritize these sections in the following
>> way:
>>     * Options on the command line are always used.
>>
>>     * Options defined in the mount point section are used
>>       as long a the options are not in the command line options.
>>
>>     * Options defined in the server section are used as long as
>>       they are not defined on the command line or in the mount point
>>       section.
>>
>>     * Options defined in the global section are used as long as the
>>       options are not on the command line or in the mount point section
>>       or in the server section.
> 
> This can become challenging to troubleshoot if there are these
> semi-hidden options (cf. selinux).
Why? 'mount -v' clearly shows what options are being used and then 
of course 'cat /proc/mounts' will show all the mount options.

> 
> I don't get why the per-mount section is even needed -- currently, the
> mount options in /etc/fstab are used automatically if no options are
> specified on the command line.
Sure... this is option redundant with /etc/fstab but why not allow people
configure all the NFS mounts in one spot? Why make them edit different files?
Plus it was just really easy to do... 
 
> 
> Is there a specific use case you have in mind for the per-mount
> section?  
Not really... I figure would handy allow different options on
different file system to the same server...

> You just want a user to say "-o noac" and she will get all the
> remaining options in the per-mount section too?  
Yes.. the per-mount section will be added on to the command line options.

> I guess that means you also need to know when to specify the opposite to 
> turn off the options listed in this section (like using "-o ac" if noac 
> is contained in this section).  So again, that doesn't seem like especially 
> helpful behaviour in some cases.
Not sure I understand your point... but regardless setting 'ac=true' will 
cause the '-o ac' option to be added and 'ac=false' will cause the
'-o noac' option to be added... 
 
> 
> This scheme doesn't allow conditional options: "always use retrans=10 if
> proto=udp was specified, but retrans=2 if proto=tcp was specified."
True.. conditional options are not supported... until there is a demand
for them... 

> 
>> This is the first step toward moving the default NFS version from 3 to 4
>> on the client.
> 
> I would think that the _only_ step is to implement the version fallback
> logic; ie. if the server doesn't support NFSv4, then use NFSv3, then
> NFSv2.  Why can't an admin simply specify a fixed NFS version
> (nfsvers=3) if there is a problem with NFSv4?  
They can... Nfsvers=3. When that is set, there will be no negotiation  


> It seems to me that if the admin hasn't specified nfsvers=, then she does 
> not care which NFS version is used.
True. And then the highest version the server supports
will be negotiated. Having a Max/Min version will allow a the
client to control that negotiation... Say the want v4 but not
v4.1 ? Or may they only v41? 

> 
>> Having a configuration file which allows users to define
>> the maximum and minimum NFS versions that should be negotiated is the
>> right thing to do... IMHO.. Even though this patch does not does not
>> do that, it does lay the ground work for that type of functionality
Well I think most admins do want complete control over which NFS
version will or will not be used... esp when it comes to v4 and v4.1

thanks!

steved.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-17 19:44   ` Steve Dickson
@ 2009-03-17 20:17     ` Chuck Lever
  2009-03-17 20:25       ` J. Bruce Fields
  2009-03-18 13:07       ` Steve Dickson
  0 siblings, 2 replies; 21+ messages in thread
From: Chuck Lever @ 2009-03-17 20:17 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

Hi Steve-

On Mar 17, 2009, at Mar 17, 2009, 3:44 PM, Steve Dickson wrote:
> Sorry for the delayed response...
>
> Chuck Lever wrote:
>> Hi Steve-
>>
>> On Mar 9, 2009, at Mar 9, 2009, 4:44 PM, Steve Dickson wrote:
>>> Hello,
>>>
>>> The following patch set introduces a configuration file where
>>> mount options can be define. The options in the file can be
>>> applied globally or per server or per mount point.
>>>
>>> The patch set reuses the configuration parsing code that
>>> idmapd uses. I did added a couple enhancements like ignoring
>>> blanks in certain definitions as well as at the beginning of
>>> assignment statements.
>>>
>>> There are man pages include in the patch set but in a
>>> nut shell, the configuration file has three basic types
>>> of sections. A global section, server section and mount point
>>> section. There can only be one global section and multiple
>>> server and mount point sections.
>>>
>>> The mount command prioritize these sections in the following
>>> way:
>>>    * Options on the command line are always used.
>>>
>>>    * Options defined in the mount point section are used
>>>      as long a the options are not in the command line options.
>>>
>>>    * Options defined in the server section are used as long as
>>>      they are not defined on the command line or in the mount point
>>>      section.
>>>
>>>    * Options defined in the global section are used as long as the
>>>      options are not on the command line or in the mount point  
>>> section
>>>      or in the server section.
>>
>> This can become challenging to troubleshoot if there are these
>> semi-hidden options (cf. selinux).
> Why? 'mount -v' clearly shows what options are being used and then
> of course 'cat /proc/mounts' will show all the mount options.

Because we expect certain default settings for these mount options.   
If they can be different on every system and every mount point, then  
you have to remember to go back and check /etc/fstab, and your  
configuration file, and so on.

>> I don't get why the per-mount section is even needed -- currently,  
>> the
>> mount options in /etc/fstab are used automatically if no options are
>> specified on the command line.
> Sure... this is option redundant with /etc/fstab but why not allow  
> people
> configure all the NFS mounts in one spot? Why make them edit  
> different files?

Indeed... why make them edit /etc/fstab and some other config file?   
We have /etc/fstab for static mounts and maps for automounter.  It's  
already bad enough, and now we are considering another layer of  
complexity.  Plus, the internal syntax of the mount command  
configuration file is unlike either /etc/fstab or automounter maps.

>> Is there a specific use case you have in mind for the per-mount
>> section?
> Not really... I figure would handy allow different options on
> different file system to the same server...
>
>> You just want a user to say "-o noac" and she will get all the
>> remaining options in the per-mount section too?
> Yes.. the per-mount section will be added on to the command line  
> options.
>
>> I guess that means you also need to know when to specify the  
>> opposite to
>> turn off the options listed in this section (like using "-o ac" if  
>> noac
>> is contained in this section).  So again, that doesn't seem like  
>> especially
>> helpful behavior in some cases.
> Not sure I understand your point... but regardless setting 'ac=true'  
> will
> cause the '-o ac' option to be added and 'ac=false' will cause the
> '-o noac' option to be added...

It's the same problem we have with "-o remount", except now the  
behavior is made even more inconsistent.  For example:

"mount -o remount sync"

on a noac mount silently strips the actimeo part of noac, and makes  
what was a "sync,actimeo=0" mount point just "sync".  Now we have a  
bunch of new little tricks to watch out for.

>>> This is the first step toward moving the default NFS version from  
>>> 3 to 4
>>> on the client.
>>
>> I would think that the _only_ step is to implement the version  
>> fallback
>> logic; ie. if the server doesn't support NFSv4, then use NFSv3, then
>> NFSv2.  Why can't an admin simply specify a fixed NFS version
>> (nfsvers=3) if there is a problem with NFSv4?
> They can... Nfsvers=3. When that is set, there will be no negotiation.

That's exactly my point.

If an admin has a problem with using a particular version, they can  
specify exactly what they want, today.  Adjusting the default version  
of a mount point is already possible today.  People who don't care now  
probably are not likely to care if they get NFSv4, as long as NFSv4 is  
working as well as NFSv3.

>> It seems to me that if the admin hasn't specified nfsvers=, then  
>> she does
>> not care which NFS version is used.
> True. And then the highest version the server supports
> will be negotiated. Having a Max/Min version will allow a the
> client to control that negotiation... Say the want v4 but not
> v4.1 ? Or may they only v41?

If they care that much, why not just say nfsvers=4 and be done with it?

>>> Having a configuration file which allows users to define
>>> the maximum and minimum NFS versions that should be negotiated is  
>>> the
>>> right thing to do... IMHO.. Even though this patch does not does not
>>> do that, it does lay the ground work for that type of functionality
> Well I think most admins do want complete control over which NFS
> version will or will not be used... esp when it comes to v4 and v4.1

I honestly don't see that.  Admins want the system to work, and not be  
bothered with the trivial details.  They want either "give me anything  
that works" or "give me this specific setting."  And we have the  
ability to do that already.

Even without a config file, Linux is open source.  If an admin cares  
that much about exactly how the system works, she will build it  
herself, and possibly modify the source code.

I think the problem I have with this (besides the complexity) is that  
if we must add more ways of avoiding NFSv4, then is NFSv4 really ready  
to be made the default?

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-17 20:17     ` Chuck Lever
@ 2009-03-17 20:25       ` J. Bruce Fields
  2009-03-17 20:36         ` Chuck Lever
  2009-03-18 13:07       ` Steve Dickson
  1 sibling, 1 reply; 21+ messages in thread
From: J. Bruce Fields @ 2009-03-17 20:25 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Steve Dickson, Linux NFS Mailing list, Linux NFSv4 mailing list

On Tue, Mar 17, 2009 at 04:17:05PM -0400, Chuck Lever wrote:
> I think the problem I have with this (besides the complexity) is that  
> if we must add more ways of avoiding NFSv4, then is NFSv4 really ready  
> to be made the default?

Even given the best possible client and server implementation, there are
enough user-visible not-completely-backwards-compatible changes in the
protocol (ACLs, string names, etc.) that silently substituting v4 can't
be the right thing for every case.

--b.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-17 20:25       ` J. Bruce Fields
@ 2009-03-17 20:36         ` Chuck Lever
  0 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2009-03-17 20:36 UTC (permalink / raw)
  To: J. Bruce Fields; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Mar 17, 2009, at Mar 17, 2009, 4:25 PM, J. Bruce Fields wrote:
> On Tue, Mar 17, 2009 at 04:17:05PM -0400, Chuck Lever wrote:
>> I think the problem I have with this (besides the complexity) is that
>> if we must add more ways of avoiding NFSv4, then is NFSv4 really  
>> ready
>> to be made the default?
>
> Even given the best possible client and server implementation, there  
> are
> enough user-visible not-completely-backwards-compatible changes in the
> protocol (ACLs, string names, etc.) that silently substituting v4  
> can't
> be the right thing for every case.

For the cases where it isn't right, why is simply adding "vers=3" not  
an appropriate workaround?

It would be nice to have a working list of areas where there are going  
to be issues, so we and our users can be prepared for the transition.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-17 20:17     ` Chuck Lever
  2009-03-17 20:25       ` J. Bruce Fields
@ 2009-03-18 13:07       ` Steve Dickson
  2009-03-18 16:31         ` Chuck Lever
  1 sibling, 1 reply; 21+ messages in thread
From: Steve Dickson @ 2009-03-18 13:07 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

Chuck Lever wrote:
> Hi Steve-
> 
> On Mar 17, 2009, at Mar 17, 2009, 3:44 PM, Steve Dickson wrote:
>> Sorry for the delayed response...
>>
>> Chuck Lever wrote:
>>> Hi Steve-
>>>
>>> On Mar 9, 2009, at Mar 9, 2009, 4:44 PM, Steve Dickson wrote:
>>>> Hello,
>>>>
>>>> The following patch set introduces a configuration file where
>>>> mount options can be define. The options in the file can be
>>>> applied globally or per server or per mount point.
>>>>
>>>> The patch set reuses the configuration parsing code that
>>>> idmapd uses. I did added a couple enhancements like ignoring
>>>> blanks in certain definitions as well as at the beginning of
>>>> assignment statements.
>>>>
>>>> There are man pages include in the patch set but in a
>>>> nut shell, the configuration file has three basic types
>>>> of sections. A global section, server section and mount point
>>>> section. There can only be one global section and multiple
>>>> server and mount point sections.
>>>>
>>>> The mount command prioritize these sections in the following
>>>> way:
>>>>    * Options on the command line are always used.
>>>>
>>>>    * Options defined in the mount point section are used
>>>>      as long a the options are not in the command line options.
>>>>
>>>>    * Options defined in the server section are used as long as
>>>>      they are not defined on the command line or in the mount point
>>>>      section.
>>>>
>>>>    * Options defined in the global section are used as long as the
>>>>      options are not on the command line or in the mount point section
>>>>      or in the server section.
>>>
>>> This can become challenging to troubleshoot if there are these
>>> semi-hidden options (cf. selinux).
>> Why? 'mount -v' clearly shows what options are being used and then
>> of course 'cat /proc/mounts' will show all the mount options.
> 
> Because we expect certain default settings for these mount options.  If
> they can be different on every system and every mount point, then you
> have to remember to go back and check /etc/fstab, and your configuration
> file, and so on.
Yes... If you do change something... the default settings may or may no longer
be in effect... But I have faith that most admins would be able realize 
this... If they change something... things are going to change! :-)

> 
>>> I don't get why the per-mount section is even needed -- currently, the
>>> mount options in /etc/fstab are used automatically if no options are
>>> specified on the command line.
>> Sure... this is option redundant with /etc/fstab but why not allow people
>> configure all the NFS mounts in one spot? Why make them edit different
>> files?
> 
> Indeed... why make them edit /etc/fstab and some other config file?  
Be the /etc/fstab file can't set global or per server options...

> We have /etc/fstab for static mounts and maps for automounter.  It's
> already bad enough, and now we are considering another layer of
> complexity.  Plus, the internal syntax of the mount command
> configuration file is unlike either /etc/fstab or automounter maps.
The format follows the already existing /etc/idmap.conf file. I
basically reused the parsing routine that were already in nfs-utils.
And I must admit, they were very well written... 

While its true the format does not follow either the /etc/fstab file
or the autofs maps, I contend its a format that is much more
straightforward... Basically uncomment things... and again I have 
faith in the admins that they will be able to quickly figure it out...
What is really need is a GUI... but that's for another day... ;-) 

> 
>>> Is there a specific use case you have in mind for the per-mount
>>> section?
>> Not really... I figure would handy allow different options on
>> different file system to the same server...
>>
>>> You just want a user to say "-o noac" and she will get all the
>>> remaining options in the per-mount section too?
>> Yes.. the per-mount section will be added on to the command line options.
>>
>>> I guess that means you also need to know when to specify the opposite to
>>> turn off the options listed in this section (like using "-o ac" if noac
>>> is contained in this section).  So again, that doesn't seem like
>>> especially
>>> helpful behavior in some cases.
>> Not sure I understand your point... but regardless setting 'ac=true' will
>> cause the '-o ac' option to be added and 'ac=false' will cause the
>> '-o noac' option to be added...
> 
> It's the same problem we have with "-o remount", except now the behavior
> is made even more inconsistent.  For example:
> 
> "mount -o remount sync"
> 
> on a noac mount silently strips the actimeo part of noac, and makes what
> was a "sync,actimeo=0" mount point just "sync".  Now we have a bunch of
> new little tricks to watch out for.
IF and Only IF... one changes the configuration file... 

> 
>>>> This is the first step toward moving the default NFS version from 3
>>>> to 4
>>>> on the client.
>>>
>>> I would think that the _only_ step is to implement the version fallback
>>> logic; ie. if the server doesn't support NFSv4, then use NFSv3, then
>>> NFSv2.  Why can't an admin simply specify a fixed NFS version
>>> (nfsvers=3) if there is a problem with NFSv4?
>> They can... Nfsvers=3. When that is set, there will be no negotiation.
> 
> That's exactly my point.
> 
> If an admin has a problem with using a particular version, they can
> specify exactly what they want, today.  
Not globally... they have to change every single mount in both /etc/fstab
and the autofs maps... verse making the change in one place.
  
> Adjusting the default version of a mount point is already possible today.
Not really.... you can not change the default version you can only 
override it with each and every mount... 
 
> People who don't care now probably are not likely to care if they get NFSv4,
> as long as NFSv4 is working as well as NFSv3.
This is an assumption I don't think we should make. 

> 
>>> It seems to me that if the admin hasn't specified nfsvers=, then she
>>> does
>>> not care which NFS version is used.
>> True. And then the highest version the server supports
>> will be negotiated. Having a Max/Min version will allow a the
>> client to control that negotiation... Say the want v4 but not
>> v4.1 ? Or may they only v41?
> 
> If they care that much, why not just say nfsvers=4 and be done with it?
v4 has had years more testing than v4.1. 
 
> 
>>>> Having a configuration file which allows users to define
>>>> the maximum and minimum NFS versions that should be negotiated is the
>>>> right thing to do... IMHO.. Even though this patch does not does not
>>>> do that, it does lay the ground work for that type of functionality
>> Well I think most admins do want complete control over which NFS
>> version will or will not be used... esp when it comes to v4 and v4.1
> 
> I honestly don't see that.  Admins want the system to work, and not be
> bothered with the trivial details.  They want either "give me anything
> that works" or "give me this specific setting."  And we have the ability
> to do that already.
Yes... But for this part "give me this specific setting" having a way to
configure a setting, globally, per server, or per mount point is a 
good thing... imho... 

> 
> Even without a config file, Linux is open source.  If an admin cares
> that much about exactly how the system works, she will build it herself,
> and possibly modify the source code.
Please... how many admins do you know that are also kernel developers... :-)

> 
> I think the problem I have with this (besides the complexity) is that if
> we must add more ways of avoiding NFSv4, then is NFSv4 really ready to
> be made the default?
Yes, I see your point... But I do think its not a bad thing to give the 
admins the final say on how and what they want to run... because they are 
the ones doing the front line support.. not us.. 

I firmly believe if the transition to V4 is seamless and uneventful, 
having a configuration file will be non-issue... But if its not, then
having a configuration file will be a good thing... 

steved.
 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-18 13:07       ` Steve Dickson
@ 2009-03-18 16:31         ` Chuck Lever
  2009-03-18 18:10           ` Steve Dickson
  0 siblings, 1 reply; 21+ messages in thread
From: Chuck Lever @ 2009-03-18 16:31 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Mar 18, 2009, at Mar 18, 2009, 9:07 AM, Steve Dickson wrote:
> Chuck Lever wrote:
>> Hi Steve-
>>
>> On Mar 17, 2009, at Mar 17, 2009, 3:44 PM, Steve Dickson wrote:
>>> Sorry for the delayed response...
>>>
>>> Chuck Lever wrote:
>>>> Hi Steve-
>>>>
>>>> On Mar 9, 2009, at Mar 9, 2009, 4:44 PM, Steve Dickson wrote:
>>>>> Hello,
>>>>>
>>>>> The following patch set introduces a configuration file where
>>>>> mount options can be define. The options in the file can be
>>>>> applied globally or per server or per mount point.
>>>>>
>>>>> The patch set reuses the configuration parsing code that
>>>>> idmapd uses. I did added a couple enhancements like ignoring
>>>>> blanks in certain definitions as well as at the beginning of
>>>>> assignment statements.
>>>>>
>>>>> There are man pages include in the patch set but in a
>>>>> nut shell, the configuration file has three basic types
>>>>> of sections. A global section, server section and mount point
>>>>> section. There can only be one global section and multiple
>>>>> server and mount point sections.
>>>>>
>>>>> The mount command prioritize these sections in the following
>>>>> way:
>>>>>   * Options on the command line are always used.
>>>>>
>>>>>   * Options defined in the mount point section are used
>>>>>     as long a the options are not in the command line options.
>>>>>
>>>>>   * Options defined in the server section are used as long as
>>>>>     they are not defined on the command line or in the mount point
>>>>>     section.
>>>>>
>>>>>   * Options defined in the global section are used as long as the
>>>>>     options are not on the command line or in the mount point  
>>>>> section
>>>>>     or in the server section.
>>>>
>>>> This can become challenging to troubleshoot if there are these
>>>> semi-hidden options (cf. selinux).
>>> Why? 'mount -v' clearly shows what options are being used and then
>>> of course 'cat /proc/mounts' will show all the mount options.
>>
>> Because we expect certain default settings for these mount  
>> options.  If
>> they can be different on every system and every mount point, then you
>> have to remember to go back and check /etc/fstab, and your  
>> configuration
>> file, and so on.
> Yes... If you do change something... the default settings may or may  
> no longer
> be in effect... But I have faith that most admins would be able  
> realize
> this... If they change something... things are going to change! :-)

The point is that these settings are changed so infrequently (or are  
managed by a team of admins) that it will be too easy to overlook or  
forget this kind of change.  You are asking administrators to look in  
one more place when tracking down mount behavior, and I don't see a  
clear benefit for the added complexity.

It seems to me that automounter maps provide exactly this kind of  
functionality.  Why are they inadequate for this task?  If they are  
inadequate, why not improve automounter?

>>>> I don't get why the per-mount section is even needed --  
>>>> currently, the
>>>> mount options in /etc/fstab are used automatically if no options  
>>>> are
>>>> specified on the command line.
>>> Sure... this is option redundant with /etc/fstab but why not allow  
>>> people
>>> configure all the NFS mounts in one spot? Why make them edit  
>>> different
>>> files?
>>
>> Indeed... why make them edit /etc/fstab and some other config file?
> But the /etc/fstab file can't set global or per server options...

Yes, however you can change /etc/fstab with a global editor command.

>> We have /etc/fstab for static mounts and maps for automounter.  It's
>> already bad enough, and now we are considering another layer of
>> complexity.  Plus, the internal syntax of the mount command
>> configuration file is unlike either /etc/fstab or automounter maps.
> The format follows the already existing /etc/idmap.conf file. I
> basically reused the parsing routine that were already in nfs-utils.
> And I must admit, they were very well written...

Since we attempt to follow the example of other implementations  
(especially the reference implementation, Solaris) why are we not  
implementing a config file format that is used by other O/Ses?

I've looked for a similar facility on Solaris 10, and I don't find it  
mentioned in either mount(1M) or mount_nfs(1M).

> While its true the format does not follow either the /etc/fstab file
> or the autofs maps, I contend its a format that is much more
> straightforward... Basically uncomment things... and again I have
> faith in the admins that they will be able to quickly figure it out...
> What is really need is a GUI... but that's for another day... ;-)

Whether it is straightforward or not, it is a new and additional  
format that has to be understood.

It would make more sense if we were introducing this global config  
feature for _all_ file systems (via the mount command, not by the  
subcommands).  A GUI would follow the current trend of system-config-*  
and many other system utilities, such as NetworkManager.

>>>>> This is the first step toward moving the default NFS version  
>>>>> from 3
>>>>> to 4
>>>>> on the client.
>>>>
>>>> I would think that the _only_ step is to implement the version  
>>>> fallback
>>>> logic; ie. if the server doesn't support NFSv4, then use NFSv3,  
>>>> then
>>>> NFSv2.  Why can't an admin simply specify a fixed NFS version
>>>> (nfsvers=3) if there is a problem with NFSv4?
>>> They can... Nfsvers=3. When that is set, there will be no  
>>> negotiation.
>>
>> That's exactly my point.
>>
>> If an admin has a problem with using a particular version, they can
>> specify exactly what they want, today.
> Not globally... they have to change every single mount in both /etc/ 
> fstab
> and the autofs maps... verse making the change in one place.

If an admin cares enough to manage global changes, he will already use  
automounter and avoid /etc/fstab.

>> Adjusting the default version of a mount point is already possible  
>> today.
> Not really.... you can not change the default version you can only
> override it with each and every mount...

Again, this is not hard to do today.

>> People who don't care now probably are not likely to care if they  
>> get NFSv4,
>> as long as NFSv4 is working as well as NFSv3.
> This is an assumption I don't think we should make.

Why not?  The point of a default setting is that it works well in most  
cases.  If NFSv4 doesn't work well in most cases, then we have bigger  
problems.  Otherwise, people who care about the minor differences  
already have specific settings and configurations to deal with them.

>>>> It seems to me that if the admin hasn't specified nfsvers=, then  
>>>> she
>>>> does
>>>> not care which NFS version is used.
>>> True. And then the highest version the server supports
>>> will be negotiated. Having a Max/Min version will allow a the
>>> client to control that negotiation... Say the want v4 but not
>>> v4.1 ? Or may they only v41?
>>
>> If they care that much, why not just say nfsvers=4 and be done with  
>> it?
> v4 has had years more testing than v4.1.

You are suggesting that one cannot choose specifically among NFSv4  
minor versions with a mount option.  I truly truly hope this is not  
the case.

>>>>> Having a configuration file which allows users to define
>>>>> the maximum and minimum NFS versions that should be negotiated  
>>>>> is the
>>>>> right thing to do... IMHO.. Even though this patch does not does  
>>>>> not
>>>>> do that, it does lay the ground work for that type of  
>>>>> functionality
>>> Well I think most admins do want complete control over which NFS
>>> version will or will not be used... esp when it comes to v4 and v4.1
>>
>> I honestly don't see that.  Admins want the system to work, and not  
>> be
>> bothered with the trivial details.  They want either "give me  
>> anything
>> that works" or "give me this specific setting."  And we have the  
>> ability
>> to do that already.
> Yes... But for this part "give me this specific setting" having a  
> way to
> configure a setting, globally, per server, or per mount point is a
> good thing... imho...

No one has demonstrated yet that what we have today cannot provide the  
same effect as a global setting.  An automounter map provides this  
functionality.  What problem, exactly, are we trying to address here?

>> Even without a config file, Linux is open source.  If an admin cares
>> that much about exactly how the system works, she will build it  
>> herself,
>> and possibly modify the source code.
> Please... how many admins do you know that are also kernel  
> developers... :-)

You don't have to be a kernel developer, you just have to be able to  
read C and run "make".  That's been the whole argument for open source  
since the very beginning: it enables people to modify the code to make  
it do exactly what they want.

Admins I have known (and I myself was a large-scale Solaris admin, and  
before that, a VM/ESA admin, in previous lives) are not afraid to  
write and modify source code (when it's available), or create and  
deploy large infrastructures of their own code.  Everyone else can  
usually live pleasantly with exactly what they get in binary form.

Any admin worth his or her NaCl never relies on default settings.

>> I think the problem I have with this (besides the complexity) is  
>> that if
>> we must add more ways of avoiding NFSv4, then is NFSv4 really ready  
>> to
>> be made the default?
> Yes, I see your point... But I do think its not a bad thing to give  
> the
> admins the final say on how and what they want to run... because  
> they are
> the ones doing the front line support.. not us..
>
> I firmly believe if the transition to V4 is seamless and uneventful,
> having a configuration file will be non-issue... But if its not, then
> having a configuration file will be a good thing...

OK, then, what are we doing to make the transition seamless?  Do we  
know where the pain points are?  Do we have them documented (something  
that for example Fedora release notes can refer to)?  Are we giving  
our users real tools for managing the transition, like tools to  
convert ACLs?

Our defaults should be settings that work well out of the box in most  
environments.  If that isn't the case yet for NFSv4, then we shouldn't  
make it the default.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-18 16:31         ` Chuck Lever
@ 2009-03-18 18:10           ` Steve Dickson
       [not found]             ` <49C13928.8040806-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 21+ messages in thread
From: Steve Dickson @ 2009-03-18 18:10 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

Chuck Lever wrote:
> On Mar 18, 2009, at Mar 18, 2009, 9:07 AM, Steve Dickson wrote:
>>>>> This can become challenging to troubleshoot if there are these
>>>>> semi-hidden options (cf. selinux).
>>>> Why? 'mount -v' clearly shows what options are being used and then
>>>> of course 'cat /proc/mounts' will show all the mount options.
>>>
>>> Because we expect certain default settings for these mount options.  If
>>> they can be different on every system and every mount point, then you
>>> have to remember to go back and check /etc/fstab, and your configuration
>>> file, and so on.
>> Yes... If you do change something... the default settings may or may
>> no longer
>> be in effect... But I have faith that most admins would be able realize
>> this... If they change something... things are going to change! :-)
> 
> The point is that these settings are changed so infrequently (or are
> managed by a team of admins) that it will be too easy to overlook or
> forget this kind of change.  You are asking administrators to look in
> one more place when tracking down mount behavior, and I don't see a
> clear benefit for the added complexity.
No. I am not asking admins to do ANYTHING! It will be up to them if/when
to used this configuration file... I'm just giving them an another way to 
set NFS mounts options... They choose use so be it.. In not that ok too..

> 
> It seems to me that automounter maps provide exactly this kind of
> functionality.  Why are they inadequate for this task?  If they are
> inadequate, why not improve automounter?
Well, configuring the automounter is a very complex endeavour. Figuring out 
how to set mount options globally, per server and per mount point is not an 
easy task but with this configuration file it is.. Plus, to use the auto
maps you have to use the automounter... There are no such requirement to use
the configuration file. 

> 
>>>>> I don't get why the per-mount section is even needed -- currently, the
>>>>> mount options in /etc/fstab are used automatically if no options are
>>>>> specified on the command line.
>>>> Sure... this is option redundant with /etc/fstab but why not allow
>>>> people
>>>> configure all the NFS mounts in one spot? Why make them edit different
>>>> files?
>>>
>>> Indeed... why make them edit /etc/fstab and some other config file?
>> But the /etc/fstab file can't set global or per server options...
> 
> Yes, however you can change /etc/fstab with a global editor command.
> 
>>> We have /etc/fstab for static mounts and maps for automounter.  It's
>>> already bad enough, and now we are considering another layer of
>>> complexity.  Plus, the internal syntax of the mount command
>>> configuration file is unlike either /etc/fstab or automounter maps.
>> The format follows the already existing /etc/idmap.conf file. I
>> basically reused the parsing routine that were already in nfs-utils.
>> And I must admit, they were very well written...
> 
> Since we attempt to follow the example of other implementations
> (especially the reference implementation, Solaris) why are we not
> implementing a config file format that is used by other O/Ses?
Suggestions are always welcomed... The fact that the parsing code
already existed in nfs-utils I thought was a plus... 

> 
> I've looked for a similar facility on Solaris 10, and I don't find it
> mentioned in either mount(1M) or mount_nfs(1M).
/etc/default/nfs has a similar look an feel... but it does not the
same feature set.

> 
>> While its true the format does not follow either the /etc/fstab file
>> or the autofs maps, I contend its a format that is much more
>> straightforward... Basically uncomment things... and again I have
>> faith in the admins that they will be able to quickly figure it out...
>> What is really need is a GUI... but that's for another day... ;-)
> 
> Whether it is straightforward or not, it is a new and additional format
> that has to be understood.
This format is not new... It used by a number other configuration files
in /etc/.... and if there was one standard format for Linux configuration
files I would adhere to it... But that is simply not the case... 
 
> 
> It would make more sense if we were introducing this global config
> feature for _all_ file systems (via the mount command, not by the
> subcommands).  A GUI would follow the current trend of system-config-*
> and many other system utilities, such as NetworkManager.
I think this would be a bit too ambitious for what I'm trying to do...

> 
>>>>>> This is the first step toward moving the default NFS version from 3
>>>>>> to 4
>>>>>> on the client.
>>>>>
>>>>> I would think that the _only_ step is to implement the version
>>>>> fallback
>>>>> logic; ie. if the server doesn't support NFSv4, then use NFSv3, then
>>>>> NFSv2.  Why can't an admin simply specify a fixed NFS version
>>>>> (nfsvers=3) if there is a problem with NFSv4?
>>>> They can... Nfsvers=3. When that is set, there will be no negotiation.
>>>
>>> That's exactly my point.
>>>
>>> If an admin has a problem with using a particular version, they can
>>> specify exactly what they want, today.
>> Not globally... they have to change every single mount in both /etc/fstab
>> and the autofs maps... verse making the change in one place.
> 
> If an admin cares enough to manage global changes, he will already use
> automounter and avoid /etc/fstab.
Again, this give them just another way of doing things...


>>> People who don't care now probably are not likely to care if they get
>>> NFSv4,
>>> as long as NFSv4 is working as well as NFSv3.
>> This is an assumption I don't think we should make.
> 
> Why not?  The point of a default setting is that it works well in most
> cases.  If NFSv4 doesn't work well in most cases, then we have bigger
> problems.  Otherwise, people who care about the minor differences
> already have specific settings and configurations to deal with them.
The key words being "most cases"... With this change the default settings
can be manipulated to work in all cases... combining both groups you
are alluding to.  
 
> 
>>>>> It seems to me that if the admin hasn't specified nfsvers=, then she
>>>>> does
>>>>> not care which NFS version is used.
>>>> True. And then the highest version the server supports
>>>> will be negotiated. Having a Max/Min version will allow a the
>>>> client to control that negotiation... Say the want v4 but not
>>>> v4.1 ? Or may they only v41?
>>>
>>> If they care that much, why not just say nfsvers=4 and be done with it?
>> v4 has had years more testing than v4.1.
> 
> You are suggesting that one cannot choose specifically among NFSv4 minor
> versions with a mount option.  I truly truly hope this is not the case.
No. Of course there is a mount option to specify v4 and v4.1. But giving
admins a way of added one line to one configuration that will either make
all mounts v4 or v4.1. Something I would thing would be a bit handy... 

> 
>>>>>> Having a configuration file which allows users to define
>>>>>> the maximum and minimum NFS versions that should be negotiated is the
>>>>>> right thing to do... IMHO.. Even though this patch does not does not
>>>>>> do that, it does lay the ground work for that type of functionality
>>>> Well I think most admins do want complete control over which NFS
>>>> version will or will not be used... esp when it comes to v4 and v4.1
>>>
>>> I honestly don't see that.  Admins want the system to work, and not be
>>> bothered with the trivial details.  They want either "give me anything
>>> that works" or "give me this specific setting."  And we have the ability
>>> to do that already.
>> Yes... But for this part "give me this specific setting" having a way to
>> configure a setting, globally, per server, or per mount point is a
>> good thing... imho...
> 
> No one has demonstrated yet that what we have today cannot provide the
> same effect as a global setting.  An automounter map provides this
> functionality.  What problem, exactly, are we trying to address here?
Complexity... Not having to use the automounter and getting the similar 
type of configurability is not a bad thing... imho... 

> 
>>> Even without a config file, Linux is open source.  If an admin cares
>>> that much about exactly how the system works, she will build it herself,
>>> and possibly modify the source code.
>> Please... how many admins do you know that are also kernel
>> developers... :-)
> 
> You don't have to be a kernel developer, you just have to be able to
> read C and run "make".  That's been the whole argument for open source
> since the very beginning: it enables people to modify the code to make
> it do exactly what they want.
I thought open source argument was to write the patch once and the
send it upstream so you only have to do it once... but this is 
not really relevant to this discussion... the "open source argument"
that is...  

> 
> Any admin worth his or her NaCl never relies on default settings.
Which is exactly the point of having a configuration file where
they can easily be manipulate the defaults... 


> 
>>> I think the problem I have with this (besides the complexity) is that if
>>> we must add more ways of avoiding NFSv4, then is NFSv4 really ready to
>>> be made the default?
>> Yes, I see your point... But I do think its not a bad thing to give the
>> admins the final say on how and what they want to run... because they are
>> the ones doing the front line support.. not us..
>>
>> I firmly believe if the transition to V4 is seamless and uneventful,
>> having a configuration file will be non-issue... But if its not, then
>> having a configuration file will be a good thing...
> 
> OK, then, what are we doing to make the transition seamless?  
Please see the Dynamic Pseudo Root discussion on the NFSv4 mailing list.

> Do we know where the pain points are?  
I've only identified the mounting pain points which I'm trying to
address with this configuration file.
 
> Do we have them documented (something that for example Fedora release notes can refer to)?
No, not yet.
  
> Are we giving our users real tools for managing the transition, like tools to convert ACLs?
Yes, I'm trying giving them a configuration file that they use to define the defaults
that work best in their environments. A tool to convert ACLS? Please see nfs4-acl-tools
tar ball on the CITI web page... Remember, patches are always welcome! :-)

> 
> Our defaults should be settings that work well out of the box in most environments.
> If that isn't the case yet for NFSv4, then we shouldn't make it the default.
Believe me.. with all the push back I'm getting from both the client and 
server side... I being to think the Linux community simply does not want 
move forward... but unfortunately I can accept that as option... which 
the the reason I will continue to push for changes that will move us forward... 

steved.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
       [not found]             ` <49C13928.8040806-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2009-03-19 11:13               ` Steve Dickson
       [not found]                 ` <49C228D3.4070005-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
  2009-03-19 16:37                 ` Chuck Lever
  0 siblings, 2 replies; 21+ messages in thread
From: Steve Dickson @ 2009-03-19 11:13 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list



Steve Dickson wrote:
> Chuck Lever wrote:
>> On Mar 18, 2009, at Mar 18, 2009, 9:07 AM, Steve Dickson wrote:
>>>>>> This can become challenging to troubleshoot if there are these
>>>>>> semi-hidden options (cf. selinux).
>>>>> Why? 'mount -v' clearly shows what options are being used and then
>>>>> of course 'cat /proc/mounts' will show all the mount options.
>>>> Because we expect certain default settings for these mount options.  If
>>>> they can be different on every system and every mount point, then you
>>>> have to remember to go back and check /etc/fstab, and your configuration
>>>> file, and so on.
>>> Yes... If you do change something... the default settings may or may
>>> no longer
>>> be in effect... But I have faith that most admins would be able realize
>>> this... If they change something... things are going to change! :-)
>> The point is that these settings are changed so infrequently (or are
>> managed by a team of admins) that it will be too easy to overlook or
>> forget this kind of change.  You are asking administrators to look in
>> one more place when tracking down mount behavior, and I don't see a
>> clear benefit for the added complexity.
> No. I am not asking admins to do ANYTHING! It will be up to them if/when
> to used this configuration file... I'm just giving them an another way to 
> set NFS mounts options... They choose use so be it.. In not that ok too..
> 
>> It seems to me that automounter maps provide exactly this kind of
>> functionality.  Why are they inadequate for this task?  If they are
>> inadequate, why not improve automounter?
> Well, configuring the automounter is a very complex endeavour. Figuring out 
> how to set mount options globally, per server and per mount point is not an 
> easy task but with this configuration file it is.. Plus, to use the auto
> maps you have to use the automounter... There are no such requirement to use
> the configuration file. 
> 
>>>>>> I don't get why the per-mount section is even needed -- currently, the
>>>>>> mount options in /etc/fstab are used automatically if no options are
>>>>>> specified on the command line.
>>>>> Sure... this is option redundant with /etc/fstab but why not allow
>>>>> people
>>>>> configure all the NFS mounts in one spot? Why make them edit different
>>>>> files?
>>>> Indeed... why make them edit /etc/fstab and some other config file?
>>> But the /etc/fstab file can't set global or per server options...
>> Yes, however you can change /etc/fstab with a global editor command.
>>
>>>> We have /etc/fstab for static mounts and maps for automounter.  It's
>>>> already bad enough, and now we are considering another layer of
>>>> complexity.  Plus, the internal syntax of the mount command
>>>> configuration file is unlike either /etc/fstab or automounter maps.
>>> The format follows the already existing /etc/idmap.conf file. I
>>> basically reused the parsing routine that were already in nfs-utils.
>>> And I must admit, they were very well written...
>> Since we attempt to follow the example of other implementations
>> (especially the reference implementation, Solaris) why are we not
>> implementing a config file format that is used by other O/Ses?
> Suggestions are always welcomed... The fact that the parsing code
> already existed in nfs-utils I thought was a plus... 
> 
>> I've looked for a similar facility on Solaris 10, and I don't find it
>> mentioned in either mount(1M) or mount_nfs(1M).
> /etc/default/nfs has a similar look an feel... but it does not the
> same feature set.
> 
>>> While its true the format does not follow either the /etc/fstab file
>>> or the autofs maps, I contend its a format that is much more
>>> straightforward... Basically uncomment things... and again I have
>>> faith in the admins that they will be able to quickly figure it out...
>>> What is really need is a GUI... but that's for another day... ;-)
>> Whether it is straightforward or not, it is a new and additional format
>> that has to be understood.
> This format is not new... It used by a number other configuration files
> in /etc/.... and if there was one standard format for Linux configuration
> files I would adhere to it... But that is simply not the case... 
>  
>> It would make more sense if we were introducing this global config
>> feature for _all_ file systems (via the mount command, not by the
>> subcommands).  A GUI would follow the current trend of system-config-*
>> and many other system utilities, such as NetworkManager.
> I think this would be a bit too ambitious for what I'm trying to do...
> 
>>>>>>> This is the first step toward moving the default NFS version from 3
>>>>>>> to 4
>>>>>>> on the client.
>>>>>> I would think that the _only_ step is to implement the version
>>>>>> fallback
>>>>>> logic; ie. if the server doesn't support NFSv4, then use NFSv3, then
>>>>>> NFSv2.  Why can't an admin simply specify a fixed NFS version
>>>>>> (nfsvers=3) if there is a problem with NFSv4?
>>>>> They can... Nfsvers=3. When that is set, there will be no negotiation.
>>>> That's exactly my point.
>>>>
>>>> If an admin has a problem with using a particular version, they can
>>>> specify exactly what they want, today.
>>> Not globally... they have to change every single mount in both /etc/fstab
>>> and the autofs maps... verse making the change in one place.
>> If an admin cares enough to manage global changes, he will already use
>> automounter and avoid /etc/fstab.
> Again, this give them just another way of doing things...
> 
> 
>>>> People who don't care now probably are not likely to care if they get
>>>> NFSv4,
>>>> as long as NFSv4 is working as well as NFSv3.
>>> This is an assumption I don't think we should make.
>> Why not?  The point of a default setting is that it works well in most
>> cases.  If NFSv4 doesn't work well in most cases, then we have bigger
>> problems.  Otherwise, people who care about the minor differences
>> already have specific settings and configurations to deal with them.
> The key words being "most cases"... With this change the default settings
> can be manipulated to work in all cases... combining both groups you
> are alluding to.  
>  
>>>>>> It seems to me that if the admin hasn't specified nfsvers=, then she
>>>>>> does
>>>>>> not care which NFS version is used.
>>>>> True. And then the highest version the server supports
>>>>> will be negotiated. Having a Max/Min version will allow a the
>>>>> client to control that negotiation... Say the want v4 but not
>>>>> v4.1 ? Or may they only v41?
>>>> If they care that much, why not just say nfsvers=4 and be done with it?
>>> v4 has had years more testing than v4.1.
>> You are suggesting that one cannot choose specifically among NFSv4 minor
>> versions with a mount option.  I truly truly hope this is not the case.
> No. Of course there is a mount option to specify v4 and v4.1. But giving
> admins a way of added one line to one configuration that will either make
> all mounts v4 or v4.1. Something I would thing would be a bit handy... 
> 
>>>>>>> Having a configuration file which allows users to define
>>>>>>> the maximum and minimum NFS versions that should be negotiated is the
>>>>>>> right thing to do... IMHO.. Even though this patch does not does not
>>>>>>> do that, it does lay the ground work for that type of functionality
>>>>> Well I think most admins do want complete control over which NFS
>>>>> version will or will not be used... esp when it comes to v4 and v4.1
>>>> I honestly don't see that.  Admins want the system to work, and not be
>>>> bothered with the trivial details.  They want either "give me anything
>>>> that works" or "give me this specific setting."  And we have the ability
>>>> to do that already.
>>> Yes... But for this part "give me this specific setting" having a way to
>>> configure a setting, globally, per server, or per mount point is a
>>> good thing... imho...
>> No one has demonstrated yet that what we have today cannot provide the
>> same effect as a global setting.  An automounter map provides this
>> functionality.  What problem, exactly, are we trying to address here?
> Complexity... Not having to use the automounter and getting the similar 
> type of configurability is not a bad thing... imho... 
> 
>>>> Even without a config file, Linux is open source.  If an admin cares
>>>> that much about exactly how the system works, she will build it herself,
>>>> and possibly modify the source code.
>>> Please... how many admins do you know that are also kernel
>>> developers... :-)
>> You don't have to be a kernel developer, you just have to be able to
>> read C and run "make".  That's been the whole argument for open source
>> since the very beginning: it enables people to modify the code to make
>> it do exactly what they want.
> I thought open source argument was to write the patch once and the
> send it upstream so you only have to do it once... but this is 
> not really relevant to this discussion... the "open source argument"
> that is...  
> 
>> Any admin worth his or her NaCl never relies on default settings.
> Which is exactly the point of having a configuration file where
> they can easily be manipulate the defaults... 
> 
> 
>>>> I think the problem I have with this (besides the complexity) is that if
>>>> we must add more ways of avoiding NFSv4, then is NFSv4 really ready to
>>>> be made the default?
>>> Yes, I see your point... But I do think its not a bad thing to give the
>>> admins the final say on how and what they want to run... because they are
>>> the ones doing the front line support.. not us..
>>>
>>> I firmly believe if the transition to V4 is seamless and uneventful,
>>> having a configuration file will be non-issue... But if its not, then
>>> having a configuration file will be a good thing...
>> OK, then, what are we doing to make the transition seamless?  
> Please see the Dynamic Pseudo Root discussion on the NFSv4 mailing list.
> 
>> Do we know where the pain points are?  
> I've only identified the mounting pain points which I'm trying to
> address with this configuration file.
>  
>> Do we have them documented (something that for example Fedora release notes can refer to)?
> No, not yet.
>   
>> Are we giving our users real tools for managing the transition, like tools to convert ACLs?
> Yes, I'm trying giving them a configuration file that they use to define the defaults
> that work best in their environments. A tool to convert ACLS? Please see nfs4-acl-tools
> tar ball on the CITI web page... Remember, patches are always welcome! :-)
> 
>> Our defaults should be settings that work well out of the box in most environments.
>> If that isn't the case yet for NFSv4, then we shouldn't make it the default.
> Believe me.. with all the push back I'm getting from both the client and 
> server side... I being to think the Linux community simply does not want 
> move forward... but unfortunately I can accept that as option... which 
> the the reason I will continue to push for changes that will move us forward... 
This comment was uncalled for and I hope it does not imped on the productive
dialogue we were having...    

steved.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
       [not found]                 ` <49C228D3.4070005-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2009-03-19 15:45                   ` J. Bruce Fields
  0 siblings, 0 replies; 21+ messages in thread
From: J. Bruce Fields @ 2009-03-19 15:45 UTC (permalink / raw)
  To: Steve Dickson
  Cc: Chuck Lever, Linux NFS Mailing list, Linux NFSv4 mailing list

On Thu, Mar 19, 2009 at 07:13:23AM -0400, Steve Dickson wrote:
> Steve Dickson wrote:
> > Chuck Lever wrote:
> >> Are we giving our users real tools for managing the transition, like tools to convert ACLs?
> > Yes, I'm trying giving them a configuration file that they use to define the defaults
> > that work best in their environments. A tool to convert ACLS? Please see nfs4-acl-tools
> > tar ball on the CITI web page... Remember, patches are always welcome! :-)
> > 
> >> Our defaults should be settings that work well out of the box in most environments.
> >> If that isn't the case yet for NFSv4, then we shouldn't make it the default.
> > Believe me.. with all the push back I'm getting from both the client and 
> > server side... I being to think the Linux community simply does not want 
> > move forward... but unfortunately I can accept that as option... which 
> > the the reason I will continue to push for changes that will move us forward... 
> This comment was uncalled for and I hope it does not imped on the productive
> dialogue we were having...    

Don't worry about it!  Your persistance is appreciated....

--b.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Patch 0/9] NFS Mount Configuration File
  2009-03-19 11:13               ` Steve Dickson
       [not found]                 ` <49C228D3.4070005-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
@ 2009-03-19 16:37                 ` Chuck Lever
  1 sibling, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2009-03-19 16:37 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Linux NFS Mailing list, Linux NFSv4 mailing list

On Mar 19, 2009, at Mar 19, 2009, 7:13 AM, Steve Dickson wrote:
> Steve Dickson wrote:
>> Chuck Lever wrote:
>>> Our defaults should be settings that work well out of the box in  
>>> most environments.
>>> If that isn't the case yet for NFSv4, then we shouldn't make it  
>>> the default.
>> Believe me.. with all the push back I'm getting from both the  
>> client and
>> server side... I being to think the Linux community simply does not  
>> want
>> move forward... but unfortunately I can accept that as option...  
>> which
>> the the reason I will continue to push for changes that will move  
>> us forward...
> This comment was uncalled for and I hope it does not imped on the  
> productive
> dialogue we were having...

I should have made it clear that I don't have any problem with having  
NFSv4 as the default.  My interest is that our NFSv4 implementation is  
ready for our users when it becomes the default, and our development  
team has a reasonably good understanding of what the problems will  
be.  When NFSv4 becomes the default, I want it to be a step forward.

I currently don't see the benefit a mount config file would add for an  
NFSv4 transition.  It's something we will have to live with for a long  
while, so I think we should take the time and effort to get it right.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2009-03-19 16:37 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-09 20:44 [Patch 0/9] NFS Mount Configuration File Steve Dickson
2009-03-09 20:47 ` [Patch 1/9] Make idmapd's Configuration Parsing Code Available Steve Dickson
2009-03-09 20:50 ` [Patch 2/9] Ignore blanks in section definitions and before assignment statements Steve Dickson
2009-03-09 20:52 ` [Patch 3/9] Ensure configuration values are stored in lower case Steve Dickson
2009-03-09 20:58 ` [Patch 4/9] Mount support routines used to convert mount options Steve Dickson
2009-03-09 21:03 ` [Patch 5/9] Hooks needs incorporate file configuration code Steve Dickson
2009-03-09 21:04 ` [Patch 6/9] An example of an nfsmount.conf file Steve Dickson
2009-03-09 21:06 ` [Patch 7/9] New nfsmount.conf(5) man page Steve Dickson
2009-03-09 21:08 ` [Patch 8/9] Another way to define the configuration file Steve Dickson
2009-03-09 21:10 ` [Patch 9/9] Fixed a couple nits Steve Dickson
2009-03-09 21:49 ` [Patch 0/9] NFS Mount Configuration File Chuck Lever
2009-03-17 19:44   ` Steve Dickson
2009-03-17 20:17     ` Chuck Lever
2009-03-17 20:25       ` J. Bruce Fields
2009-03-17 20:36         ` Chuck Lever
2009-03-18 13:07       ` Steve Dickson
2009-03-18 16:31         ` Chuck Lever
2009-03-18 18:10           ` Steve Dickson
     [not found]             ` <49C13928.8040806-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
2009-03-19 11:13               ` Steve Dickson
     [not found]                 ` <49C228D3.4070005-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
2009-03-19 15:45                   ` J. Bruce Fields
2009-03-19 16:37                 ` Chuck Lever

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.