linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id
@ 2020-07-16 13:51 Alice Mitchell
  2020-07-16 13:53 ` [PATCH v2 1/4] nfs-utils: Factor out common structure cleanup calls Alice Mitchell
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Alice Mitchell @ 2020-07-16 13:51 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Steve Dickson

This patch set introduces some additional features to the nfs.conf tool
chain that allows automatic use of /etc/machine-id or other unique
values for setups that otherwise do not have a unique hostname or disk
image and would thus otherwise generate non-unique EXCHANGE_ID and
SETCLIENTID messages. 

v2 Added man page and documentation updates.

Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>

---

Alice Mitchell (4):
  Factor out common structure cleanup calls
  Enable the retrieval of raw config settings without expansion
  Add support for futher ${variable} expansions in nfs.conf
  Update nfs4_unique_id module parameter from the nfs.conf value

 nfs.conf                      |   1 +
 support/include/conffile.h    |   1 +
 support/nfs/conffile.c        | 375 +++++++++++++++++++++++++++++-----
 systemd/Makefile.am           |   3 +
 systemd/README                |   5 +
 systemd/nfs-conf-export.sh    |  28 +++
 systemd/nfs-config.service.in |  17 ++
 systemd/nfs.conf.man          |  60 +++++-
 tools/nfsconf/nfsconf.man     |  17 +-
 tools/nfsconf/nfsconfcli.c    |  22 +-
 10 files changed, 458 insertions(+), 71 deletions(-)
 create mode 100755 systemd/nfs-conf-export.sh
 create mode 100644 systemd/nfs-config.service.in

-- 
2.18.1



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

* [PATCH v2 1/4] nfs-utils: Factor out common structure cleanup calls
  2020-07-16 13:51 [PATCH v2 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id Alice Mitchell
@ 2020-07-16 13:53 ` Alice Mitchell
  2020-07-16 13:54 ` [PATCH v2 2/4] nfs-utils: Enable the retrieval of raw config settings without Alice Mitchell
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Alice Mitchell @ 2020-07-16 13:53 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Steve Dickson

Factor out common structure cleanup calls

Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
---
 support/nfs/conffile.c | 84 +++++++++++++++++++++---------------------
 1 file changed, 41 insertions(+), 43 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index 3d13610e..aea35917 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -128,6 +128,39 @@ conf_hash(const char *s)
 	return hash;
 }
 
+/*
+ * free all the component parts of a conf_binding struct
+ */
+static void free_confbind(struct conf_binding *cb)
+{
+	if (!cb)
+		return;
+	if (cb->section)
+		free(cb->section);
+	if (cb->arg)
+		free(cb->arg);
+	if (cb->tag)
+		free(cb->tag);
+	if (cb->value)
+		free(cb->value);
+	free(cb);
+}
+
+static void free_conftrans(struct conf_trans *ct)
+{
+	if (!ct)
+		return;
+	if (ct->section)
+		free(ct->section);
+	if (ct->arg)
+		free(ct->arg);
+	if (ct->tag)
+		free(ct->tag);
+	if (ct->value)
+		free(ct->value);
+	free(ct);
+}
+
 /*
  * Insert a tag-value combination from LINE (the equal sign is at POS)
  */
@@ -143,11 +176,7 @@ conf_remove_now(const char *section, const char *tag)
 				&& strcasecmp(cb->tag, tag) == 0) {
 			LIST_REMOVE(cb, link);
 			xlog(LOG_INFO,"[%s]:%s->%s removed", section, tag, cb->value);
-			free(cb->section);
-			free(cb->arg);
-			free(cb->tag);
-			free(cb->value);
-			free(cb);
+			free_confbind(cb);
 			return 0;
 		}
 	}
@@ -167,11 +196,7 @@ conf_remove_section_now(const char *section)
 			unseen = 0;
 			LIST_REMOVE(cb, link);
 			xlog(LOG_INFO, "[%s]:%s->%s removed", section, cb->tag, cb->value);
-			free(cb->section);
-			free(cb->arg);
-			free(cb->tag);
-			free(cb->value);
-			free(cb);
+			free_confbind(cb);
 			}
 		}
 	return unseen;
@@ -567,11 +592,7 @@ static void conf_free_bindings(void)
 		for (; cb; cb = next) {
 			next = LIST_NEXT(cb, link);
 			LIST_REMOVE(cb, link);
-			free(cb->section);
-			free(cb->arg);
-			free(cb->tag);
-			free(cb->value);
-			free(cb);
+			free_confbind(cb);
 		}
 		LIST_INIT(&conf_bindings[i]);
 	}
@@ -635,11 +656,7 @@ conf_cleanup(void)
 	for (node = TAILQ_FIRST(&conf_trans_queue); node; node = next) {
 		next = TAILQ_NEXT(node, link);
 		TAILQ_REMOVE (&conf_trans_queue, node, link);
-		if (node->section) free(node->section);
-		if (node->arg) free(node->arg);
-		if (node->tag) free(node->tag);
-		if (node->value) free(node->value);
-		free (node);
+		free_conftrans(node);
 	}
 	TAILQ_INIT(&conf_trans_queue);
 }
@@ -1005,14 +1022,7 @@ conf_set(int transaction, const char *section, const char *arg,
 	return 0;
 
 fail:
-	if (node->tag)
-		free(node->tag);
-	if (node->arg)
-		free(node->arg);
-	if (node->section)
-		free(node->section);
-	if (node)
-		free(node);
+	free_conftrans(node);
 	return 1;
 }
 
@@ -1038,10 +1048,7 @@ conf_remove(int transaction, const char *section, const char *tag)
 	return 0;
 
 fail:
-	if (node && node->section)
-		free (node->section);
-	if (node)
-		free (node);
+	free_conftrans(node);
 	return 1;
 }
 
@@ -1062,8 +1069,7 @@ conf_remove_section(int transaction, const char *section)
 	return 0;
 
 fail:
-	if (node)
-		free(node);
+	free_conftrans(node);
 	return 1;
 }
 
@@ -1094,15 +1100,7 @@ conf_end(int transaction, int commit)
 				}
 			}
 			TAILQ_REMOVE (&conf_trans_queue, node, link);
-			if (node->section)
-				free(node->section);
-			if (node->arg)
-				free(node->arg);
-			if (node->tag)
-				free(node->tag);
-			if (node->value)
-				free(node->value);
-			free (node);
+			free_conftrans(node);
 		}
 	}
 	return 0;
-- 
2.18.1



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

* [PATCH v2 2/4] nfs-utils: Enable the retrieval of raw config settings without
  2020-07-16 13:51 [PATCH v2 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id Alice Mitchell
  2020-07-16 13:53 ` [PATCH v2 1/4] nfs-utils: Factor out common structure cleanup calls Alice Mitchell
@ 2020-07-16 13:54 ` Alice Mitchell
  2020-07-16 13:55 ` [PATCH v2 3/4] nfs-utils: Add support for futher ${variable} expansions in nfs.conf Alice Mitchell
  2020-07-16 13:56 ` [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value Alice Mitchell
  3 siblings, 0 replies; 12+ messages in thread
From: Alice Mitchell @ 2020-07-16 13:54 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Steve Dickson

Config entries sometimes contain variable expansions, this adds options
to retrieve the config entry rather than its current expanded value.

Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
---
 support/include/conffile.h |  1 +
 support/nfs/conffile.c     | 23 +++++++++++++++++++++++
 tools/nfsconf/nfsconf.man  | 17 +++++++++++++----
 tools/nfsconf/nfsconfcli.c | 22 ++++++++++++++++------
 4 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/support/include/conffile.h b/support/include/conffile.h
index 7d974fe9..c4a3ca62 100644
--- a/support/include/conffile.h
+++ b/support/include/conffile.h
@@ -61,6 +61,7 @@ extern _Bool    conf_get_bool(const char *, const char *, _Bool);
 extern char    *conf_get_str(const char *, const char *);
 extern char    *conf_get_str_with_def(const char *, const char *, char *);
 extern char    *conf_get_section(const char *, const char *, const char *);
+extern char    *conf_get_entry(const char *, const char *, const char *);
 extern int      conf_init_file(const char *);
 extern void     conf_cleanup(void);
 extern int      conf_match_num(const char *, const char *, int);
diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index aea35917..cbeef10d 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -752,6 +752,29 @@ conf_get_str_with_def(const char *section, const char *tag, char *def)
 	return result;
 }
 
+/*
+ * Retrieve an entry without interpreting its contents
+ */
+char *
+conf_get_entry(const char *section, const char *arg, const char *tag)
+{
+	struct conf_binding *cb;
+
+	cb = LIST_FIRST (&conf_bindings[conf_hash (section)]);
+	for (; cb; cb = LIST_NEXT (cb, link)) {
+		if (strcasecmp(section, cb->section) != 0)
+			continue;
+		if (arg && (cb->arg == NULL || strcasecmp(arg, cb->arg) != 0))
+			continue;
+		if (!arg && cb->arg)
+			continue;
+		if (strcasecmp(tag, cb->tag) != 0)
+			continue;
+		return cb->value;
+	}
+	return 0;
+}
+
 /*
  * Find a section that may or may not have an argument
  */
diff --git a/tools/nfsconf/nfsconf.man b/tools/nfsconf/nfsconf.man
index 30791988..26c72a69 100644
--- a/tools/nfsconf/nfsconf.man
+++ b/tools/nfsconf/nfsconf.man
@@ -1,7 +1,7 @@
 .\"
 .\" nfsconf(8)
 .\"
-.TH nfsconf 8 "2 May 2018"
+.TH nfsconf 8 "16 July 2020"
 .SH NAME
 nfsconf \- Query various NFS configuration settings
 .SH SYNOPSIS
@@ -20,6 +20,15 @@ nfsconf \- Query various NFS configuration settings
 .IR section
 .IR tag
 .P
+.B nfsconf \-\-entry
+.RB [ \-v | \-\-verbose ]
+.RB [ \-f | \-\-file
+.IR infile.conf ]
+.RB [ \-a | \-\-arg
+.IR subsection ]
+.IR section
+.IR tag
+.P
 .B nfsconf \-\-isset
 .RB [ \-v | \-\-verbose ]
 .RB [ \-f | \-\-file
@@ -62,6 +71,8 @@ Output an alphabetically sorted dump of the current configuration in conf file f
 Test if a specific tag has a value set.
 .IP "\fB\-g, \-\-get\fP"
 Output the current value of the specified tag.
+.IP "\fB\-e, \-\-entry\fP"
+Output the unmodified value of the specified tag without any variable expansion.
 .IP "\fB\-s, \-\-set\fP"
 Update or Add a tag and value to the config file in a specified section, creating the tag, section, and file if necessary. If the section is defined as '#' then a comment is appended to the file. If a comment is set with a tag name then any exiting tagged comment with a matching name is replaced.
 .IP "\fB\-u, \-\-unset\fP"
@@ -75,7 +86,7 @@ Increase verbosity and print debugging information.
 .B \-f, \-\-file \fIinfile\fR
 Select a different config file to operate upon, default is
 .I /etc/nfs.conf
-.SS Options only valid in \fB\-\-get\fR and \fB\-\-isset\fR modes.
+.SS Options only valid in \fB\-\-get\fR, \fB\-\-entry\fR, and \fB\-\-isset\fR modes.
 .TP
 .B \-a, \-\-arg \fIsubsection\fR
 Select a specific sub-section
@@ -108,5 +119,3 @@ Enable debugging in nfsd
 .BR exportfs (8),
 .BR idmapd (8),
 .BR statd (8)
-.SH AUTHOR
-Justin Mitchell <jumitche@redhat.com>
diff --git a/tools/nfsconf/nfsconfcli.c b/tools/nfsconf/nfsconfcli.c
index 361d386e..b2ef96d1 100644
--- a/tools/nfsconf/nfsconfcli.c
+++ b/tools/nfsconf/nfsconfcli.c
@@ -11,6 +11,7 @@
 typedef enum {
 	MODE_NONE,
 	MODE_GET,
+	MODE_ENTRY,
 	MODE_ISSET,
 	MODE_DUMP,
 	MODE_SET,
@@ -30,6 +31,8 @@ static void usage(const char *name)
 	fprintf(stderr, "      Outputs the configuration to the named file\n");
 	fprintf(stderr, "  --get [--arg subsection] {section} {tag}\n");
 	fprintf(stderr, "      Output one specific config value\n");
+	fprintf(stderr, "  --entry [--arg subsection] {section} {tag}\n");
+	fprintf(stderr, "      Output the uninterpreted config entry\n");
 	fprintf(stderr, "  --isset [--arg subsection] {section} {tag}\n");
 	fprintf(stderr, "      Return code indicates if config value is present\n");
 	fprintf(stderr, "  --set [--arg subsection] {section} {tag} {value}\n");
@@ -55,6 +58,7 @@ int main(int argc, char **argv)
 		int index = 0;
 		struct option long_options[] = {
 			{"get",		no_argument, 0, 'g' },
+			{"entry",	no_argument, 0, 'e' },
 			{"set",		no_argument, 0, 's' },
 			{"unset",	no_argument, 0, 'u' },
 			{"arg",	  required_argument, 0, 'a' },
@@ -66,7 +70,7 @@ int main(int argc, char **argv)
 			{NULL,			  0, 0, 0 }
 		};
 
-		c = getopt_long(argc, argv, "gsua:id::f:vm:", long_options, &index);
+		c = getopt_long(argc, argv, "gesua:id::f:vm:", long_options, &index);
 		if (c == -1) break;
 
 		switch (c) {
@@ -86,6 +90,9 @@ int main(int argc, char **argv)
 			case 'g':
 				mode = MODE_GET;
 				break;
+			case 'e':
+				mode = MODE_ENTRY;
+				break;
 			case 's':
 				mode = MODE_SET;
 				break;
@@ -167,8 +174,8 @@ int main(int argc, char **argv)
 		if (dumpfile)
 			fclose(out);
 	} else
-	/* --iset and --get share a lot of code */
-	if (mode == MODE_GET || mode == MODE_ISSET) {
+	/* --isset and --get share a lot of code */
+	if (mode == MODE_GET || mode == MODE_ISSET || mode == MODE_ENTRY) {
 		char * section = NULL;
 		char * tag = NULL;
 		const char * val;
@@ -186,14 +193,17 @@ int main(int argc, char **argv)
 		tag = argv[optind++];
 
 		/* retrieve the specified tags value */
-		val = conf_get_section(section, arg, tag);
+		if (mode == MODE_ENTRY)
+			val = conf_get_entry(section, arg, tag);
+		else
+			val = conf_get_section(section, arg, tag);
 		if (val != NULL) {
 			/* ret=0, success, mode --get wants to output the value as well */
-			if (mode == MODE_GET)
+			if (mode != MODE_ISSET)
 				printf("%s\n", val);
 		} else {
 			/* ret=1, no value found, tell the user if they asked */
-			if (mode == MODE_GET && verbose)
+			if (mode != MODE_ISSET && verbose)
 				fprintf(stderr, "Tag '%s' not found\n", tag);
 			ret = 1;
 		}
-- 
2.18.1



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

* [PATCH v2 3/4] nfs-utils: Add support for futher ${variable} expansions in nfs.conf
  2020-07-16 13:51 [PATCH v2 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id Alice Mitchell
  2020-07-16 13:53 ` [PATCH v2 1/4] nfs-utils: Factor out common structure cleanup calls Alice Mitchell
  2020-07-16 13:54 ` [PATCH v2 2/4] nfs-utils: Enable the retrieval of raw config settings without Alice Mitchell
@ 2020-07-16 13:55 ` Alice Mitchell
  2020-07-16 13:56 ` [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value Alice Mitchell
  3 siblings, 0 replies; 12+ messages in thread
From: Alice Mitchell @ 2020-07-16 13:55 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Steve Dickson

This adds support for substituting in the systems machine_id as well as random generated
uuid or hostname, and caches the results

Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
---
 support/nfs/conffile.c | 268 +++++++++++++++++++++++++++++++++++++++--
 systemd/nfs.conf.man   |  48 +++++++-
 2 files changed, 299 insertions(+), 17 deletions(-)

diff --git a/support/nfs/conffile.c b/support/nfs/conffile.c
index cbeef10d..58c03911 100644
--- a/support/nfs/conffile.c
+++ b/support/nfs/conffile.c
@@ -40,6 +40,7 @@
 #include <sys/stat.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#include <linux/if_alg.h>
 #include <ctype.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -110,12 +111,66 @@ struct conf_binding {
   char *tag;
   char *value;
   int is_default;
+  char *cache;
 };
 
 LIST_HEAD (conf_bindings, conf_binding) conf_bindings[256];
 
+typedef char * (*expand_fn_t)(void);
+struct expansion_types {
+	const char *name;
+	expand_fn_t func;
+};
+
+typedef struct {
+	uint8_t bytes[16];
+} id128_t;
+
+/*
+ * Application ID for use with generating a machine-id string
+ */
+static id128_t nfs_appid = {.bytes = {0xff,0x3b,0xf0,0x0f,0x34,0xa6,0x43,0xc5, \
+                                       0x93,0xdd,0x16,0xdc,0x7c,0xeb,0x88,0xc8}};
+
 const char *modified_by = NULL;
 
+static __inline__ char
+hexchar(int x) {
+	static const char table[16] = "0123456789abcdef";
+	return table[x & 15];
+}
+
+static __inline__ int
+unhexchar(char h)
+{
+	if (h >= '0' && h <= '9')
+		return h - '0';
+	if (h >= 'a' && h <= 'f')
+		return h - 'a' + 10;
+	if (h >= 'A' && h <= 'F')
+		return h - 'A' + 10;
+	return -1;
+}
+
+static char *
+tohexstr(const unsigned char *data, int len)
+{
+	int i;
+	char *result = NULL;
+
+	result = calloc(1, (len*2)+1);
+	if (!result) {
+		xlog(L_ERROR, "malloc error formatting string");
+		return NULL;
+	}
+
+	for (i = 0; i < len; i++) {
+		result[i*2] = hexchar(data[i] >> 4);
+		result[i*2+1] = hexchar(data[i] & 0x0F);
+	}
+	return result;
+}
+
 static __inline__ uint8_t
 conf_hash(const char *s)
 {
@@ -128,6 +183,201 @@ conf_hash(const char *s)
 	return hash;
 }
 
+static int
+id128_from_string(const char s[], id128_t *ret)
+{
+	id128_t t;
+	unsigned int n, i;
+	for (n=0, i=0; n<16; ) {
+		int a, b;
+		a = unhexchar(s[i++]);
+		if (a < 0)
+			return 1;
+		b = unhexchar(s[i++]);
+		if (b < 0)
+			return 1;
+
+		t.bytes[n++] = (a << 4) | b;
+	}
+	if (s[i] != 0)
+		return 1;
+	if (ret)
+		*ret = t;
+	return 0;
+}
+
+/*
+ * cryptographic hash (sha256) data into a hex encoded string
+ */
+static char *
+strhash(unsigned char *key, size_t keylen, unsigned char *data, size_t dlen)
+{
+	union {
+		struct sockaddr sa;
+		struct sockaddr_alg alg;
+	} sa;
+	int sock = -1;
+	int hfd = -1;
+	uint8_t digest[129];
+	int n;
+	char *result = NULL;
+
+	memset(&sa, 0, sizeof(sa));
+	sa.alg.salg_family = AF_ALG;
+	strcpy((char *)sa.alg.salg_type, "hash");
+	strcpy((char *)sa.alg.salg_name, "hmac(sha256)");
+
+	sock = socket(AF_ALG, SOCK_SEQPACKET|SOCK_CLOEXEC, 0);
+	if (sock < 0) {
+		xlog(L_ERROR, "error creating socket");
+		goto cleanup;
+	}
+
+	if (bind(sock, (struct sockaddr *)&sa.sa, sizeof(sa)) < 0) {
+		xlog(L_ERROR, "error opening khash interface");
+		goto cleanup;
+	}
+
+	if (key && keylen > 0) {
+		if (setsockopt(sock, SOL_ALG, ALG_SET_KEY, key, keylen) < 0) {
+			xlog(L_ERROR, "Error setting key: %s", strerror(errno));
+			goto cleanup;
+		}
+	}
+
+	hfd = accept4(sock, NULL, 0, SOCK_CLOEXEC);
+	if (hfd < 0) {
+		xlog(L_ERROR, "Error initiating khash: %s", strerror(errno));
+		goto cleanup;
+	}
+
+	n = send(hfd, data, dlen, 0);
+	if (n < 0) {
+		xlog(L_ERROR, "Error updating khash: %s", strerror(errno));
+		goto cleanup;
+	}
+
+	n = recv(hfd, digest, sizeof(digest), 0);
+	if (n < 0) {
+		xlog(L_ERROR, "Error fetching khash: %s", strerror(errno));
+		goto cleanup;
+	}
+
+	result = tohexstr(digest, n);
+cleanup:
+	if (sock != -1)
+		close(sock);
+	if (hfd != -1)
+		close(hfd);
+	if (hfd != -1)
+		close(hfd);
+
+	return result;
+}
+
+/*
+ * Read one line of content from a file
+ */
+static char *
+read_oneline(const char *filename)
+{
+	char *content = conf_readfile(filename);
+	char *end;
+
+	if (content == NULL)
+		return NULL;
+
+	/* trim to only the first line */
+	end = strchr(content, '\n');
+	if (end != NULL)
+		*end = '\0';
+	end = strchr(content, '\r');
+	if (end != NULL)
+		*end = '\0';
+
+	return content;
+}
+
+static char *
+expand_machine_id(void)
+{
+	char *key = read_oneline("/etc/machine-id");
+	id128_t mid;
+	char * result = NULL;
+	size_t idlen = 0;
+
+	if (key == NULL)
+		return NULL;
+
+	idlen = strlen(key);
+	if (!id128_from_string(key, &mid)) {
+		result = strhash(mid.bytes, sizeof(mid), nfs_appid.bytes, sizeof(nfs_appid));
+		if (result && strlen(result) > idlen)
+			result[idlen]=0;
+	}
+	free(key);
+	return result;
+}
+
+static char *
+expand_random_uuid(void)
+{
+	return read_oneline("/proc/sys/kernel/random/uuid");
+}
+
+static char *
+expand_hostname(void)
+{
+	int maxlen = HOST_NAME_MAX + 1;
+	char * hostname = calloc(1, maxlen);
+
+	if (!hostname)
+		return NULL;
+	if ((gethostname(hostname, maxlen)) == -1) {
+		free(hostname);
+		return NULL;
+	}
+	return hostname;
+}
+
+static struct expansion_types  var_expansions[] = {
+	{ "machine_id", expand_machine_id },
+	{ "machine-id", expand_machine_id },
+	{ "random-uuid", expand_random_uuid },
+	{ "hostname", expand_hostname },
+};
+
+/* Deal with more complex variable substitutions */
+static char *
+expand_variable(const char *name)
+{
+	size_t len;
+
+	if (name == NULL || name[0] != '$')
+		return NULL;
+
+	len = strlen(name);
+	if (name[1] == '{' && name[len-1] == '}') {
+		char *varname = strndupa(&name[2], len-3);
+
+		for (size_t i=0; i<sizeof(var_expansions); i++) {
+			if (!strcasecmp(varname, var_expansions[i].name)) {
+				return var_expansions[i].func();
+			}
+		}
+		xlog_warn("get_conf: Unknown variable ${%s}", varname);
+	} else {
+		/* expand $name from [environment] section,
+		* or from environment
+		*/
+		char *env = getenv(&name[1]);
+		if (env == NULL || *env == 0)
+			env = conf_get_section("environment", NULL, &name[1]);
+		return env;
+	}
+	return NULL;
+}
+
 /*
  * free all the component parts of a conf_binding struct
  */
@@ -143,6 +393,8 @@ static void free_confbind(struct conf_binding *cb)
 		free(cb->tag);
 	if (cb->value)
 		free(cb->value);
+	if (cb->cache)
+		free(cb->cache);
 	free(cb);
 }
 
@@ -782,7 +1034,7 @@ char *
 conf_get_section(const char *section, const char *arg, const char *tag)
 {
 	struct conf_binding *cb;
-retry:
+
 	cb = LIST_FIRST (&conf_bindings[conf_hash (section)]);
 	for (; cb; cb = LIST_NEXT (cb, link)) {
 		if (strcasecmp(section, cb->section) != 0)
@@ -794,19 +1046,13 @@ retry:
 		if (strcasecmp(tag, cb->tag) != 0)
 			continue;
 		if (cb->value[0] == '$') {
-			/* expand $name from [environment] section,
-			 * or from environment
-			 */
-			char *env = getenv(cb->value+1);
-			if (env && *env)
-				return env;
-			section = "environment";
-			tag = cb->value + 1;
-			goto retry;
+			if (!cb->cache)
+				cb->cache = expand_variable(cb->value);
+			return cb->cache;
 		}
 		return cb->value;
 	}
-	return 0;
+	return NULL;
 }
 
 /*
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index 3f1c7261..28dbaa99 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -45,13 +45,13 @@ or
 .RB \*(lq ; \*(rq
 is ignored, as is any blank line.
 .PP
-If the assigned value started with a
+If the value to be assigned starts with a
 .RB \*(lq $ \*(rq
-then the remainder is treated as a name and looked for in the section
-.B [environment]
-or in the processes environment (see
-.BR environ (7)).
-The value found is used for this value.
+then it will be substituted with a variable expansion
+as detailed in The
+.SM
+.B EXPANSIONS
+section.
 .PP
 The value name
 .B include
@@ -264,6 +264,42 @@ Only
 .B debug=
 is recognized.
 
+.SH EXPANSIONS
+.PP
+Assigned values which begin with
+.RB \*(lq $ \*(rq
+are variable expansions and will be substituted with
+a value determined by one of the following methods.
+.PP
+Values which are of the form
+.RB \*(lq $word \*(rq
+will first be looked for in the
+.B [environment]
+section, and if not found will then be looked for in the processes
+environment (see
+.BR environ (7)).
+.PP
+Values which are of the form
+.RB \*(lq ${word} \*(rq
+will be substituted with an appropriate value according to the
+following list. Note that as these values are generated on demand
+it is possible they would vary on subsequent readings.
+.TP
+.B hostname
+Expands to the system hostname as returned by the
+.BR gethostname (2)
+function.
+.TP
+.B random-uuid
+Expands to a randomly generated UUID string obtained from
+.I /proc/sys/kernel/random/uuid
+as defined in
+.BR random (4)
+.TP
+.B machine-id
+This returns the unique machine ID for the local system that has
+been cryptographically hashed to obscure it as detailed in
+.BR machine-id (5)
 .SH FILES
 .I /etc/nfs.conf
 .SH SEE ALSO
-- 
2.18.1



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

* [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-16 13:51 [PATCH v2 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id Alice Mitchell
                   ` (2 preceding siblings ...)
  2020-07-16 13:55 ` [PATCH v2 3/4] nfs-utils: Add support for futher ${variable} expansions in nfs.conf Alice Mitchell
@ 2020-07-16 13:56 ` Alice Mitchell
  2020-07-16 14:02   ` Chuck Lever
  3 siblings, 1 reply; 12+ messages in thread
From: Alice Mitchell @ 2020-07-16 13:56 UTC (permalink / raw)
  To: Linux NFS Mailing list; +Cc: Steve Dickson

This reintroduces the nfs-config.service in order to ensure
that values are taken from nfs.conf and fed to the kernel
module if it is loaded and modprobe.d config incase it is not

Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
---
 nfs.conf                      |  1 +
 systemd/Makefile.am           |  3 +++
 systemd/README                |  5 +++++
 systemd/nfs-conf-export.sh    | 28 ++++++++++++++++++++++++++++
 systemd/nfs-config.service.in | 17 +++++++++++++++++
 systemd/nfs.conf.man          | 12 +++++++++++-
 6 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100755 systemd/nfs-conf-export.sh
 create mode 100644 systemd/nfs-config.service.in

diff --git a/nfs.conf b/nfs.conf
index 186a5b19..8bb41227 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -4,6 +4,7 @@
 #
 [general]
 # pipefs-directory=/var/lib/nfs/rpc_pipefs
+# nfs4_unique_id = ${machine-id}
 #
 [exports]
 # rootdir=/export
diff --git a/systemd/Makefile.am b/systemd/Makefile.am
index 75cdd9f5..51acdc3f 100644
--- a/systemd/Makefile.am
+++ b/systemd/Makefile.am
@@ -9,6 +9,7 @@ unit_files =  \
     nfs-mountd.service \
     nfs-server.service \
     nfs-utils.service \
+    nfs-config.service \
     rpc-statd-notify.service \
     rpc-statd.service \
     \
@@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
 install-data-hook: $(unit_files)
 	mkdir -p $(DESTDIR)/$(unitdir)
 	cp $(unit_files) $(DESTDIR)/$(unitdir)
+	mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
+	install  nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
 endif
diff --git a/systemd/README b/systemd/README
index da23d6f6..56108b10 100644
--- a/systemd/README
+++ b/systemd/README
@@ -28,6 +28,11 @@ by a suitable 'preset' setting:
     If enabled, then blkmapd will be run when nfs-client.target is
     started.
 
+ nfs-config.service
+    Invoked by nfs-client.target to export values from nfs.conf to
+    any kernel modules that require it, such as setting nfs4_unique_id
+    for the nfs client modules
+
 Another special unit is "nfs-utils.service".  This doesn't really do
 anything, but exists so that other units may declare themselves as
 "PartOf" nfs-utils.service.
diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-export.sh
new file mode 100755
index 00000000..486e8df9
--- /dev/null
+++ b/systemd/nfs-conf-export.sh
@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# This script pulls values out of /etc/nfs.conf and configures
+# the appropriate kernel modules which cannot read it directly
+
+NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
+NFSPROBE=/etc/modprobe.d/nfs.conf
+
+# Now read the values from nfs.conf
+MACHINEID=`nfsconf --get general nfs4_unique_id`
+if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
+then
+# No config vaue found, assume blank
+MACHINEID=""
+fi
+
+# Kernel module is already loaded, update the live one
+if [ -e $NFSMOD ]; then
+echo -n "$MACHINEID" >> $NFSMOD
+fi
+
+# Rewrite the modprobe file for next reboot
+echo "# This file is overwritten by systemd nfs-config.service" > $NFSPROBE
+echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
+echo "# Do not hand modify" >> $NFSPROBE
+echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
+
+echo "Set to: $MACHINEID"
diff --git a/systemd/nfs-config.service.in b/systemd/nfs-config.service.in
new file mode 100644
index 00000000..c5ef1024
--- /dev/null
+++ b/systemd/nfs-config.service.in
@@ -0,0 +1,17 @@
+[Unit]
+Description=Preprocess NFS configuration
+PartOf=nfs-client.target
+After=nfs-client.target
+DefaultDependencies=no
+
+[Service]
+Type=oneshot
+# This service needs to run any time any nfs service
+# is started, so changes to local config files get
+# incorporated.  Having "RemainAfterExit=no" (the default)
+# ensures this happens.
+RemainAfterExit=no
+ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
+
+[Install]
+WantedBy=nfs-client.target
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index 28dbaa99..fb9d2dab 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -101,8 +101,11 @@ When a list is given, the members should be comma-separated.
 .TP
 .B general
 Recognized values:
-.BR pipefs-directory .
+.BR pipefs-directory ,
+.BR nfs4_unique_id .
 
+For 
+.BR pipefs-directory
 See
 .BR blkmapd (8),
 .BR rpc.idmapd (8),
@@ -110,6 +113,13 @@ and
 .BR rpc.gssd (8)
 for details.
 
+The
+.BR nfs4_unique_id
+value is used by the NFS4 client when identifying itself to servers and
+can be used to ensure that this value is unique when the local system name
+perhaps is not. For full details please refer to the kernel Documentation
+.I filesystems/nfs/nfs.txt
+
 .TP
 .B exports
 Recognized values:
-- 
2.18.1



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

* Re: [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-16 13:56 ` [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value Alice Mitchell
@ 2020-07-16 14:02   ` Chuck Lever
  2020-07-19 19:57     ` Alice Mitchell
  0 siblings, 1 reply; 12+ messages in thread
From: Chuck Lever @ 2020-07-16 14:02 UTC (permalink / raw)
  To: Alice Mitchell; +Cc: Linux NFS Mailing List, Steve Dickson

Hi Alice-

I agree that selecting a unique nfs4_client_id string is a problem.

However, I thought that Trond is working on a udev-based mechanism
for automatically choosing one that uniquifies containers as well
as stand-alone clients.

I'd prefer if we stuck with one mechanism for doing this rather than
having both.

Is there rationale for having this in nfs.conf instead of being
completely opaque to the administrator? I don't see a compelling
need for an administrator to adjust this if it is truly a random
string of bytes. Do you know of one?


> On Jul 16, 2020, at 9:56 AM, Alice Mitchell <ajmitchell@redhat.com> wrote:
> 
> This reintroduces the nfs-config.service in order to ensure
> that values are taken from nfs.conf and fed to the kernel
> module if it is loaded and modprobe.d config incase it is not
> 
> Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
> ---
> nfs.conf                      |  1 +
> systemd/Makefile.am           |  3 +++
> systemd/README                |  5 +++++
> systemd/nfs-conf-export.sh    | 28 ++++++++++++++++++++++++++++
> systemd/nfs-config.service.in | 17 +++++++++++++++++
> systemd/nfs.conf.man          | 12 +++++++++++-
> 6 files changed, 65 insertions(+), 1 deletion(-)
> create mode 100755 systemd/nfs-conf-export.sh
> create mode 100644 systemd/nfs-config.service.in
> 
> diff --git a/nfs.conf b/nfs.conf
> index 186a5b19..8bb41227 100644
> --- a/nfs.conf
> +++ b/nfs.conf
> @@ -4,6 +4,7 @@
> #
> [general]
> # pipefs-directory=/var/lib/nfs/rpc_pipefs
> +# nfs4_unique_id = ${machine-id}
> #
> [exports]
> # rootdir=/export
> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
> index 75cdd9f5..51acdc3f 100644
> --- a/systemd/Makefile.am
> +++ b/systemd/Makefile.am
> @@ -9,6 +9,7 @@ unit_files =  \
>     nfs-mountd.service \
>     nfs-server.service \
>     nfs-utils.service \
> +    nfs-config.service \
>     rpc-statd-notify.service \
>     rpc-statd.service \
>     \
> @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
> install-data-hook: $(unit_files)
> 	mkdir -p $(DESTDIR)/$(unitdir)
> 	cp $(unit_files) $(DESTDIR)/$(unitdir)
> +	mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
> +	install  nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
> endif
> diff --git a/systemd/README b/systemd/README
> index da23d6f6..56108b10 100644
> --- a/systemd/README
> +++ b/systemd/README
> @@ -28,6 +28,11 @@ by a suitable 'preset' setting:
>     If enabled, then blkmapd will be run when nfs-client.target is
>     started.
> 
> + nfs-config.service
> +    Invoked by nfs-client.target to export values from nfs.conf to
> +    any kernel modules that require it, such as setting nfs4_unique_id
> +    for the nfs client modules
> +
> Another special unit is "nfs-utils.service".  This doesn't really do
> anything, but exists so that other units may declare themselves as
> "PartOf" nfs-utils.service.
> diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-export.sh
> new file mode 100755
> index 00000000..486e8df9
> --- /dev/null
> +++ b/systemd/nfs-conf-export.sh
> @@ -0,0 +1,28 @@
> +#!/bin/bash
> +#
> +# This script pulls values out of /etc/nfs.conf and configures
> +# the appropriate kernel modules which cannot read it directly
> +
> +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
> +NFSPROBE=/etc/modprobe.d/nfs.conf
> +
> +# Now read the values from nfs.conf
> +MACHINEID=`nfsconf --get general nfs4_unique_id`
> +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
> +then
> +# No config vaue found, assume blank
> +MACHINEID=""
> +fi
> +
> +# Kernel module is already loaded, update the live one
> +if [ -e $NFSMOD ]; then
> +echo -n "$MACHINEID" >> $NFSMOD
> +fi
> +
> +# Rewrite the modprobe file for next reboot
> +echo "# This file is overwritten by systemd nfs-config.service" > $NFSPROBE
> +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
> +echo "# Do not hand modify" >> $NFSPROBE
> +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
> +
> +echo "Set to: $MACHINEID"
> diff --git a/systemd/nfs-config.service.in b/systemd/nfs-config.service.in
> new file mode 100644
> index 00000000..c5ef1024
> --- /dev/null
> +++ b/systemd/nfs-config.service.in
> @@ -0,0 +1,17 @@
> +[Unit]
> +Description=Preprocess NFS configuration
> +PartOf=nfs-client.target
> +After=nfs-client.target
> +DefaultDependencies=no
> +
> +[Service]
> +Type=oneshot
> +# This service needs to run any time any nfs service
> +# is started, so changes to local config files get
> +# incorporated.  Having "RemainAfterExit=no" (the default)
> +# ensures this happens.
> +RemainAfterExit=no
> +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
> +
> +[Install]
> +WantedBy=nfs-client.target
> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
> index 28dbaa99..fb9d2dab 100644
> --- a/systemd/nfs.conf.man
> +++ b/systemd/nfs.conf.man
> @@ -101,8 +101,11 @@ When a list is given, the members should be comma-separated.
> .TP
> .B general
> Recognized values:
> -.BR pipefs-directory .
> +.BR pipefs-directory ,
> +.BR nfs4_unique_id .
> 
> +For 
> +.BR pipefs-directory
> See
> .BR blkmapd (8),
> .BR rpc.idmapd (8),
> @@ -110,6 +113,13 @@ and
> .BR rpc.gssd (8)
> for details.
> 
> +The
> +.BR nfs4_unique_id
> +value is used by the NFS4 client when identifying itself to servers and
> +can be used to ensure that this value is unique when the local system name
> +perhaps is not. For full details please refer to the kernel Documentation
> +.I filesystems/nfs/nfs.txt
> +
> .TP
> .B exports
> Recognized values:
> -- 
> 2.18.1
> 
> 

--
Chuck Lever




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

* Re: [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-16 14:02   ` Chuck Lever
@ 2020-07-19 19:57     ` Alice Mitchell
  2020-07-20 12:25       ` Chuck Lever
  2020-07-20 17:54       ` Steve Dickson
  0 siblings, 2 replies; 12+ messages in thread
From: Alice Mitchell @ 2020-07-19 19:57 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List, Steve Dickson

Hi Chuck,
I must have missed the discussion on Trond's work sorry, and I agree
that having it fixed in a way that is both automatic and transparent to
the user is far preferable to the solution I have posted. Do we have
any timeline on this yet ?

My proposed solution would therefore be a stop-gap if required, as it
does not force any specific solution upon the system and merely adds a
few small features in order to assist the administrator if they choose
to make use of the existing kernel module option, in a way which would
preserve the idea of centralised configuration.

As an aside I was also going to propose the use of this same mechanism
to address the issue of the lockd options for port numbers, which as it
currently stands are manually set in both modprobe.d and in nfs.conf,
which as i understand it both must match for successful operation.  A
small addition to the scripts I have posted could see the modules.d
options automatically generated from the nfs.conf options, thus
reducing the scope for mistakes if the administrator chooses to alter
those values and further solidifying the idea of gathering all the
configuration in a single location.

Your thoughts as always are appreciated.

-Alice 

 
On Thu, 2020-07-16 at 10:02 -0400, Chuck Lever wrote:
> Hi Alice-
> 
> I agree that selecting a unique nfs4_client_id string is a problem.
> 
> However, I thought that Trond is working on a udev-based mechanism
> for automatically choosing one that uniquifies containers as well
> as stand-alone clients.
> 
> I'd prefer if we stuck with one mechanism for doing this rather than
> having both.
> 
> Is there rationale for having this in nfs.conf instead of being
> completely opaque to the administrator? I don't see a compelling
> need for an administrator to adjust this if it is truly a random
> string of bytes. Do you know of one?
> 
> 
> > On Jul 16, 2020, at 9:56 AM, Alice Mitchell <ajmitchell@redhat.com>
> > wrote:
> > 
> > This reintroduces the nfs-config.service in order to ensure
> > that values are taken from nfs.conf and fed to the kernel
> > module if it is loaded and modprobe.d config incase it is not
> > 
> > Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
> > ---
> > nfs.conf                      |  1 +
> > systemd/Makefile.am           |  3 +++
> > systemd/README                |  5 +++++
> > systemd/nfs-conf-export.sh    | 28 ++++++++++++++++++++++++++++
> > systemd/nfs-config.service.in | 17 +++++++++++++++++
> > systemd/nfs.conf.man          | 12 +++++++++++-
> > 6 files changed, 65 insertions(+), 1 deletion(-)
> > create mode 100755 systemd/nfs-conf-export.sh
> > create mode 100644 systemd/nfs-config.service.in
> > 
> > diff --git a/nfs.conf b/nfs.conf
> > index 186a5b19..8bb41227 100644
> > --- a/nfs.conf
> > +++ b/nfs.conf
> > @@ -4,6 +4,7 @@
> > #
> > [general]
> > # pipefs-directory=/var/lib/nfs/rpc_pipefs
> > +# nfs4_unique_id = ${machine-id}
> > #
> > [exports]
> > # rootdir=/export
> > diff --git a/systemd/Makefile.am b/systemd/Makefile.am
> > index 75cdd9f5..51acdc3f 100644
> > --- a/systemd/Makefile.am
> > +++ b/systemd/Makefile.am
> > @@ -9,6 +9,7 @@ unit_files =  \
> >     nfs-mountd.service \
> >     nfs-server.service \
> >     nfs-utils.service \
> > +    nfs-config.service \
> >     rpc-statd-notify.service \
> >     rpc-statd.service \
> >     \
> > @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-
> > pipefs-generator
> > install-data-hook: $(unit_files)
> > 	mkdir -p $(DESTDIR)/$(unitdir)
> > 	cp $(unit_files) $(DESTDIR)/$(unitdir)
> > +	mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
> > +	install  nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
> > endif
> > diff --git a/systemd/README b/systemd/README
> > index da23d6f6..56108b10 100644
> > --- a/systemd/README
> > +++ b/systemd/README
> > @@ -28,6 +28,11 @@ by a suitable 'preset' setting:
> >     If enabled, then blkmapd will be run when nfs-client.target is
> >     started.
> > 
> > + nfs-config.service
> > +    Invoked by nfs-client.target to export values from nfs.conf to
> > +    any kernel modules that require it, such as setting
> > nfs4_unique_id
> > +    for the nfs client modules
> > +
> > Another special unit is "nfs-utils.service".  This doesn't really
> > do
> > anything, but exists so that other units may declare themselves as
> > "PartOf" nfs-utils.service.
> > diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-
> > export.sh
> > new file mode 100755
> > index 00000000..486e8df9
> > --- /dev/null
> > +++ b/systemd/nfs-conf-export.sh
> > @@ -0,0 +1,28 @@
> > +#!/bin/bash
> > +#
> > +# This script pulls values out of /etc/nfs.conf and configures
> > +# the appropriate kernel modules which cannot read it directly
> > +
> > +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
> > +NFSPROBE=/etc/modprobe.d/nfs.conf
> > +
> > +# Now read the values from nfs.conf
> > +MACHINEID=`nfsconf --get general nfs4_unique_id`
> > +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
> > +then
> > +# No config vaue found, assume blank
> > +MACHINEID=""
> > +fi
> > +
> > +# Kernel module is already loaded, update the live one
> > +if [ -e $NFSMOD ]; then
> > +echo -n "$MACHINEID" >> $NFSMOD
> > +fi
> > +
> > +# Rewrite the modprobe file for next reboot
> > +echo "# This file is overwritten by systemd nfs-config.service" >
> > $NFSPROBE
> > +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
> > +echo "# Do not hand modify" >> $NFSPROBE
> > +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
> > +
> > +echo "Set to: $MACHINEID"
> > diff --git a/systemd/nfs-config.service.in b/systemd/nfs-
> > config.service.in
> > new file mode 100644
> > index 00000000..c5ef1024
> > --- /dev/null
> > +++ b/systemd/nfs-config.service.in
> > @@ -0,0 +1,17 @@
> > +[Unit]
> > +Description=Preprocess NFS configuration
> > +PartOf=nfs-client.target
> > +After=nfs-client.target
> > +DefaultDependencies=no
> > +
> > +[Service]
> > +Type=oneshot
> > +# This service needs to run any time any nfs service
> > +# is started, so changes to local config files get
> > +# incorporated.  Having "RemainAfterExit=no" (the default)
> > +# ensures this happens.
> > +RemainAfterExit=no
> > +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
> > +
> > +[Install]
> > +WantedBy=nfs-client.target
> > diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
> > index 28dbaa99..fb9d2dab 100644
> > --- a/systemd/nfs.conf.man
> > +++ b/systemd/nfs.conf.man
> > @@ -101,8 +101,11 @@ When a list is given, the members should be
> > comma-separated.
> > .TP
> > .B general
> > Recognized values:
> > -.BR pipefs-directory .
> > +.BR pipefs-directory ,
> > +.BR nfs4_unique_id .
> > 
> > +For 
> > +.BR pipefs-directory
> > See
> > .BR blkmapd (8),
> > .BR rpc.idmapd (8),
> > @@ -110,6 +113,13 @@ and
> > .BR rpc.gssd (8)
> > for details.
> > 
> > +The
> > +.BR nfs4_unique_id
> > +value is used by the NFS4 client when identifying itself to
> > servers and
> > +can be used to ensure that this value is unique when the local
> > system name
> > +perhaps is not. For full details please refer to the kernel
> > Documentation
> > +.I filesystems/nfs/nfs.txt
> > +
> > .TP
> > .B exports
> > Recognized values:
> > -- 
> > 2.18.1
> > 
> > 
> 
> --
> Chuck Lever
> 
> 
> 


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

* Re: [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-19 19:57     ` Alice Mitchell
@ 2020-07-20 12:25       ` Chuck Lever
  2020-07-20 17:54       ` Steve Dickson
  1 sibling, 0 replies; 12+ messages in thread
From: Chuck Lever @ 2020-07-20 12:25 UTC (permalink / raw)
  To: Alice Mitchell, Trond Myklebust; +Cc: Linux NFS Mailing List, Steve Dickson

Hi Alice-

> On Jul 19, 2020, at 3:57 PM, Alice Mitchell <ajmitchell@redhat.com> wrote:
> 
> Hi Chuck,
> I must have missed the discussion on Trond's work sorry, and I agree
> that having it fixed in a way that is both automatic and transparent to
> the user is far preferable to the solution I have posted. Do we have
> any timeline on this yet ?

No timeline, unless Trond has something ready now.


> My proposed solution would therefore be a stop-gap if required, as it
> does not force any specific solution upon the system and merely adds a
> few small features in order to assist the administrator if they choose
> to make use of the existing kernel module option, in a way which would
> preserve the idea of centralised configuration.

My concern is that you are proposing a mechanism that is visible to
administrators that we would need to alter again in the near- to mid-
term. For a stop-gap, a mechanism not visible to administrators would
be easier for us to modify/improve at will.

It looks like this can be containerized by having a unique nfs.conf
for each container. Is that correct?


> As an aside I was also going to propose the use of this same mechanism
> to address the issue of the lockd options for port numbers, which as it
> currently stands are manually set in both modprobe.d and in nfs.conf,
> which as i understand it both must match for successful operation.  A
> small addition to the scripts I have posted could see the modules.d
> options automatically generated from the nfs.conf options, thus
> reducing the scope for mistakes if the administrator chooses to alter
> those values and further solidifying the idea of gathering all the
> configuration in a single location.

Using module parameters means the initramfs needs to be rebuilt after
a parameter has changed, at least on my RHEL-based systems. 

Also, these parameters are not containerized -- the setting would take
effect for all network namespaces.


> Your thoughts as always are appreciated.
> 
> -Alice 
> 
> 
> On Thu, 2020-07-16 at 10:02 -0400, Chuck Lever wrote:
>> Hi Alice-
>> 
>> I agree that selecting a unique nfs4_client_id string is a problem.
>> 
>> However, I thought that Trond is working on a udev-based mechanism
>> for automatically choosing one that uniquifies containers as well
>> as stand-alone clients.
>> 
>> I'd prefer if we stuck with one mechanism for doing this rather than
>> having both.
>> 
>> Is there rationale for having this in nfs.conf instead of being
>> completely opaque to the administrator? I don't see a compelling
>> need for an administrator to adjust this if it is truly a random
>> string of bytes. Do you know of one?
>> 
>> 
>>> On Jul 16, 2020, at 9:56 AM, Alice Mitchell <ajmitchell@redhat.com>
>>> wrote:
>>> 
>>> This reintroduces the nfs-config.service in order to ensure
>>> that values are taken from nfs.conf and fed to the kernel
>>> module if it is loaded and modprobe.d config incase it is not
>>> 
>>> Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
>>> ---
>>> nfs.conf                      |  1 +
>>> systemd/Makefile.am           |  3 +++
>>> systemd/README                |  5 +++++
>>> systemd/nfs-conf-export.sh    | 28 ++++++++++++++++++++++++++++
>>> systemd/nfs-config.service.in | 17 +++++++++++++++++
>>> systemd/nfs.conf.man          | 12 +++++++++++-
>>> 6 files changed, 65 insertions(+), 1 deletion(-)
>>> create mode 100755 systemd/nfs-conf-export.sh
>>> create mode 100644 systemd/nfs-config.service.in
>>> 
>>> diff --git a/nfs.conf b/nfs.conf
>>> index 186a5b19..8bb41227 100644
>>> --- a/nfs.conf
>>> +++ b/nfs.conf
>>> @@ -4,6 +4,7 @@
>>> #
>>> [general]
>>> # pipefs-directory=/var/lib/nfs/rpc_pipefs
>>> +# nfs4_unique_id = ${machine-id}
>>> #
>>> [exports]
>>> # rootdir=/export
>>> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
>>> index 75cdd9f5..51acdc3f 100644
>>> --- a/systemd/Makefile.am
>>> +++ b/systemd/Makefile.am
>>> @@ -9,6 +9,7 @@ unit_files =  \
>>>    nfs-mountd.service \
>>>    nfs-server.service \
>>>    nfs-utils.service \
>>> +    nfs-config.service \
>>>    rpc-statd-notify.service \
>>>    rpc-statd.service \
>>>    \
>>> @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-
>>> pipefs-generator
>>> install-data-hook: $(unit_files)
>>> 	mkdir -p $(DESTDIR)/$(unitdir)
>>> 	cp $(unit_files) $(DESTDIR)/$(unitdir)
>>> +	mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
>>> +	install  nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
>>> endif
>>> diff --git a/systemd/README b/systemd/README
>>> index da23d6f6..56108b10 100644
>>> --- a/systemd/README
>>> +++ b/systemd/README
>>> @@ -28,6 +28,11 @@ by a suitable 'preset' setting:
>>>    If enabled, then blkmapd will be run when nfs-client.target is
>>>    started.
>>> 
>>> + nfs-config.service
>>> +    Invoked by nfs-client.target to export values from nfs.conf to
>>> +    any kernel modules that require it, such as setting
>>> nfs4_unique_id
>>> +    for the nfs client modules
>>> +
>>> Another special unit is "nfs-utils.service".  This doesn't really
>>> do
>>> anything, but exists so that other units may declare themselves as
>>> "PartOf" nfs-utils.service.
>>> diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-
>>> export.sh
>>> new file mode 100755
>>> index 00000000..486e8df9
>>> --- /dev/null
>>> +++ b/systemd/nfs-conf-export.sh
>>> @@ -0,0 +1,28 @@
>>> +#!/bin/bash
>>> +#
>>> +# This script pulls values out of /etc/nfs.conf and configures
>>> +# the appropriate kernel modules which cannot read it directly
>>> +
>>> +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
>>> +NFSPROBE=/etc/modprobe.d/nfs.conf
>>> +
>>> +# Now read the values from nfs.conf
>>> +MACHINEID=`nfsconf --get general nfs4_unique_id`
>>> +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
>>> +then
>>> +# No config vaue found, assume blank
>>> +MACHINEID=""
>>> +fi
>>> +
>>> +# Kernel module is already loaded, update the live one
>>> +if [ -e $NFSMOD ]; then
>>> +echo -n "$MACHINEID" >> $NFSMOD
>>> +fi
>>> +
>>> +# Rewrite the modprobe file for next reboot
>>> +echo "# This file is overwritten by systemd nfs-config.service" >
>>> $NFSPROBE
>>> +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
>>> +echo "# Do not hand modify" >> $NFSPROBE
>>> +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
>>> +
>>> +echo "Set to: $MACHINEID"
>>> diff --git a/systemd/nfs-config.service.in b/systemd/nfs-
>>> config.service.in
>>> new file mode 100644
>>> index 00000000..c5ef1024
>>> --- /dev/null
>>> +++ b/systemd/nfs-config.service.in
>>> @@ -0,0 +1,17 @@
>>> +[Unit]
>>> +Description=Preprocess NFS configuration
>>> +PartOf=nfs-client.target
>>> +After=nfs-client.target
>>> +DefaultDependencies=no
>>> +
>>> +[Service]
>>> +Type=oneshot
>>> +# This service needs to run any time any nfs service
>>> +# is started, so changes to local config files get
>>> +# incorporated.  Having "RemainAfterExit=no" (the default)
>>> +# ensures this happens.
>>> +RemainAfterExit=no
>>> +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
>>> +
>>> +[Install]
>>> +WantedBy=nfs-client.target
>>> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
>>> index 28dbaa99..fb9d2dab 100644
>>> --- a/systemd/nfs.conf.man
>>> +++ b/systemd/nfs.conf.man
>>> @@ -101,8 +101,11 @@ When a list is given, the members should be
>>> comma-separated.
>>> .TP
>>> .B general
>>> Recognized values:
>>> -.BR pipefs-directory .
>>> +.BR pipefs-directory ,
>>> +.BR nfs4_unique_id .
>>> 
>>> +For 
>>> +.BR pipefs-directory
>>> See
>>> .BR blkmapd (8),
>>> .BR rpc.idmapd (8),
>>> @@ -110,6 +113,13 @@ and
>>> .BR rpc.gssd (8)
>>> for details.
>>> 
>>> +The
>>> +.BR nfs4_unique_id
>>> +value is used by the NFS4 client when identifying itself to
>>> servers and
>>> +can be used to ensure that this value is unique when the local
>>> system name
>>> +perhaps is not. For full details please refer to the kernel
>>> Documentation
>>> +.I filesystems/nfs/nfs.txt
>>> +
>>> .TP
>>> .B exports
>>> Recognized values:
>>> -- 
>>> 2.18.1
>>> 
>>> 
>> 
>> --
>> Chuck Lever
>> 
>> 
>> 
> 

--
Chuck Lever




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

* Re: [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-19 19:57     ` Alice Mitchell
  2020-07-20 12:25       ` Chuck Lever
@ 2020-07-20 17:54       ` Steve Dickson
  2020-07-20 18:23         ` Chuck Lever
  1 sibling, 1 reply; 12+ messages in thread
From: Steve Dickson @ 2020-07-20 17:54 UTC (permalink / raw)
  To: Alice Mitchell, Chuck Lever; +Cc: Linux NFS Mailing List

Hello,

On 7/19/20 3:57 PM, Alice Mitchell wrote:
> Hi Chuck,
> I must have missed the discussion on Trond's work sorry, and I agree
> that having it fixed in a way that is both automatic and transparent to
> the user is far preferable to the solution I have posted. Do we have
> any timeline on this yet ?
I too did missed  the discussion... Chuck or Trond can you give us more 
context on how this is going to work automatically and transparent?
Is there a thread you can point us to?

> 
> My proposed solution would therefore be a stop-gap if required, as it
> does not force any specific solution upon the system and merely adds a
> few small features in order to assist the administrator if they choose
> to make use of the existing kernel module option, in a way which would
> preserve the idea of centralised configuration.
I think I agree with Chuck... Once we add something to a 
configuration file it is awful hard to back it out... 

I'm not against setting it in nfs.conf... But if it can be
set easier from the kernel... so be it!
 
> 
> As an aside I was also going to propose the use of this same mechanism
> to address the issue of the lockd options for port numbers, which as it
> currently stands are manually set in both modprobe.d and in nfs.conf,
> which as i understand it both must match for successful operation.  A
> small addition to the scripts I have posted could see the modules.d
> options automatically generated from the nfs.conf options, thus
> reducing the scope for mistakes if the administrator chooses to alter
> those values and further solidifying the idea of gathering all the
> configuration in a single location.
I kinda like the idea to be able to set lockd ports from
nfs.conf. We've done that in the past which was lost
when we moved systemd. 

steved.

> 
> Your thoughts as always are appreciated.
> 
> -Alice 
> 
>  
> On Thu, 2020-07-16 at 10:02 -0400, Chuck Lever wrote:
>> Hi Alice-
>>
>> I agree that selecting a unique nfs4_client_id string is a problem.
>>
>> However, I thought that Trond is working on a udev-based mechanism
>> for automatically choosing one that uniquifies containers as well
>> as stand-alone clients.
>>
>> I'd prefer if we stuck with one mechanism for doing this rather than
>> having both.
>>
>> Is there rationale for having this in nfs.conf instead of being
>> completely opaque to the administrator? I don't see a compelling
>> need for an administrator to adjust this if it is truly a random
>> string of bytes. Do you know of one?
>>
>>
>>> On Jul 16, 2020, at 9:56 AM, Alice Mitchell <ajmitchell@redhat.com>
>>> wrote:
>>>
>>> This reintroduces the nfs-config.service in order to ensure
>>> that values are taken from nfs.conf and fed to the kernel
>>> module if it is loaded and modprobe.d config incase it is not
>>>
>>> Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
>>> ---
>>> nfs.conf                      |  1 +
>>> systemd/Makefile.am           |  3 +++
>>> systemd/README                |  5 +++++
>>> systemd/nfs-conf-export.sh    | 28 ++++++++++++++++++++++++++++
>>> systemd/nfs-config.service.in | 17 +++++++++++++++++
>>> systemd/nfs.conf.man          | 12 +++++++++++-
>>> 6 files changed, 65 insertions(+), 1 deletion(-)
>>> create mode 100755 systemd/nfs-conf-export.sh
>>> create mode 100644 systemd/nfs-config.service.in
>>>
>>> diff --git a/nfs.conf b/nfs.conf
>>> index 186a5b19..8bb41227 100644
>>> --- a/nfs.conf
>>> +++ b/nfs.conf
>>> @@ -4,6 +4,7 @@
>>> #
>>> [general]
>>> # pipefs-directory=/var/lib/nfs/rpc_pipefs
>>> +# nfs4_unique_id = ${machine-id}
>>> #
>>> [exports]
>>> # rootdir=/export
>>> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
>>> index 75cdd9f5..51acdc3f 100644
>>> --- a/systemd/Makefile.am
>>> +++ b/systemd/Makefile.am
>>> @@ -9,6 +9,7 @@ unit_files =  \
>>>     nfs-mountd.service \
>>>     nfs-server.service \
>>>     nfs-utils.service \
>>> +    nfs-config.service \
>>>     rpc-statd-notify.service \
>>>     rpc-statd.service \
>>>     \
>>> @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-
>>> pipefs-generator
>>> install-data-hook: $(unit_files)
>>> 	mkdir -p $(DESTDIR)/$(unitdir)
>>> 	cp $(unit_files) $(DESTDIR)/$(unitdir)
>>> +	mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
>>> +	install  nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
>>> endif
>>> diff --git a/systemd/README b/systemd/README
>>> index da23d6f6..56108b10 100644
>>> --- a/systemd/README
>>> +++ b/systemd/README
>>> @@ -28,6 +28,11 @@ by a suitable 'preset' setting:
>>>     If enabled, then blkmapd will be run when nfs-client.target is
>>>     started.
>>>
>>> + nfs-config.service
>>> +    Invoked by nfs-client.target to export values from nfs.conf to
>>> +    any kernel modules that require it, such as setting
>>> nfs4_unique_id
>>> +    for the nfs client modules
>>> +
>>> Another special unit is "nfs-utils.service".  This doesn't really
>>> do
>>> anything, but exists so that other units may declare themselves as
>>> "PartOf" nfs-utils.service.
>>> diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-
>>> export.sh
>>> new file mode 100755
>>> index 00000000..486e8df9
>>> --- /dev/null
>>> +++ b/systemd/nfs-conf-export.sh
>>> @@ -0,0 +1,28 @@
>>> +#!/bin/bash
>>> +#
>>> +# This script pulls values out of /etc/nfs.conf and configures
>>> +# the appropriate kernel modules which cannot read it directly
>>> +
>>> +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
>>> +NFSPROBE=/etc/modprobe.d/nfs.conf
>>> +
>>> +# Now read the values from nfs.conf
>>> +MACHINEID=`nfsconf --get general nfs4_unique_id`
>>> +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
>>> +then
>>> +# No config vaue found, assume blank
>>> +MACHINEID=""
>>> +fi
>>> +
>>> +# Kernel module is already loaded, update the live one
>>> +if [ -e $NFSMOD ]; then
>>> +echo -n "$MACHINEID" >> $NFSMOD
>>> +fi
>>> +
>>> +# Rewrite the modprobe file for next reboot
>>> +echo "# This file is overwritten by systemd nfs-config.service" >
>>> $NFSPROBE
>>> +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
>>> +echo "# Do not hand modify" >> $NFSPROBE
>>> +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
>>> +
>>> +echo "Set to: $MACHINEID"
>>> diff --git a/systemd/nfs-config.service.in b/systemd/nfs-
>>> config.service.in
>>> new file mode 100644
>>> index 00000000..c5ef1024
>>> --- /dev/null
>>> +++ b/systemd/nfs-config.service.in
>>> @@ -0,0 +1,17 @@
>>> +[Unit]
>>> +Description=Preprocess NFS configuration
>>> +PartOf=nfs-client.target
>>> +After=nfs-client.target
>>> +DefaultDependencies=no
>>> +
>>> +[Service]
>>> +Type=oneshot
>>> +# This service needs to run any time any nfs service
>>> +# is started, so changes to local config files get
>>> +# incorporated.  Having "RemainAfterExit=no" (the default)
>>> +# ensures this happens.
>>> +RemainAfterExit=no
>>> +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
>>> +
>>> +[Install]
>>> +WantedBy=nfs-client.target
>>> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
>>> index 28dbaa99..fb9d2dab 100644
>>> --- a/systemd/nfs.conf.man
>>> +++ b/systemd/nfs.conf.man
>>> @@ -101,8 +101,11 @@ When a list is given, the members should be
>>> comma-separated.
>>> .TP
>>> .B general
>>> Recognized values:
>>> -.BR pipefs-directory .
>>> +.BR pipefs-directory ,
>>> +.BR nfs4_unique_id .
>>>
>>> +For 
>>> +.BR pipefs-directory
>>> See
>>> .BR blkmapd (8),
>>> .BR rpc.idmapd (8),
>>> @@ -110,6 +113,13 @@ and
>>> .BR rpc.gssd (8)
>>> for details.
>>>
>>> +The
>>> +.BR nfs4_unique_id
>>> +value is used by the NFS4 client when identifying itself to
>>> servers and
>>> +can be used to ensure that this value is unique when the local
>>> system name
>>> +perhaps is not. For full details please refer to the kernel
>>> Documentation
>>> +.I filesystems/nfs/nfs.txt
>>> +
>>> .TP
>>> .B exports
>>> Recognized values:
>>> -- 
>>> 2.18.1
>>>
>>>
>>
>> --
>> Chuck Lever
>>
>>
>>
> 


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

* Re: [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-20 17:54       ` Steve Dickson
@ 2020-07-20 18:23         ` Chuck Lever
  2020-07-21 15:20           ` Steve Dickson
  0 siblings, 1 reply; 12+ messages in thread
From: Chuck Lever @ 2020-07-20 18:23 UTC (permalink / raw)
  To: Steve Dickson; +Cc: Alice Mitchell, Linux NFS Mailing List



> On Jul 20, 2020, at 1:54 PM, Steve Dickson <SteveD@RedHat.com> wrote:
> 
> Hello,
> 
> On 7/19/20 3:57 PM, Alice Mitchell wrote:
>> Hi Chuck,
>> I must have missed the discussion on Trond's work sorry, and I agree
>> that having it fixed in a way that is both automatic and transparent to
>> the user is far preferable to the solution I have posted. Do we have
>> any timeline on this yet ?
> I too did missed  the discussion... Chuck or Trond can you give us more 
> context on how this is going to work automatically and transparent?
> Is there a thread you can point us to?

https://lore.kernel.org/linux-nfs/20190611180832.119488-1-trond.myklebust@hammerspace.com/


>> My proposed solution would therefore be a stop-gap if required, as it
>> does not force any specific solution upon the system and merely adds a
>> few small features in order to assist the administrator if they choose
>> to make use of the existing kernel module option, in a way which would
>> preserve the idea of centralised configuration.
> I think I agree with Chuck... Once we add something to a 
> configuration file it is awful hard to back it out... 
> 
> I'm not against setting it in nfs.conf... But if it can be
> set easier from the kernel... so be it!
> 
>> 
>> As an aside I was also going to propose the use of this same mechanism
>> to address the issue of the lockd options for port numbers, which as it
>> currently stands are manually set in both modprobe.d and in nfs.conf,
>> which as i understand it both must match for successful operation.  A
>> small addition to the scripts I have posted could see the modules.d
>> options automatically generated from the nfs.conf options, thus
>> reducing the scope for mistakes if the administrator chooses to alter
>> those values and further solidifying the idea of gathering all the
>> configuration in a single location.
> I kinda like the idea to be able to set lockd ports from
> nfs.conf. We've done that in the past which was lost
> when we moved systemd. 
> 
> steved.
> 
>> 
>> Your thoughts as always are appreciated.
>> 
>> -Alice 
>> 
>> 
>> On Thu, 2020-07-16 at 10:02 -0400, Chuck Lever wrote:
>>> Hi Alice-
>>> 
>>> I agree that selecting a unique nfs4_client_id string is a problem.
>>> 
>>> However, I thought that Trond is working on a udev-based mechanism
>>> for automatically choosing one that uniquifies containers as well
>>> as stand-alone clients.
>>> 
>>> I'd prefer if we stuck with one mechanism for doing this rather than
>>> having both.
>>> 
>>> Is there rationale for having this in nfs.conf instead of being
>>> completely opaque to the administrator? I don't see a compelling
>>> need for an administrator to adjust this if it is truly a random
>>> string of bytes. Do you know of one?
>>> 
>>> 
>>>> On Jul 16, 2020, at 9:56 AM, Alice Mitchell <ajmitchell@redhat.com>
>>>> wrote:
>>>> 
>>>> This reintroduces the nfs-config.service in order to ensure
>>>> that values are taken from nfs.conf and fed to the kernel
>>>> module if it is loaded and modprobe.d config incase it is not
>>>> 
>>>> Signed-off-by: Alice Mitchell <ajmitchell@redhat.com>
>>>> ---
>>>> nfs.conf                      |  1 +
>>>> systemd/Makefile.am           |  3 +++
>>>> systemd/README                |  5 +++++
>>>> systemd/nfs-conf-export.sh    | 28 ++++++++++++++++++++++++++++
>>>> systemd/nfs-config.service.in | 17 +++++++++++++++++
>>>> systemd/nfs.conf.man          | 12 +++++++++++-
>>>> 6 files changed, 65 insertions(+), 1 deletion(-)
>>>> create mode 100755 systemd/nfs-conf-export.sh
>>>> create mode 100644 systemd/nfs-config.service.in
>>>> 
>>>> diff --git a/nfs.conf b/nfs.conf
>>>> index 186a5b19..8bb41227 100644
>>>> --- a/nfs.conf
>>>> +++ b/nfs.conf
>>>> @@ -4,6 +4,7 @@
>>>> #
>>>> [general]
>>>> # pipefs-directory=/var/lib/nfs/rpc_pipefs
>>>> +# nfs4_unique_id = ${machine-id}
>>>> #
>>>> [exports]
>>>> # rootdir=/export
>>>> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
>>>> index 75cdd9f5..51acdc3f 100644
>>>> --- a/systemd/Makefile.am
>>>> +++ b/systemd/Makefile.am
>>>> @@ -9,6 +9,7 @@ unit_files =  \
>>>>    nfs-mountd.service \
>>>>    nfs-server.service \
>>>>    nfs-utils.service \
>>>> +    nfs-config.service \
>>>>    rpc-statd-notify.service \
>>>>    rpc-statd.service \
>>>>    \
>>>> @@ -69,4 +70,6 @@ genexec_PROGRAMS = nfs-server-generator rpc-
>>>> pipefs-generator
>>>> install-data-hook: $(unit_files)
>>>> 	mkdir -p $(DESTDIR)/$(unitdir)
>>>> 	cp $(unit_files) $(DESTDIR)/$(unitdir)
>>>> +	mkdir -p $(DESTDIR)/$(libexecdir)/nfs-utils
>>>> +	install  nfs-conf-export.sh $(DESTDIR)/$(libexecdir)/nfs-utils/
>>>> endif
>>>> diff --git a/systemd/README b/systemd/README
>>>> index da23d6f6..56108b10 100644
>>>> --- a/systemd/README
>>>> +++ b/systemd/README
>>>> @@ -28,6 +28,11 @@ by a suitable 'preset' setting:
>>>>    If enabled, then blkmapd will be run when nfs-client.target is
>>>>    started.
>>>> 
>>>> + nfs-config.service
>>>> +    Invoked by nfs-client.target to export values from nfs.conf to
>>>> +    any kernel modules that require it, such as setting
>>>> nfs4_unique_id
>>>> +    for the nfs client modules
>>>> +
>>>> Another special unit is "nfs-utils.service".  This doesn't really
>>>> do
>>>> anything, but exists so that other units may declare themselves as
>>>> "PartOf" nfs-utils.service.
>>>> diff --git a/systemd/nfs-conf-export.sh b/systemd/nfs-conf-
>>>> export.sh
>>>> new file mode 100755
>>>> index 00000000..486e8df9
>>>> --- /dev/null
>>>> +++ b/systemd/nfs-conf-export.sh
>>>> @@ -0,0 +1,28 @@
>>>> +#!/bin/bash
>>>> +#
>>>> +# This script pulls values out of /etc/nfs.conf and configures
>>>> +# the appropriate kernel modules which cannot read it directly
>>>> +
>>>> +NFSMOD=/sys/module/nfs/parameters/nfs4_unique_id
>>>> +NFSPROBE=/etc/modprobe.d/nfs.conf
>>>> +
>>>> +# Now read the values from nfs.conf
>>>> +MACHINEID=`nfsconf --get general nfs4_unique_id`
>>>> +if [ $? -ne 0 ] || [ "$MACHINEID" == "" ]
>>>> +then
>>>> +# No config vaue found, assume blank
>>>> +MACHINEID=""
>>>> +fi
>>>> +
>>>> +# Kernel module is already loaded, update the live one
>>>> +if [ -e $NFSMOD ]; then
>>>> +echo -n "$MACHINEID" >> $NFSMOD
>>>> +fi
>>>> +
>>>> +# Rewrite the modprobe file for next reboot
>>>> +echo "# This file is overwritten by systemd nfs-config.service" >
>>>> $NFSPROBE
>>>> +echo "# with values taken from /etc/nfs.conf" >> $NFSPROBE
>>>> +echo "# Do not hand modify" >> $NFSPROBE
>>>> +echo "options nfs nfs4_unique_id=\"$MACHINEID\"" >> $NFSPROBE
>>>> +
>>>> +echo "Set to: $MACHINEID"
>>>> diff --git a/systemd/nfs-config.service.in b/systemd/nfs-
>>>> config.service.in
>>>> new file mode 100644
>>>> index 00000000..c5ef1024
>>>> --- /dev/null
>>>> +++ b/systemd/nfs-config.service.in
>>>> @@ -0,0 +1,17 @@
>>>> +[Unit]
>>>> +Description=Preprocess NFS configuration
>>>> +PartOf=nfs-client.target
>>>> +After=nfs-client.target
>>>> +DefaultDependencies=no
>>>> +
>>>> +[Service]
>>>> +Type=oneshot
>>>> +# This service needs to run any time any nfs service
>>>> +# is started, so changes to local config files get
>>>> +# incorporated.  Having "RemainAfterExit=no" (the default)
>>>> +# ensures this happens.
>>>> +RemainAfterExit=no
>>>> +ExecStart=@_libexecdir@/nfs-utils/nfs-conf-export.sh
>>>> +
>>>> +[Install]
>>>> +WantedBy=nfs-client.target
>>>> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
>>>> index 28dbaa99..fb9d2dab 100644
>>>> --- a/systemd/nfs.conf.man
>>>> +++ b/systemd/nfs.conf.man
>>>> @@ -101,8 +101,11 @@ When a list is given, the members should be
>>>> comma-separated.
>>>> .TP
>>>> .B general
>>>> Recognized values:
>>>> -.BR pipefs-directory .
>>>> +.BR pipefs-directory ,
>>>> +.BR nfs4_unique_id .
>>>> 
>>>> +For 
>>>> +.BR pipefs-directory
>>>> See
>>>> .BR blkmapd (8),
>>>> .BR rpc.idmapd (8),
>>>> @@ -110,6 +113,13 @@ and
>>>> .BR rpc.gssd (8)
>>>> for details.
>>>> 
>>>> +The
>>>> +.BR nfs4_unique_id
>>>> +value is used by the NFS4 client when identifying itself to
>>>> servers and
>>>> +can be used to ensure that this value is unique when the local
>>>> system name
>>>> +perhaps is not. For full details please refer to the kernel
>>>> Documentation
>>>> +.I filesystems/nfs/nfs.txt
>>>> +
>>>> .TP
>>>> .B exports
>>>> Recognized values:
>>>> -- 
>>>> 2.18.1
>>>> 
>>>> 
>>> 
>>> --
>>> Chuck Lever
>>> 
>>> 
>>> 
>> 
> 

--
Chuck Lever




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

* Re: [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-20 18:23         ` Chuck Lever
@ 2020-07-21 15:20           ` Steve Dickson
  2020-07-21 15:23             ` Chuck Lever
  0 siblings, 1 reply; 12+ messages in thread
From: Steve Dickson @ 2020-07-21 15:20 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Alice Mitchell, Linux NFS Mailing List

Hey!

On 7/20/20 2:23 PM, Chuck Lever wrote:
> 
> 
>> On Jul 20, 2020, at 1:54 PM, Steve Dickson <SteveD@RedHat.com> wrote:
>>
>> Hello,
>>
>> On 7/19/20 3:57 PM, Alice Mitchell wrote:
>>> Hi Chuck,
>>> I must have missed the discussion on Trond's work sorry, and I agree
>>> that having it fixed in a way that is both automatic and transparent to
>>> the user is far preferable to the solution I have posted. Do we have
>>> any timeline on this yet ?
>> I too did missed  the discussion... Chuck or Trond can you give us more 
>> context on how this is going to work automatically and transparent?
>> Is there a thread you can point us to?
> 
> https://lore.kernel.org/linux-nfs/20190611180832.119488-1-trond.myklebust@hammerspace.com/
Thanks! That is the kernel support but you mentioned something
about a udev-based mechanism for automation... What the story
on that? 

steved.


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

* Re: [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value
  2020-07-21 15:20           ` Steve Dickson
@ 2020-07-21 15:23             ` Chuck Lever
  0 siblings, 0 replies; 12+ messages in thread
From: Chuck Lever @ 2020-07-21 15:23 UTC (permalink / raw)
  To: Steve Dickson, Trond Myklebust; +Cc: Alice Mitchell, Linux NFS Mailing List



> On Jul 21, 2020, at 11:20 AM, Steve Dickson <SteveD@RedHat.com> wrote:
> 
> Hey!
> 
> On 7/20/20 2:23 PM, Chuck Lever wrote:
>> 
>> 
>>> On Jul 20, 2020, at 1:54 PM, Steve Dickson <SteveD@RedHat.com> wrote:
>>> 
>>> Hello,
>>> 
>>> On 7/19/20 3:57 PM, Alice Mitchell wrote:
>>>> Hi Chuck,
>>>> I must have missed the discussion on Trond's work sorry, and I agree
>>>> that having it fixed in a way that is both automatic and transparent to
>>>> the user is far preferable to the solution I have posted. Do we have
>>>> any timeline on this yet ?
>>> I too did missed  the discussion... Chuck or Trond can you give us more 
>>> context on how this is going to work automatically and transparent?
>>> Is there a thread you can point us to?
>> 
>> https://lore.kernel.org/linux-nfs/20190611180832.119488-1-trond.myklebust@hammerspace.com/
> Thanks! That is the kernel support but you mentioned something
> about a udev-based mechanism for automation... What the story
> on that?

Trond is working on that.


--
Chuck Lever




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

end of thread, other threads:[~2020-07-21 15:23 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 13:51 [PATCH v2 0/4] nfs-utils: nfs.conf features to enable use of machine-id as nfs4_unique_id Alice Mitchell
2020-07-16 13:53 ` [PATCH v2 1/4] nfs-utils: Factor out common structure cleanup calls Alice Mitchell
2020-07-16 13:54 ` [PATCH v2 2/4] nfs-utils: Enable the retrieval of raw config settings without Alice Mitchell
2020-07-16 13:55 ` [PATCH v2 3/4] nfs-utils: Add support for futher ${variable} expansions in nfs.conf Alice Mitchell
2020-07-16 13:56 ` [PATCH v2 4/4] nfs-utils: Update nfs4_unique_id module parameter from the nfs.conf value Alice Mitchell
2020-07-16 14:02   ` Chuck Lever
2020-07-19 19:57     ` Alice Mitchell
2020-07-20 12:25       ` Chuck Lever
2020-07-20 17:54       ` Steve Dickson
2020-07-20 18:23         ` Chuck Lever
2020-07-21 15:20           ` Steve Dickson
2020-07-21 15:23             ` Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).