[nfs-utils PATCH v2 0/4] add systemd generator for the rpc_pipefs mountpoint

[nfs-utils PATCH v2 0/4] add systemd generator for the rpc_pipefs mountpoint

[Scott Mayhew]

These patches aim to make it a little easier to change the mountpoint.
Right now if you change the pipefs-directory in /etc/nfs.conf, you still
need to manually override the dependencies in the systemd unit files in
order for the change to actually work.

The first patch moves rpc.idmapd's (mostly) undocumented
pipefs-directory from /etc/idmapd.conf to a newly created [global]
section in /etc/nfs.conf.

The second patch makes rpc.gssd read its pipefs-directory setting from
the [global] section of /etc/nfs.conf instead of the [gssd] section.

The third patch allows blkmapd to read its pipefs-directory setting from
the [global] section of /etc/nfs.conf as well (previously it was
hard-coded).

The final patch adds a systemd generator that reads the
pipefs-directory configuration from /etc/nfs.conf, and if it differs
from the default it will automatically create a systemd mount unit
file for the pipefs mountpoint and it will override the dependencies on
the pipefs mountpoint via the rpc_pipefs.target unit file.

Scott Mayhew (4):
  idmapd: move the pipefs-directory config option to nfs.conf
  gssd: move the pipefs-directory setting to the nfs.conf global section
  blkmapd:  allow the rpc_pipefs mountpoint to be overridden
  systemd: add a generator for the rpc_pipefs mountpoint

 .gitignore                       |   1 +
 nfs.conf                         |   4 +-
 systemd/Makefile.am              |   5 +-
 systemd/nfs-blkmap.service       |   4 +-
 systemd/nfs-idmapd.service       |   4 +-
 systemd/nfs.conf.man             |  13 ++-
 systemd/rpc-gssd.service.in      |   4 +-
 systemd/rpc-pipefs-generator.c   | 216 +++++++++++++++++++++++++++++++++++++++
 systemd/rpc-svcgssd.service      |   3 +-
 systemd/rpc_pipefs.target        |   3 +
 utils/blkmapd/blkmapd.man        |  17 ++-
 utils/blkmapd/device-discovery.c |  47 +++++++--
 utils/gssd/gssd.c                |   4 +-
 utils/gssd/gssd.man              |  12 ++-
 utils/idmapd/idmapd.c            |  40 +++++---
 utils/idmapd/idmapd.man          |  21 +++-
 16 files changed, 357 insertions(+), 41 deletions(-)
 create mode 100644 systemd/rpc-pipefs-generator.c
 create mode 100644 systemd/rpc_pipefs.target

-- 
2.9.3

[nfs-utils PATCH v2 1/4] idmapd: move the pipefs-directory config option to nfs.conf

[Scott Mayhew]

Changed idmapd to read its value for the pipefs-directory from
/etc/nfs.conf rather than /etc/idmapd.conf.  All other configurations
related to id mapping still reside in /etc/idmapd.conf for now.

Added a warning to indicate that idmapd's -c option is deprecated.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 nfs.conf                |  3 +++
 systemd/nfs.conf.man    |  9 +++++++++
 utils/idmapd/idmapd.c   | 40 ++++++++++++++++++++++++++++------------
 utils/idmapd/idmapd.man | 21 ++++++++++++++++++++-
 4 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/nfs.conf b/nfs.conf
index 81ece06..ed516f5 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -2,6 +2,9 @@
 # This is a general conifguration for the 
 # NFS daemons and tools
 #
+#[global]
+# pipefs-directory=/var/lib/nfs/rpc_pipefs
+#
 #[exportfs]
 # debug=0
 #
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index bdc0988..f8849c5 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -96,6 +96,15 @@ value, which can be one or more from the list
 .BR all .
 When a list is given, the members should be comma-separated.
 .TP
+.B global
+Recognized values:
+.BR pipefs-directory .
+
+See
+.BR rpc.idmapd (8)
+for details.
+
+.TP
 .B nfsdcltrack
 Recognized values:
 .BR storagedir .
diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index f4e083a..5cac3a5 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -166,7 +166,7 @@ static uid_t nobodyuid;
 static gid_t nobodygid;
 
 /* Used by conffile.c in libnfs.a */
-char *conf_path;
+char *conf_path = NULL;
 
 static int
 flush_nfsd_cache(char *path, time_t now)
@@ -220,7 +220,6 @@ main(int argc, char **argv)
 	int ret;
 	char *progname;
 
-	conf_path = _PATH_IDMAPDCONF;
 	nobodyuser = NFS4NOBODY_USER;
 	nobodygroup = NFS4NOBODY_GROUP;
 	strlcpy(pipefsdir, PIPEFS_DIR, sizeof(pipefsdir));
@@ -234,8 +233,11 @@ main(int argc, char **argv)
 #define GETOPTSTR "hvfd:p:U:G:c:CS"
 	opterr=0; /* Turn off error messages */
 	while ((opt = getopt(argc, argv, GETOPTSTR)) != -1) {
-		if (opt == 'c')
+		if (opt == 'c') {
+			warnx("-c is deprecated and may be removed in the "
+			      "future.  See idmapd(8).");
 			conf_path = optarg;
+		}
 		if (opt == '?') {
 			if (strchr(GETOPTSTR, optopt))
 				warnx("'-%c' option requires an argument.", optopt);
@@ -247,17 +249,33 @@ main(int argc, char **argv)
 	}
 	optind = 1;
 
-	if (stat(conf_path, &sb) == -1 && (errno == ENOENT || errno == EACCES)) {
-		warn("Skipping configuration file \"%s\"", conf_path);
-		conf_path = NULL;
+	if (conf_path) { /* deprecated -c option was specified */
+		if (stat(conf_path, &sb) == -1 && (errno == ENOENT || errno == EACCES)) {
+			warn("Skipping configuration file \"%s\"", conf_path);
+			conf_path = NULL;
+		} else {
+			conf_init();
+			verbose = conf_get_num("General", "Verbosity", 0);
+			cache_entry_expiration = conf_get_num("General",
+					"Cache-Expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
+			CONF_SAVE(xpipefsdir, conf_get_str("General", "Pipefs-Directory"));
+			if (xpipefsdir != NULL)
+				strlcpy(pipefsdir, xpipefsdir, sizeof(pipefsdir));
+			CONF_SAVE(nobodyuser, conf_get_str("Mapping", "Nobody-User"));
+			CONF_SAVE(nobodygroup, conf_get_str("Mapping", "Nobody-Group"));
+		}
 	} else {
+		conf_path = NFS_CONFFILE;
 		conf_init();
-		verbose = conf_get_num("General", "Verbosity", 0);
-		cache_entry_expiration = conf_get_num("General",
-				"Cache-Expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
-		CONF_SAVE(xpipefsdir, conf_get_str("General", "Pipefs-Directory"));
+		CONF_SAVE(xpipefsdir, conf_get_str("global", "pipefs-directory"));
 		if (xpipefsdir != NULL)
 			strlcpy(pipefsdir, xpipefsdir, sizeof(pipefsdir));
+
+		conf_path = _PATH_IDMAPDCONF;
+		conf_init();
+		verbose = conf_get_num("General", "Verbosity", 0);
+		cache_entry_expiration = conf_get_num("General",
+				"cache-expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
 		CONF_SAVE(nobodyuser, conf_get_str("Mapping", "Nobody-User"));
 		CONF_SAVE(nobodygroup, conf_get_str("Mapping", "Nobody-Group"));
 	}
@@ -307,8 +325,6 @@ main(int argc, char **argv)
 #ifdef HAVE_NFS4_SET_DEBUG
 	nfs4_set_debug(verbose, xlog_warn);
 #endif
-	if (conf_path == NULL)
-		conf_path = _PATH_IDMAPDCONF;
 	if (nfs4_init_name_mapping(conf_path))
 		errx(1, "Unable to create name to user id mappings.");
 
diff --git a/utils/idmapd/idmapd.man b/utils/idmapd/idmapd.man
index d4ab894..301a8e9 100644
--- a/utils/idmapd/idmapd.man
+++ b/utils/idmapd/idmapd.man
@@ -73,11 +73,28 @@ The default value is \&"/var/lib/nfs/rpc_pipefs\&".
 .It Fl c Ar path
 Use configuration file
 .Ar path .
+This option is deprecated.
 .It Fl C
 Client-only: perform no idmapping for any NFS server, even if one is detected.
 .It Fl S
 Server-only: perform no idmapping for any NFS client, even if one is detected.
 .El
+.Sh CONFIGURATION FILES
+.Nm
+recognizes the following value from the
+.Sy [global]
+section of the
+.Pa /etc/nfs.conf
+configuration file:
+.Bl -tag -width Ds_imagedir
+.It Sy pipefs-directory
+Equivalent to
+.Sy -p .
+.El
+.Pp
+All other settings related to id mapping are found in the
+.Pa /etc/idmapd.conf
+configuration file.
 .Sh EXAMPLES
 .Cm rpc.idmapd -f -vvv
 .Pp
@@ -94,9 +111,11 @@ messages to console, and with a verbosity level of 3.
 .\" This next request is for sections 1, 6, 7 & 8 only.
 .\" .Sh ENVIRONMENT
 .Sh FILES
-.Pa /etc/idmapd.conf
+.Pa /etc/idmapd.conf ,
+.Pa /etc/nfs.conf
 .Sh SEE ALSO
 .Xr idmapd.conf 5 ,
+.Xr nfs.conf 5 ,
 .Xr nfsidmap 8
 .\".Sh SEE ALSO
 .\".Xr nylon.conf 4
-- 
2.9.3

[nfs-utils PATCH v2 2/4] gssd: move the pipefs-directory setting to the nfs.conf global section

[Scott Mayhew]

All the daemons should use the same rpc_pipefs, so have them read the
setting from the [global] section instead of from their program-specific
section.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 nfs.conf             |  1 -
 systemd/nfs.conf.man |  3 ++-
 utils/gssd/gssd.c    |  4 +---
 utils/gssd/gssd.man  | 12 ++++++++----
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/nfs.conf b/nfs.conf
index ed516f5..ae1e002 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -15,7 +15,6 @@
 # limit-to-legacy-enctypes=0
 # context-timeout=0
 # rpc-timeout=5
-# pipefs-directory=/var/lib/nfs/rpc_pipefs
 # keytab-file=/etc/krb5.keytab
 # cred-cache-directory=
 # preferred-realm=
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index f8849c5..fb59616 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -102,6 +102,8 @@ Recognized values:
 
 See
 .BR rpc.idmapd (8)
+and
+.BR rpc.gssd (8)
 for details.
 
 .TP
@@ -214,7 +216,6 @@ Recognized values:
 .BR limit-to-legacy-enctypes ,
 .BR context-timeout ,
 .BR rpc-timeout ,
-.BR pipefs-directory ,
 .BR keytab-file ,
 .BR cred-cache-directory ,
 .BR preferred-realm .
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 77125f1..cab3919 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -854,9 +854,7 @@ read_gss_conf(void)
 #endif
 	context_timeout = conf_get_num("gssd", "context-timeout", context_timeout);
 	rpc_timeout = conf_get_num("gssd", "rpc-timeout", rpc_timeout);
-	s = conf_get_str("gssd", "pipefs-directory");
-	if (!s)
-		s = conf_get_str("general", "pipefs-directory");
+	s = conf_get_str("global", "pipefs-directory");
 	if (s)
 		pipefs_path = s;
 	s = conf_get_str("gssd", "keytab-file");
diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
index 87eef02..c90c49e 100644
--- a/utils/gssd/gssd.man
+++ b/utils/gssd/gssd.man
@@ -335,10 +335,6 @@ Equivalent to
 Equivalent to
 .BR -t .
 .TP
-.B pipefs-directory
-Equivalent to
-.BR -p .
-.TP
 .B keytab-file
 Equivalent to
 .BR -k .
@@ -350,6 +346,14 @@ Equivalent to
 .B preferred-realm
 Equivalent to
 .BR -R .
+.P
+In addtion, the following value is recognized from the
+.B [global]
+section:
+.TP
+.B pipefs-directory
+Equivalent to
+.BR -p .
 
 .SH SEE ALSO
 .BR rpc.svcgssd (8),
-- 
2.9.3

[nfs-utils PATCH v2 3/4] blkmapd: allow the rpc_pipefs mountpoint to be overridden

[Scott Mayhew]

Allow the rpc_pipefs mountpoint to be overriden via the pipefs-directory
variable in the [global] section of /etc/nfs.conf.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 systemd/nfs.conf.man             |  3 ++-
 utils/blkmapd/blkmapd.man        | 17 ++++++++++++++-
 utils/blkmapd/device-discovery.c | 47 ++++++++++++++++++++++++++++++++--------
 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index fb59616..c35611a 100644
--- a/systemd/nfs.conf.man
+++ b/systemd/nfs.conf.man
@@ -101,7 +101,8 @@ Recognized values:
 .BR pipefs-directory .
 
 See
-.BR rpc.idmapd (8)
+.BR blkmapd (8),
+.BR rpc.idmapd (8),
 and
 .BR rpc.gssd (8)
 for details.
diff --git a/utils/blkmapd/blkmapd.man b/utils/blkmapd/blkmapd.man
index 914b80f..775ef57 100644
--- a/utils/blkmapd/blkmapd.man
+++ b/utils/blkmapd/blkmapd.man
@@ -43,9 +43,24 @@ Performs device discovery only then exits.
 Runs
 .B blkmapd
 in the foreground and sends output to stderr (as opposed to syslogd)
+.SH CONFIGURATION FILE
+The
+.B blkmapd
+daemon recognizes the following value from the
+.B [global]
+section of the
+.I /etc/nfs.conf
+configuration file:
+.TP
+.B pipefs-directory
+Tells
+.B blkmapd
+where to look for the rpc_pipefs filesystem.  The default value is
+.IR /var/lib/nfs/rpc_pipefs .
 .SH SEE ALSO
 .BR nfs (5),
-.BR dmsetup (8)
+.BR dmsetup (8),
+.BR nfs.conf (5)
 .sp
 RFC 5661 for the NFS version 4.1 specification.
 .br
diff --git a/utils/blkmapd/device-discovery.c b/utils/blkmapd/device-discovery.c
index 8eb3fd0..c81e99b 100644
--- a/utils/blkmapd/device-discovery.c
+++ b/utils/blkmapd/device-discovery.c
@@ -50,20 +50,36 @@
 #include <errno.h>
 #include <libdevmapper.h>
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif /* HAVE_CONFIG_H */
+
 #include "device-discovery.h"
 #include "xcommon.h"
+#include "nfslib.h"
+#include "conffile.h"
 
 #define EVENT_SIZE (sizeof(struct inotify_event))
 #define EVENT_BUFSIZE (1024 * EVENT_SIZE)
 
-#define BL_PIPE_FILE	"/var/lib/nfs/rpc_pipefs/nfs/blocklayout"
-#define NFSPIPE_DIR	"/var/lib/nfs/rpc_pipefs/nfs"
 #define RPCPIPE_DIR	"/var/lib/nfs/rpc_pipefs"
 #define PID_FILE	"/var/run/blkmapd.pid"
 
+#define CONF_SAVE(w, f) do {			\
+	char *p = f;				\
+	if (p != NULL)				\
+		(w) = p;			\
+} while (0)
+
+static char bl_pipe_file[PATH_MAX];
+static char nfspipe_dir[PATH_MAX];
+static char rpcpipe_dir[PATH_MAX];
+
 struct bl_disk *visible_disk_list;
 int    bl_watch_fd, bl_pipe_fd, nfs_pipedir_wfd, rpc_pipedir_wfd;
 int    pidfd = -1;
+char   *conf_path = NULL;
+
 
 struct bl_disk_path *bl_get_path(const char *filepath,
 				 struct bl_disk_path *paths)
@@ -358,8 +374,8 @@ static void bl_rpcpipe_cb(void)
 				continue;
 			if (event->mask & IN_CREATE) {
 				BL_LOG_WARNING("nfs pipe dir created\n");
-				bl_watch_dir(NFSPIPE_DIR, &nfs_pipedir_wfd);
-				bl_pipe_fd = open(BL_PIPE_FILE, O_RDWR);
+				bl_watch_dir(nfspipe_dir, &nfs_pipedir_wfd);
+				bl_pipe_fd = open(bl_pipe_file, O_RDWR);
 			} else if (event->mask & IN_DELETE) {
 				BL_LOG_WARNING("nfs pipe dir deleted\n");
 				inotify_rm_watch(bl_watch_fd, nfs_pipedir_wfd);
@@ -372,7 +388,7 @@ static void bl_rpcpipe_cb(void)
 				continue;
 			if (event->mask & IN_CREATE) {
 				BL_LOG_WARNING("blocklayout pipe file created\n");
-				bl_pipe_fd = open(BL_PIPE_FILE, O_RDWR);
+				bl_pipe_fd = open(bl_pipe_file, O_RDWR);
 				if (bl_pipe_fd < 0)
 					BL_LOG_ERR("open %s failed: %s\n",
 						event->name, strerror(errno));
@@ -437,6 +453,19 @@ int main(int argc, char **argv)
 {
 	int opt, dflag = 0, fg = 0, ret = 1;
 	char pidbuf[64];
+	char *xrpcpipe_dir = NULL;
+
+	strncpy(rpcpipe_dir, RPCPIPE_DIR, sizeof(rpcpipe_dir));
+	conf_path = NFS_CONFFILE;
+	conf_init();
+	CONF_SAVE(xrpcpipe_dir, conf_get_str("global", "pipefs-directory"));
+	if (xrpcpipe_dir != NULL)
+		strlcpy(rpcpipe_dir, xrpcpipe_dir, sizeof(rpcpipe_dir));
+
+	strncpy(nfspipe_dir, rpcpipe_dir, sizeof(nfspipe_dir));
+	strlcat(nfspipe_dir, "/nfs", sizeof(nfspipe_dir));
+	strncpy(bl_pipe_file, rpcpipe_dir, sizeof(bl_pipe_file));
+	strlcat(bl_pipe_file, "/nfs/blocklayout", sizeof(bl_pipe_file));
 
 	while ((opt = getopt(argc, argv, "hdf")) != -1) {
 		switch (opt) {
@@ -496,12 +525,12 @@ int main(int argc, char **argv)
 	}
 
 	/* open pipe file */
-	bl_watch_dir(RPCPIPE_DIR, &rpc_pipedir_wfd);
-	bl_watch_dir(NFSPIPE_DIR, &nfs_pipedir_wfd);
+	bl_watch_dir(rpcpipe_dir, &rpc_pipedir_wfd);
+	bl_watch_dir(nfspipe_dir, &nfs_pipedir_wfd);
 
-	bl_pipe_fd = open(BL_PIPE_FILE, O_RDWR);
+	bl_pipe_fd = open(bl_pipe_file, O_RDWR);
 	if (bl_pipe_fd < 0)
-		BL_LOG_ERR("open pipe file %s failed: %s\n", BL_PIPE_FILE, strerror(errno));
+		BL_LOG_ERR("open pipe file %s failed: %s\n", bl_pipe_file, strerror(errno));
 
 	while (1) {
 		/* discover device when needed */
-- 
2.9.3

[nfs-utils PATCH v2 4/4] systemd: add a generator for the rpc_pipefs mountpoint

[Scott Mayhew]

The nfs.conf has config options for the rpc_pipefs mountpoint.
Currently, changing these from the default also requires manually
overriding the systemd unit files that are hard-coded to mount the
filesystem on /var/lib/nfs/rpc_pipefs.

This patch adds a generator that creates a mount unit file for the
rpc_pipefs when a non-default value is specified in /etc/nfs.conf, as
well as a target unit file to override the dependencies for the systemd
units using the rpc_pipefs.  The blkmapd, idmapd, and gssd service unit
files have been modified to define their dependencies on the rpc_pipefs
mountpoint indirectly via the rpc_pipefs target unit file.

This patch also removes the dependency on the rpc_pipefs from the
rpc-svcgssd.service unit file.  rpc.svcgssd uses the sunrpc cache
mechanism to exchange data with the kernel, not the rpc_pipefs.

Signed-off-by: Scott Mayhew <smayhew@redhat.com>
---
 .gitignore                     |   1 +
 systemd/Makefile.am            |   5 +-
 systemd/nfs-blkmap.service     |   4 +-
 systemd/nfs-idmapd.service     |   4 +-
 systemd/rpc-gssd.service.in    |   4 +-
 systemd/rpc-pipefs-generator.c | 216 +++++++++++++++++++++++++++++++++++++++++
 systemd/rpc-svcgssd.service    |   3 +-
 systemd/rpc_pipefs.target      |   3 +
 8 files changed, 231 insertions(+), 9 deletions(-)
 create mode 100644 systemd/rpc-pipefs-generator.c
 create mode 100644 systemd/rpc_pipefs.target

diff --git a/.gitignore b/.gitignore
index 126d12c..941aca0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -70,6 +70,7 @@ tests/nsm_client/nlm_sm_inter_svc.c
 tests/nsm_client/nlm_sm_inter_xdr.c
 utils/nfsidmap/nfsidmap
 systemd/nfs-server-generator
+systemd/rpc-pipefs-generator
 systemd/nfs-config.service
 systemd/rpc-gssd.service
 # cscope database files
diff --git a/systemd/Makefile.am b/systemd/Makefile.am
index 0d15b9f..4becb77 100644
--- a/systemd/Makefile.am
+++ b/systemd/Makefile.am
@@ -4,6 +4,7 @@ MAINTAINERCLEANFILES = Makefile.in
 
 unit_files =  \
     nfs-client.target \
+    rpc_pipefs.target \
     \
     nfs-mountd.service \
     nfs-server.service \
@@ -42,12 +43,14 @@ EXTRA_DIST = $(unit_files) $(man5_MANS) $(man7_MANS)
 unit_dir = /usr/lib/systemd/system
 generator_dir = /usr/lib/systemd/system-generators
 
-EXTRA_PROGRAMS	= nfs-server-generator
+EXTRA_PROGRAMS	= nfs-server-generator rpc-pipefs-generator
 genexecdir = $(generator_dir)
 nfs_server_generator_LDADD = ../support/export/libexport.a \
 			     ../support/nfs/libnfs.a \
 			     ../support/misc/libmisc.a
 
+rpc_pipefs_generator_LDADD = ../support/nfs/libnfs.a
+
 if INSTALL_SYSTEMD
 genexec_PROGRAMS = nfs-server-generator
 install-data-hook: $(unit_files)
diff --git a/systemd/nfs-blkmap.service b/systemd/nfs-blkmap.service
index ddc324e..2bbcee6 100644
--- a/systemd/nfs-blkmap.service
+++ b/systemd/nfs-blkmap.service
@@ -2,8 +2,8 @@
 Description=pNFS block layout mapping daemon
 DefaultDependencies=no
 Conflicts=umount.target
-After=var-lib-nfs-rpc_pipefs.mount
-Requires=var-lib-nfs-rpc_pipefs.mount
+After=rpc_pipefs.target
+Requires=rpc_pipefs.target
 
 PartOf=nfs-utils.service
 
diff --git a/systemd/nfs-idmapd.service b/systemd/nfs-idmapd.service
index acca86b..f38fe52 100644
--- a/systemd/nfs-idmapd.service
+++ b/systemd/nfs-idmapd.service
@@ -1,8 +1,8 @@
 [Unit]
 Description=NFSv4 ID-name mapping service
 DefaultDependencies=no
-Requires=var-lib-nfs-rpc_pipefs.mount
-After=var-lib-nfs-rpc_pipefs.mount local-fs.target
+Requires=rpc_pipefs.target
+After=rpc_pipefs.target local-fs.target
 
 BindsTo=nfs-server.service
 
diff --git a/systemd/rpc-gssd.service.in b/systemd/rpc-gssd.service.in
index b353027..6807db3 100644
--- a/systemd/rpc-gssd.service.in
+++ b/systemd/rpc-gssd.service.in
@@ -2,8 +2,8 @@
 Description=RPC security service for NFS client and server
 DefaultDependencies=no
 Conflicts=umount.target
-Requires=var-lib-nfs-rpc_pipefs.mount
-After=var-lib-nfs-rpc_pipefs.mount
+Requires=rpc_pipefs.target
+After=rpc_pipefs.target
 
 ConditionPathExists=@_sysconfdir@/krb5.keytab
 
diff --git a/systemd/rpc-pipefs-generator.c b/systemd/rpc-pipefs-generator.c
new file mode 100644
index 0000000..b0a6c19
--- /dev/null
+++ b/systemd/rpc-pipefs-generator.c
@@ -0,0 +1,216 @@
+/*
+ * rpc-pipefs-generator:
+ *   systemd generator to create ordering dependencies between
+ *   nfs services and the rpc_pipefs mountpoint
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+#include <stdio.h>
+#include <mntent.h>
+
+#include "nfslib.h"
+#include "conffile.h"
+
+#define RPC_PIPEFS_DEFAULT "/var/lib/nfs/rpc_pipefs"
+char *conf_path = NFS_CONFFILE;
+
+int systemd_len(char *path)
+{
+	char *p;
+	int len = 0;
+
+	p = path;
+	while (*p == '/')
+		p++;
+
+	if (!*p)
+		/* "/" becomes "-", otherwise leading "/" is ignored */
+		return 1;
+
+	while (*p) {
+		unsigned char c = *p++;
+
+		if (c == '/') {
+			/* multiple non-trailing slashes become '-' */
+			while (*p == '/')
+				p++;
+			if (*p)
+				len++;
+		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
+			len++;
+		else {
+			len += 4;
+		}
+	}
+
+	return len;
+}
+
+char *systemd_escape(char *path)
+{
+	char *result = NULL;
+	char *p;
+	int len;
+
+	len = systemd_len(path);
+	if (!len)
+		goto out;
+
+	result = malloc(len + strlen(".mount") + 1);
+	if (!result)
+		goto out;
+
+	p = result;
+	while (*path == '/')
+		path++;
+	if (!*path) {
+		/* "/" becomes "-", otherwise leading "/" is ignored */
+		*p++ = '-';
+		goto out_append;
+	}
+	while (*path) {
+		unsigned char c = *path++;
+
+		if (c == '/') {
+			/* multiple non-trailing slashes become '-' */
+			while (*path == '/')
+				path++;
+			if (*path)
+				*p++ = '-';
+		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
+			*p++ = c;
+		else {
+			*p++ = '\\';
+			*p++ = 'x';
+			*p++ = '0' + ((c&0xf0)>>4) + (c>=0xa0)*('a'-'9'-1);
+			*p++ = '0' + (c&0x0f) + ((c&0x0f)>=0x0a)*('a'-'9'-1);
+		}
+	}
+
+out_append:
+	sprintf(p, ".mount");
+out:
+	return result;
+}
+
+static int generate_mount_unit(const char *pipefs_path, const char *pipefs_unit,
+			       const char *dirname)
+{
+	char	*path;
+	FILE	*f;
+
+	path = malloc(strlen(dirname) + 1 + strlen(pipefs_unit));
+	if (!path)
+		return 1;
+	sprintf(path, "%s/%s", dirname, pipefs_unit);
+	f = fopen(path, "w");
+	if (!f)
+		return 1;
+
+	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
+	fprintf(f, "Description=RPC Pipe File System\n");
+	fprintf(f, "DefaultDependencies=no\n");
+	fprintf(f, "After=systemd-tmpfiles-setup.service\n");
+	fprintf(f, "Conflicts=umount.target\n");
+	fprintf(f, "\n[Mount]\n");
+	fprintf(f, "What=sunrpc\n");
+	fprintf(f, "Where=%s\n", pipefs_path);
+	fprintf(f, "Type=rpc_pipefs\n");
+
+	fclose(f);
+	return 0;
+}
+
+static
+int generate_target(char *pipefs_path, const char *dirname)
+{
+	char	*path;
+	char	filebase[] = "/rpc_pipefs.target";
+	char	*pipefs_unit;
+	FILE	*f;
+	int 	ret = 0;
+
+	pipefs_unit = systemd_escape(pipefs_path);
+	if (!pipefs_unit)
+		return 1;
+
+	ret = generate_mount_unit(pipefs_path, pipefs_unit, dirname);
+	if (ret)
+		return ret;
+
+	path = malloc(strlen(dirname) + 1 + sizeof(filebase));
+	if (!path)
+		return 2;
+	sprintf(path, "%s", dirname);
+	mkdir(path, 0755);
+	strcat(path, filebase);
+	f = fopen(path, "w");
+	if (!f)
+		return 1;
+
+	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
+	fprintf(f, "Requires=%s\n", pipefs_unit);
+	fprintf(f, "After=%s\n", pipefs_unit);
+	fclose(f);
+
+	return 0;
+}
+
+static int is_non_pipefs_mountpoint(char *path)
+{
+	FILE		*mtab;
+	struct mntent	*mnt;
+
+	mtab = setmntent("/etc/mtab", "r");
+	if (!mtab)
+		return 0;
+
+	while ((mnt = getmntent(mtab)) != NULL) {
+		if (strlen(mnt->mnt_dir) != strlen(path))
+			continue;
+		if (strncmp(mnt->mnt_dir, path, strlen(mnt->mnt_dir)))
+			continue;
+		if (strncmp(mnt->mnt_type, "rpc_pipefs", strlen(mnt->mnt_type)))
+			break;
+	}
+	fclose(mtab);
+	return mnt != NULL;
+}
+
+int main(int argc, char *argv[])
+{
+	int 	ret;
+	char	*s;
+
+	/* Avoid using any external services */
+	xlog_syslog(0);
+
+	if (argc != 4 || argv[1][0] != '/') {
+		fprintf(stderr, "rpc-pipefs-generator: create systemd dependencies for nfs services\n");
+		fprintf(stderr, "Usage: normal-dir early-dir late-dir\n");
+		exit(1);
+	}
+
+	conf_init();
+	s = conf_get_str("global", "pipefs-directory");
+	if (!s)
+		exit(0);
+	if (strlen(s) == strlen(RPC_PIPEFS_DEFAULT) &&
+			strcmp(s, RPC_PIPEFS_DEFAULT) == 0)
+		exit(0);
+
+	if (is_non_pipefs_mountpoint(s))
+		exit(1);
+
+	ret = generate_target(s, argv[1]);
+	exit(ret);
+}
diff --git a/systemd/rpc-svcgssd.service b/systemd/rpc-svcgssd.service
index 7187e3c..cb2bcd4 100644
--- a/systemd/rpc-svcgssd.service
+++ b/systemd/rpc-svcgssd.service
@@ -1,8 +1,7 @@
 [Unit]
 Description=RPC security service for NFS server
 DefaultDependencies=no
-Requires=var-lib-nfs-rpc_pipefs.mount
-After=var-lib-nfs-rpc_pipefs.mount local-fs.target
+After=local-fs.target
 PartOf=nfs-server.service
 PartOf=nfs-utils.service
 
diff --git a/systemd/rpc_pipefs.target b/systemd/rpc_pipefs.target
new file mode 100644
index 0000000..01d4d27
--- /dev/null
+++ b/systemd/rpc_pipefs.target
@@ -0,0 +1,3 @@
+[Unit]
+Requires=var-lib-nfs-rpc_pipefs.mount
+After=var-lib-nfs-rpc_pipefs.mount
-- 
2.9.3

Re: [nfs-utils PATCH v2 1/4] idmapd: move the pipefs-directory config option to nfs.conf

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 4366 bytes --]

On Wed, Apr 05 2017, Scott Mayhew wrote:

> Changed idmapd to read its value for the pipefs-directory from
> /etc/nfs.conf rather than /etc/idmapd.conf.  All other configurations
> related to id mapping still reside in /etc/idmapd.conf for now.
>
> Added a warning to indicate that idmapd's -c option is deprecated.
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> ---
>  nfs.conf                |  3 +++
>  systemd/nfs.conf.man    |  9 +++++++++
>  utils/idmapd/idmapd.c   | 40 ++++++++++++++++++++++++++++------------
>  utils/idmapd/idmapd.man | 21 ++++++++++++++++++++-
>  4 files changed, 60 insertions(+), 13 deletions(-)
>
> diff --git a/nfs.conf b/nfs.conf
> index 81ece06..ed516f5 100644
> --- a/nfs.conf
> +++ b/nfs.conf
> @@ -2,6 +2,9 @@
>  # This is a general conifguration for the 
>  # NFS daemons and tools
>  #
> +#[global]
> +# pipefs-directory=/var/lib/nfs/rpc_pipefs
> +#
>  #[exportfs]
>  # debug=0
>  #
> diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
> index bdc0988..f8849c5 100644
> --- a/systemd/nfs.conf.man
> +++ b/systemd/nfs.conf.man
> @@ -96,6 +96,15 @@ value, which can be one or more from the list
>  .BR all .
>  When a list is given, the members should be comma-separated.
>  .TP
> +.B global
> +Recognized values:
> +.BR pipefs-directory .
> +
> +See
> +.BR rpc.idmapd (8)
> +for details.
> +
> +.TP
>  .B nfsdcltrack
>  Recognized values:
>  .BR storagedir .
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index f4e083a..5cac3a5 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -166,7 +166,7 @@ static uid_t nobodyuid;
>  static gid_t nobodygid;
>  
>  /* Used by conffile.c in libnfs.a */
> -char *conf_path;
> +char *conf_path = NULL;
>  
>  static int
>  flush_nfsd_cache(char *path, time_t now)
> @@ -220,7 +220,6 @@ main(int argc, char **argv)
>  	int ret;
>  	char *progname;
>  
> -	conf_path = _PATH_IDMAPDCONF;
>  	nobodyuser = NFS4NOBODY_USER;
>  	nobodygroup = NFS4NOBODY_GROUP;
>  	strlcpy(pipefsdir, PIPEFS_DIR, sizeof(pipefsdir));
> @@ -234,8 +233,11 @@ main(int argc, char **argv)
>  #define GETOPTSTR "hvfd:p:U:G:c:CS"
>  	opterr=0; /* Turn off error messages */
>  	while ((opt = getopt(argc, argv, GETOPTSTR)) != -1) {
> -		if (opt == 'c')
> +		if (opt == 'c') {
> +			warnx("-c is deprecated and may be removed in the "
> +			      "future.  See idmapd(8).");
>  			conf_path = optarg;
> +		}
>  		if (opt == '?') {
>  			if (strchr(GETOPTSTR, optopt))
>  				warnx("'-%c' option requires an argument.", optopt);
> @@ -247,17 +249,33 @@ main(int argc, char **argv)
>  	}
>  	optind = 1;
>  
> -	if (stat(conf_path, &sb) == -1 && (errno == ENOENT || errno == EACCES)) {
> -		warn("Skipping configuration file \"%s\"", conf_path);
> -		conf_path = NULL;
> +	if (conf_path) { /* deprecated -c option was specified */
> +		if (stat(conf_path, &sb) == -1 && (errno == ENOENT || errno == EACCES)) {
> +			warn("Skipping configuration file \"%s\"", conf_path);
> +			conf_path = NULL;
> +		} else {
> +			conf_init();
> +			verbose = conf_get_num("General", "Verbosity", 0);
> +			cache_entry_expiration = conf_get_num("General",
> +					"Cache-Expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
> +			CONF_SAVE(xpipefsdir, conf_get_str("General", "Pipefs-Directory"));
> +			if (xpipefsdir != NULL)
> +				strlcpy(pipefsdir, xpipefsdir, sizeof(pipefsdir));
> +			CONF_SAVE(nobodyuser, conf_get_str("Mapping", "Nobody-User"));
> +			CONF_SAVE(nobodygroup, conf_get_str("Mapping", "Nobody-Group"));
> +		}
>  	} else {
> +		conf_path = NFS_CONFFILE;
>  		conf_init();
> -		verbose = conf_get_num("General", "Verbosity", 0);
> -		cache_entry_expiration = conf_get_num("General",
> -				"Cache-Expiration", DEFAULT_IDMAP_CACHE_EXPIRY);
> -		CONF_SAVE(xpipefsdir, conf_get_str("General", "Pipefs-Directory"));
> +		CONF_SAVE(xpipefsdir, conf_get_str("global", "pipefs-directory"));

Can we leave the global section of the config file named "general" -
i.e. it contains general configuration.
conf_get_str() uses strcasecmp() so "General","Pipefs-Directory" works
the same as "general","pipefs-directory".
Then gssd.c wouldn't need to be changed (unless you want to add a
deprecation warning when the gssd-specific setting is found).

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [nfs-utils PATCH v2 4/4] systemd: add a generator for the rpc_pipefs mountpoint

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 11701 bytes --]

On Wed, Apr 05 2017, Scott Mayhew wrote:

> The nfs.conf has config options for the rpc_pipefs mountpoint.
> Currently, changing these from the default also requires manually
> overriding the systemd unit files that are hard-coded to mount the
> filesystem on /var/lib/nfs/rpc_pipefs.
>
> This patch adds a generator that creates a mount unit file for the
> rpc_pipefs when a non-default value is specified in /etc/nfs.conf, as
> well as a target unit file to override the dependencies for the systemd
> units using the rpc_pipefs.  The blkmapd, idmapd, and gssd service unit
> files have been modified to define their dependencies on the rpc_pipefs
> mountpoint indirectly via the rpc_pipefs target unit file.
>
> This patch also removes the dependency on the rpc_pipefs from the
> rpc-svcgssd.service unit file.  rpc.svcgssd uses the sunrpc cache
> mechanism to exchange data with the kernel, not the rpc_pipefs.
>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>

I was reading through this, which I really thought would be a good patch
that I would give an Ack to, and looking at some of the detail involved,
I started to wonder if we were really doing the right thing here.
You go to some trouble to make the name of the .mount unit file match
the name of the location of the mountpoint.  Is that really necessary?

Unit files created by systemd-fstab-generator do follow that pattern,
but they don't have to.

Maybe we should just:

1/ rename var-lib-nfs-rpc_pipefs.mount to rpc_pipefs.mount,
   and modify the dependencies accordingly.

2/ write a generator which (if necessary) creates a drop-in for
   rpc_pipefs.mount which simply overrides "Where".

 i.e /run/systemd/generator/rpc_pipefs.mount.d/pipefs.conf
 would contain

    [Mount]
    Where=/foo/bar

I'm sorry to discard your good work, but I now think that a lot of it is
unnecessary.  Sorry for not seeing that earlier.

Thanks,
NeilBrown


> ---
>  .gitignore                     |   1 +
>  systemd/Makefile.am            |   5 +-
>  systemd/nfs-blkmap.service     |   4 +-
>  systemd/nfs-idmapd.service     |   4 +-
>  systemd/rpc-gssd.service.in    |   4 +-
>  systemd/rpc-pipefs-generator.c | 216 +++++++++++++++++++++++++++++++++++++++++
>  systemd/rpc-svcgssd.service    |   3 +-
>  systemd/rpc_pipefs.target      |   3 +
>  8 files changed, 231 insertions(+), 9 deletions(-)
>  create mode 100644 systemd/rpc-pipefs-generator.c
>  create mode 100644 systemd/rpc_pipefs.target
>
> diff --git a/.gitignore b/.gitignore
> index 126d12c..941aca0 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -70,6 +70,7 @@ tests/nsm_client/nlm_sm_inter_svc.c
>  tests/nsm_client/nlm_sm_inter_xdr.c
>  utils/nfsidmap/nfsidmap
>  systemd/nfs-server-generator
> +systemd/rpc-pipefs-generator
>  systemd/nfs-config.service
>  systemd/rpc-gssd.service
>  # cscope database files
> diff --git a/systemd/Makefile.am b/systemd/Makefile.am
> index 0d15b9f..4becb77 100644
> --- a/systemd/Makefile.am
> +++ b/systemd/Makefile.am
> @@ -4,6 +4,7 @@ MAINTAINERCLEANFILES = Makefile.in
>  
>  unit_files =  \
>      nfs-client.target \
> +    rpc_pipefs.target \
>      \
>      nfs-mountd.service \
>      nfs-server.service \
> @@ -42,12 +43,14 @@ EXTRA_DIST = $(unit_files) $(man5_MANS) $(man7_MANS)
>  unit_dir = /usr/lib/systemd/system
>  generator_dir = /usr/lib/systemd/system-generators
>  
> -EXTRA_PROGRAMS	= nfs-server-generator
> +EXTRA_PROGRAMS	= nfs-server-generator rpc-pipefs-generator
>  genexecdir = $(generator_dir)
>  nfs_server_generator_LDADD = ../support/export/libexport.a \
>  			     ../support/nfs/libnfs.a \
>  			     ../support/misc/libmisc.a
>  
> +rpc_pipefs_generator_LDADD = ../support/nfs/libnfs.a
> +
>  if INSTALL_SYSTEMD
>  genexec_PROGRAMS = nfs-server-generator
>  install-data-hook: $(unit_files)
> diff --git a/systemd/nfs-blkmap.service b/systemd/nfs-blkmap.service
> index ddc324e..2bbcee6 100644
> --- a/systemd/nfs-blkmap.service
> +++ b/systemd/nfs-blkmap.service
> @@ -2,8 +2,8 @@
>  Description=pNFS block layout mapping daemon
>  DefaultDependencies=no
>  Conflicts=umount.target
> -After=var-lib-nfs-rpc_pipefs.mount
> -Requires=var-lib-nfs-rpc_pipefs.mount
> +After=rpc_pipefs.target
> +Requires=rpc_pipefs.target
>  
>  PartOf=nfs-utils.service
>  
> diff --git a/systemd/nfs-idmapd.service b/systemd/nfs-idmapd.service
> index acca86b..f38fe52 100644
> --- a/systemd/nfs-idmapd.service
> +++ b/systemd/nfs-idmapd.service
> @@ -1,8 +1,8 @@
>  [Unit]
>  Description=NFSv4 ID-name mapping service
>  DefaultDependencies=no
> -Requires=var-lib-nfs-rpc_pipefs.mount
> -After=var-lib-nfs-rpc_pipefs.mount local-fs.target
> +Requires=rpc_pipefs.target
> +After=rpc_pipefs.target local-fs.target
>  
>  BindsTo=nfs-server.service
>  
> diff --git a/systemd/rpc-gssd.service.in b/systemd/rpc-gssd.service.in
> index b353027..6807db3 100644
> --- a/systemd/rpc-gssd.service.in
> +++ b/systemd/rpc-gssd.service.in
> @@ -2,8 +2,8 @@
>  Description=RPC security service for NFS client and server
>  DefaultDependencies=no
>  Conflicts=umount.target
> -Requires=var-lib-nfs-rpc_pipefs.mount
> -After=var-lib-nfs-rpc_pipefs.mount
> +Requires=rpc_pipefs.target
> +After=rpc_pipefs.target
>  
>  ConditionPathExists=@_sysconfdir@/krb5.keytab
>  
> diff --git a/systemd/rpc-pipefs-generator.c b/systemd/rpc-pipefs-generator.c
> new file mode 100644
> index 0000000..b0a6c19
> --- /dev/null
> +++ b/systemd/rpc-pipefs-generator.c
> @@ -0,0 +1,216 @@
> +/*
> + * rpc-pipefs-generator:
> + *   systemd generator to create ordering dependencies between
> + *   nfs services and the rpc_pipefs mountpoint
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <ctype.h>
> +#include <stdio.h>
> +#include <mntent.h>
> +
> +#include "nfslib.h"
> +#include "conffile.h"
> +
> +#define RPC_PIPEFS_DEFAULT "/var/lib/nfs/rpc_pipefs"
> +char *conf_path = NFS_CONFFILE;
> +
> +int systemd_len(char *path)
> +{
> +	char *p;
> +	int len = 0;
> +
> +	p = path;
> +	while (*p == '/')
> +		p++;
> +
> +	if (!*p)
> +		/* "/" becomes "-", otherwise leading "/" is ignored */
> +		return 1;
> +
> +	while (*p) {
> +		unsigned char c = *p++;
> +
> +		if (c == '/') {
> +			/* multiple non-trailing slashes become '-' */
> +			while (*p == '/')
> +				p++;
> +			if (*p)
> +				len++;
> +		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
> +			len++;
> +		else {
> +			len += 4;
> +		}
> +	}
> +
> +	return len;
> +}
> +
> +char *systemd_escape(char *path)
> +{
> +	char *result = NULL;
> +	char *p;
> +	int len;
> +
> +	len = systemd_len(path);
> +	if (!len)
> +		goto out;
> +
> +	result = malloc(len + strlen(".mount") + 1);
> +	if (!result)
> +		goto out;
> +
> +	p = result;
> +	while (*path == '/')
> +		path++;
> +	if (!*path) {
> +		/* "/" becomes "-", otherwise leading "/" is ignored */
> +		*p++ = '-';
> +		goto out_append;
> +	}
> +	while (*path) {
> +		unsigned char c = *path++;
> +
> +		if (c == '/') {
> +			/* multiple non-trailing slashes become '-' */
> +			while (*path == '/')
> +				path++;
> +			if (*path)
> +				*p++ = '-';
> +		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
> +			*p++ = c;
> +		else {
> +			*p++ = '\\';
> +			*p++ = 'x';
> +			*p++ = '0' + ((c&0xf0)>>4) + (c>=0xa0)*('a'-'9'-1);
> +			*p++ = '0' + (c&0x0f) + ((c&0x0f)>=0x0a)*('a'-'9'-1);
> +		}
> +	}
> +
> +out_append:
> +	sprintf(p, ".mount");
> +out:
> +	return result;
> +}
> +
> +static int generate_mount_unit(const char *pipefs_path, const char *pipefs_unit,
> +			       const char *dirname)
> +{
> +	char	*path;
> +	FILE	*f;
> +
> +	path = malloc(strlen(dirname) + 1 + strlen(pipefs_unit));
> +	if (!path)
> +		return 1;
> +	sprintf(path, "%s/%s", dirname, pipefs_unit);
> +	f = fopen(path, "w");
> +	if (!f)
> +		return 1;
> +
> +	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
> +	fprintf(f, "Description=RPC Pipe File System\n");
> +	fprintf(f, "DefaultDependencies=no\n");
> +	fprintf(f, "After=systemd-tmpfiles-setup.service\n");
> +	fprintf(f, "Conflicts=umount.target\n");
> +	fprintf(f, "\n[Mount]\n");
> +	fprintf(f, "What=sunrpc\n");
> +	fprintf(f, "Where=%s\n", pipefs_path);
> +	fprintf(f, "Type=rpc_pipefs\n");
> +
> +	fclose(f);
> +	return 0;
> +}
> +
> +static
> +int generate_target(char *pipefs_path, const char *dirname)
> +{
> +	char	*path;
> +	char	filebase[] = "/rpc_pipefs.target";
> +	char	*pipefs_unit;
> +	FILE	*f;
> +	int 	ret = 0;
> +
> +	pipefs_unit = systemd_escape(pipefs_path);
> +	if (!pipefs_unit)
> +		return 1;
> +
> +	ret = generate_mount_unit(pipefs_path, pipefs_unit, dirname);
> +	if (ret)
> +		return ret;
> +
> +	path = malloc(strlen(dirname) + 1 + sizeof(filebase));
> +	if (!path)
> +		return 2;
> +	sprintf(path, "%s", dirname);
> +	mkdir(path, 0755);
> +	strcat(path, filebase);
> +	f = fopen(path, "w");
> +	if (!f)
> +		return 1;
> +
> +	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
> +	fprintf(f, "Requires=%s\n", pipefs_unit);
> +	fprintf(f, "After=%s\n", pipefs_unit);
> +	fclose(f);
> +
> +	return 0;
> +}
> +
> +static int is_non_pipefs_mountpoint(char *path)
> +{
> +	FILE		*mtab;
> +	struct mntent	*mnt;
> +
> +	mtab = setmntent("/etc/mtab", "r");
> +	if (!mtab)
> +		return 0;
> +
> +	while ((mnt = getmntent(mtab)) != NULL) {
> +		if (strlen(mnt->mnt_dir) != strlen(path))
> +			continue;
> +		if (strncmp(mnt->mnt_dir, path, strlen(mnt->mnt_dir)))
> +			continue;
> +		if (strncmp(mnt->mnt_type, "rpc_pipefs", strlen(mnt->mnt_type)))
> +			break;
> +	}
> +	fclose(mtab);
> +	return mnt != NULL;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	int 	ret;
> +	char	*s;
> +
> +	/* Avoid using any external services */
> +	xlog_syslog(0);
> +
> +	if (argc != 4 || argv[1][0] != '/') {
> +		fprintf(stderr, "rpc-pipefs-generator: create systemd dependencies for nfs services\n");
> +		fprintf(stderr, "Usage: normal-dir early-dir late-dir\n");
> +		exit(1);
> +	}
> +
> +	conf_init();
> +	s = conf_get_str("global", "pipefs-directory");
> +	if (!s)
> +		exit(0);
> +	if (strlen(s) == strlen(RPC_PIPEFS_DEFAULT) &&
> +			strcmp(s, RPC_PIPEFS_DEFAULT) == 0)
> +		exit(0);
> +
> +	if (is_non_pipefs_mountpoint(s))
> +		exit(1);
> +
> +	ret = generate_target(s, argv[1]);
> +	exit(ret);
> +}
> diff --git a/systemd/rpc-svcgssd.service b/systemd/rpc-svcgssd.service
> index 7187e3c..cb2bcd4 100644
> --- a/systemd/rpc-svcgssd.service
> +++ b/systemd/rpc-svcgssd.service
> @@ -1,8 +1,7 @@
>  [Unit]
>  Description=RPC security service for NFS server
>  DefaultDependencies=no
> -Requires=var-lib-nfs-rpc_pipefs.mount
> -After=var-lib-nfs-rpc_pipefs.mount local-fs.target
> +After=local-fs.target
>  PartOf=nfs-server.service
>  PartOf=nfs-utils.service
>  
> diff --git a/systemd/rpc_pipefs.target b/systemd/rpc_pipefs.target
> new file mode 100644
> index 0000000..01d4d27
> --- /dev/null
> +++ b/systemd/rpc_pipefs.target
> @@ -0,0 +1,3 @@
> +[Unit]
> +Requires=var-lib-nfs-rpc_pipefs.mount
> +After=var-lib-nfs-rpc_pipefs.mount
> -- 
> 2.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [nfs-utils PATCH v2 4/4] systemd: add a generator for the rpc_pipefs mountpoint

[Scott Mayhew]

On Thu, 06 Apr 2017, NeilBrown wrote:

> On Wed, Apr 05 2017, Scott Mayhew wrote:
> 
> > The nfs.conf has config options for the rpc_pipefs mountpoint.
> > Currently, changing these from the default also requires manually
> > overriding the systemd unit files that are hard-coded to mount the
> > filesystem on /var/lib/nfs/rpc_pipefs.
> >
> > This patch adds a generator that creates a mount unit file for the
> > rpc_pipefs when a non-default value is specified in /etc/nfs.conf, as
> > well as a target unit file to override the dependencies for the systemd
> > units using the rpc_pipefs.  The blkmapd, idmapd, and gssd service unit
> > files have been modified to define their dependencies on the rpc_pipefs
> > mountpoint indirectly via the rpc_pipefs target unit file.
> >
> > This patch also removes the dependency on the rpc_pipefs from the
> > rpc-svcgssd.service unit file.  rpc.svcgssd uses the sunrpc cache
> > mechanism to exchange data with the kernel, not the rpc_pipefs.
> >
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> 
> I was reading through this, which I really thought would be a good patch
> that I would give an Ack to, and looking at some of the detail involved,
> I started to wonder if we were really doing the right thing here.
> You go to some trouble to make the name of the .mount unit file match
> the name of the location of the mountpoint.  Is that really necessary?
> 
> Unit files created by systemd-fstab-generator do follow that pattern,
> but they don't have to.

I didn't know that.
> 
> Maybe we should just:
> 
> 1/ rename var-lib-nfs-rpc_pipefs.mount to rpc_pipefs.mount,
>    and modify the dependencies accordingly.
> 
> 2/ write a generator which (if necessary) creates a drop-in for
>    rpc_pipefs.mount which simply overrides "Where".
> 
>  i.e /run/systemd/generator/rpc_pipefs.mount.d/pipefs.conf
>  would contain
> 
>     [Mount]
>     Where=/foo/bar

Sounds good to me, I'll take a look.

-Scott
> 
> I'm sorry to discard your good work, but I now think that a lot of it is
> unnecessary.  Sorry for not seeing that earlier.
> 
> Thanks,
> NeilBrown
> 
> 
> > ---
> >  .gitignore                     |   1 +
> >  systemd/Makefile.am            |   5 +-
> >  systemd/nfs-blkmap.service     |   4 +-
> >  systemd/nfs-idmapd.service     |   4 +-
> >  systemd/rpc-gssd.service.in    |   4 +-
> >  systemd/rpc-pipefs-generator.c | 216 +++++++++++++++++++++++++++++++++++++++++
> >  systemd/rpc-svcgssd.service    |   3 +-
> >  systemd/rpc_pipefs.target      |   3 +
> >  8 files changed, 231 insertions(+), 9 deletions(-)
> >  create mode 100644 systemd/rpc-pipefs-generator.c
> >  create mode 100644 systemd/rpc_pipefs.target
> >
> > diff --git a/.gitignore b/.gitignore
> > index 126d12c..941aca0 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -70,6 +70,7 @@ tests/nsm_client/nlm_sm_inter_svc.c
> >  tests/nsm_client/nlm_sm_inter_xdr.c
> >  utils/nfsidmap/nfsidmap
> >  systemd/nfs-server-generator
> > +systemd/rpc-pipefs-generator
> >  systemd/nfs-config.service
> >  systemd/rpc-gssd.service
> >  # cscope database files
> > diff --git a/systemd/Makefile.am b/systemd/Makefile.am
> > index 0d15b9f..4becb77 100644
> > --- a/systemd/Makefile.am
> > +++ b/systemd/Makefile.am
> > @@ -4,6 +4,7 @@ MAINTAINERCLEANFILES = Makefile.in
> >  
> >  unit_files =  \
> >      nfs-client.target \
> > +    rpc_pipefs.target \
> >      \
> >      nfs-mountd.service \
> >      nfs-server.service \
> > @@ -42,12 +43,14 @@ EXTRA_DIST = $(unit_files) $(man5_MANS) $(man7_MANS)
> >  unit_dir = /usr/lib/systemd/system
> >  generator_dir = /usr/lib/systemd/system-generators
> >  
> > -EXTRA_PROGRAMS	= nfs-server-generator
> > +EXTRA_PROGRAMS	= nfs-server-generator rpc-pipefs-generator
> >  genexecdir = $(generator_dir)
> >  nfs_server_generator_LDADD = ../support/export/libexport.a \
> >  			     ../support/nfs/libnfs.a \
> >  			     ../support/misc/libmisc.a
> >  
> > +rpc_pipefs_generator_LDADD = ../support/nfs/libnfs.a
> > +
> >  if INSTALL_SYSTEMD
> >  genexec_PROGRAMS = nfs-server-generator
> >  install-data-hook: $(unit_files)
> > diff --git a/systemd/nfs-blkmap.service b/systemd/nfs-blkmap.service
> > index ddc324e..2bbcee6 100644
> > --- a/systemd/nfs-blkmap.service
> > +++ b/systemd/nfs-blkmap.service
> > @@ -2,8 +2,8 @@
> >  Description=pNFS block layout mapping daemon
> >  DefaultDependencies=no
> >  Conflicts=umount.target
> > -After=var-lib-nfs-rpc_pipefs.mount
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > +After=rpc_pipefs.target
> > +Requires=rpc_pipefs.target
> >  
> >  PartOf=nfs-utils.service
> >  
> > diff --git a/systemd/nfs-idmapd.service b/systemd/nfs-idmapd.service
> > index acca86b..f38fe52 100644
> > --- a/systemd/nfs-idmapd.service
> > +++ b/systemd/nfs-idmapd.service
> > @@ -1,8 +1,8 @@
> >  [Unit]
> >  Description=NFSv4 ID-name mapping service
> >  DefaultDependencies=no
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > -After=var-lib-nfs-rpc_pipefs.mount local-fs.target
> > +Requires=rpc_pipefs.target
> > +After=rpc_pipefs.target local-fs.target
> >  
> >  BindsTo=nfs-server.service
> >  
> > diff --git a/systemd/rpc-gssd.service.in b/systemd/rpc-gssd.service.in
> > index b353027..6807db3 100644
> > --- a/systemd/rpc-gssd.service.in
> > +++ b/systemd/rpc-gssd.service.in
> > @@ -2,8 +2,8 @@
> >  Description=RPC security service for NFS client and server
> >  DefaultDependencies=no
> >  Conflicts=umount.target
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > -After=var-lib-nfs-rpc_pipefs.mount
> > +Requires=rpc_pipefs.target
> > +After=rpc_pipefs.target
> >  
> >  ConditionPathExists=@_sysconfdir@/krb5.keytab
> >  
> > diff --git a/systemd/rpc-pipefs-generator.c b/systemd/rpc-pipefs-generator.c
> > new file mode 100644
> > index 0000000..b0a6c19
> > --- /dev/null
> > +++ b/systemd/rpc-pipefs-generator.c
> > @@ -0,0 +1,216 @@
> > +/*
> > + * rpc-pipefs-generator:
> > + *   systemd generator to create ordering dependencies between
> > + *   nfs services and the rpc_pipefs mountpoint
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <ctype.h>
> > +#include <stdio.h>
> > +#include <mntent.h>
> > +
> > +#include "nfslib.h"
> > +#include "conffile.h"
> > +
> > +#define RPC_PIPEFS_DEFAULT "/var/lib/nfs/rpc_pipefs"
> > +char *conf_path = NFS_CONFFILE;
> > +
> > +int systemd_len(char *path)
> > +{
> > +	char *p;
> > +	int len = 0;
> > +
> > +	p = path;
> > +	while (*p == '/')
> > +		p++;
> > +
> > +	if (!*p)
> > +		/* "/" becomes "-", otherwise leading "/" is ignored */
> > +		return 1;
> > +
> > +	while (*p) {
> > +		unsigned char c = *p++;
> > +
> > +		if (c == '/') {
> > +			/* multiple non-trailing slashes become '-' */
> > +			while (*p == '/')
> > +				p++;
> > +			if (*p)
> > +				len++;
> > +		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
> > +			len++;
> > +		else {
> > +			len += 4;
> > +		}
> > +	}
> > +
> > +	return len;
> > +}
> > +
> > +char *systemd_escape(char *path)
> > +{
> > +	char *result = NULL;
> > +	char *p;
> > +	int len;
> > +
> > +	len = systemd_len(path);
> > +	if (!len)
> > +		goto out;
> > +
> > +	result = malloc(len + strlen(".mount") + 1);
> > +	if (!result)
> > +		goto out;
> > +
> > +	p = result;
> > +	while (*path == '/')
> > +		path++;
> > +	if (!*path) {
> > +		/* "/" becomes "-", otherwise leading "/" is ignored */
> > +		*p++ = '-';
> > +		goto out_append;
> > +	}
> > +	while (*path) {
> > +		unsigned char c = *path++;
> > +
> > +		if (c == '/') {
> > +			/* multiple non-trailing slashes become '-' */
> > +			while (*path == '/')
> > +				path++;
> > +			if (*path)
> > +				*p++ = '-';
> > +		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
> > +			*p++ = c;
> > +		else {
> > +			*p++ = '\\';
> > +			*p++ = 'x';
> > +			*p++ = '0' + ((c&0xf0)>>4) + (c>=0xa0)*('a'-'9'-1);
> > +			*p++ = '0' + (c&0x0f) + ((c&0x0f)>=0x0a)*('a'-'9'-1);
> > +		}
> > +	}
> > +
> > +out_append:
> > +	sprintf(p, ".mount");
> > +out:
> > +	return result;
> > +}
> > +
> > +static int generate_mount_unit(const char *pipefs_path, const char *pipefs_unit,
> > +			       const char *dirname)
> > +{
> > +	char	*path;
> > +	FILE	*f;
> > +
> > +	path = malloc(strlen(dirname) + 1 + strlen(pipefs_unit));
> > +	if (!path)
> > +		return 1;
> > +	sprintf(path, "%s/%s", dirname, pipefs_unit);
> > +	f = fopen(path, "w");
> > +	if (!f)
> > +		return 1;
> > +
> > +	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
> > +	fprintf(f, "Description=RPC Pipe File System\n");
> > +	fprintf(f, "DefaultDependencies=no\n");
> > +	fprintf(f, "After=systemd-tmpfiles-setup.service\n");
> > +	fprintf(f, "Conflicts=umount.target\n");
> > +	fprintf(f, "\n[Mount]\n");
> > +	fprintf(f, "What=sunrpc\n");
> > +	fprintf(f, "Where=%s\n", pipefs_path);
> > +	fprintf(f, "Type=rpc_pipefs\n");
> > +
> > +	fclose(f);
> > +	return 0;
> > +}
> > +
> > +static
> > +int generate_target(char *pipefs_path, const char *dirname)
> > +{
> > +	char	*path;
> > +	char	filebase[] = "/rpc_pipefs.target";
> > +	char	*pipefs_unit;
> > +	FILE	*f;
> > +	int 	ret = 0;
> > +
> > +	pipefs_unit = systemd_escape(pipefs_path);
> > +	if (!pipefs_unit)
> > +		return 1;
> > +
> > +	ret = generate_mount_unit(pipefs_path, pipefs_unit, dirname);
> > +	if (ret)
> > +		return ret;
> > +
> > +	path = malloc(strlen(dirname) + 1 + sizeof(filebase));
> > +	if (!path)
> > +		return 2;
> > +	sprintf(path, "%s", dirname);
> > +	mkdir(path, 0755);
> > +	strcat(path, filebase);
> > +	f = fopen(path, "w");
> > +	if (!f)
> > +		return 1;
> > +
> > +	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
> > +	fprintf(f, "Requires=%s\n", pipefs_unit);
> > +	fprintf(f, "After=%s\n", pipefs_unit);
> > +	fclose(f);
> > +
> > +	return 0;
> > +}
> > +
> > +static int is_non_pipefs_mountpoint(char *path)
> > +{
> > +	FILE		*mtab;
> > +	struct mntent	*mnt;
> > +
> > +	mtab = setmntent("/etc/mtab", "r");
> > +	if (!mtab)
> > +		return 0;
> > +
> > +	while ((mnt = getmntent(mtab)) != NULL) {
> > +		if (strlen(mnt->mnt_dir) != strlen(path))
> > +			continue;
> > +		if (strncmp(mnt->mnt_dir, path, strlen(mnt->mnt_dir)))
> > +			continue;
> > +		if (strncmp(mnt->mnt_type, "rpc_pipefs", strlen(mnt->mnt_type)))
> > +			break;
> > +	}
> > +	fclose(mtab);
> > +	return mnt != NULL;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	int 	ret;
> > +	char	*s;
> > +
> > +	/* Avoid using any external services */
> > +	xlog_syslog(0);
> > +
> > +	if (argc != 4 || argv[1][0] != '/') {
> > +		fprintf(stderr, "rpc-pipefs-generator: create systemd dependencies for nfs services\n");
> > +		fprintf(stderr, "Usage: normal-dir early-dir late-dir\n");
> > +		exit(1);
> > +	}
> > +
> > +	conf_init();
> > +	s = conf_get_str("global", "pipefs-directory");
> > +	if (!s)
> > +		exit(0);
> > +	if (strlen(s) == strlen(RPC_PIPEFS_DEFAULT) &&
> > +			strcmp(s, RPC_PIPEFS_DEFAULT) == 0)
> > +		exit(0);
> > +
> > +	if (is_non_pipefs_mountpoint(s))
> > +		exit(1);
> > +
> > +	ret = generate_target(s, argv[1]);
> > +	exit(ret);
> > +}
> > diff --git a/systemd/rpc-svcgssd.service b/systemd/rpc-svcgssd.service
> > index 7187e3c..cb2bcd4 100644
> > --- a/systemd/rpc-svcgssd.service
> > +++ b/systemd/rpc-svcgssd.service
> > @@ -1,8 +1,7 @@
> >  [Unit]
> >  Description=RPC security service for NFS server
> >  DefaultDependencies=no
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > -After=var-lib-nfs-rpc_pipefs.mount local-fs.target
> > +After=local-fs.target
> >  PartOf=nfs-server.service
> >  PartOf=nfs-utils.service
> >  
> > diff --git a/systemd/rpc_pipefs.target b/systemd/rpc_pipefs.target
> > new file mode 100644
> > index 0000000..01d4d27
> > --- /dev/null
> > +++ b/systemd/rpc_pipefs.target
> > @@ -0,0 +1,3 @@
> > +[Unit]
> > +Requires=var-lib-nfs-rpc_pipefs.mount
> > +After=var-lib-nfs-rpc_pipefs.mount
> > -- 
> > 2.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nfs-utils PATCH v2 4/4] systemd: add a generator for the rpc_pipefs mountpoint

[Scott Mayhew]

On Thu, 06 Apr 2017, NeilBrown wrote:

> On Wed, Apr 05 2017, Scott Mayhew wrote:
> 
> > The nfs.conf has config options for the rpc_pipefs mountpoint.
> > Currently, changing these from the default also requires manually
> > overriding the systemd unit files that are hard-coded to mount the
> > filesystem on /var/lib/nfs/rpc_pipefs.
> >
> > This patch adds a generator that creates a mount unit file for the
> > rpc_pipefs when a non-default value is specified in /etc/nfs.conf, as
> > well as a target unit file to override the dependencies for the systemd
> > units using the rpc_pipefs.  The blkmapd, idmapd, and gssd service unit
> > files have been modified to define their dependencies on the rpc_pipefs
> > mountpoint indirectly via the rpc_pipefs target unit file.
> >
> > This patch also removes the dependency on the rpc_pipefs from the
> > rpc-svcgssd.service unit file.  rpc.svcgssd uses the sunrpc cache
> > mechanism to exchange data with the kernel, not the rpc_pipefs.
> >
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> 
> I was reading through this, which I really thought would be a good patch
> that I would give an Ack to, and looking at some of the detail involved,
> I started to wonder if we were really doing the right thing here.
> You go to some trouble to make the name of the .mount unit file match
> the name of the location of the mountpoint.  Is that really necessary?
> 
> Unit files created by systemd-fstab-generator do follow that pattern,
> but they don't have to.

> 
> Maybe we should just:
> 
> 1/ rename var-lib-nfs-rpc_pipefs.mount to rpc_pipefs.mount,
>    and modify the dependencies accordingly.
> 
The mount doesn't work when I change the name.

[root@fedora25 ~]# systemctl status rpc_pipefs.mount
● rpc_pipefs.mount - RPC Pipe File System
   Loaded: error (Reason: Invalid argument)
   Active: inactive (dead)
    Where: /var/lib/nfs/rpc_pipefs
     What: sunrpc

Apr 06 08:17:59 localhost.localdomain systemd[1]: rpc_pipefs.mount: Where= setting doesn't match unit name. Refusing.


Looking back at systemd.mount(5), it looks like they do have to follow
that pattern:

       Mount units must be named after the mount point directories they control.
       Example: the mount point /home/lennart must be configured in a unit file
       home-lennart.mount. For details about the escaping logic used to convert a
       file system path to a unit name, see systemd.unit(5). Note that mount units
       cannot be templated, nor is possible to add multiple names to a mount unit
       by creating additional symlinks to it.


-Scott

> 2/ write a generator which (if necessary) creates a drop-in for
>    rpc_pipefs.mount which simply overrides "Where".
> 
>  i.e /run/systemd/generator/rpc_pipefs.mount.d/pipefs.conf
>  would contain
> 
>     [Mount]
>     Where=/foo/bar

> 
> I'm sorry to discard your good work, but I now think that a lot of it is
> unnecessary.  Sorry for not seeing that earlier.
> 
> Thanks,
> NeilBrown
> 
> 
> > ---
> >  .gitignore                     |   1 +
> >  systemd/Makefile.am            |   5 +-
> >  systemd/nfs-blkmap.service     |   4 +-
> >  systemd/nfs-idmapd.service     |   4 +-
> >  systemd/rpc-gssd.service.in    |   4 +-
> >  systemd/rpc-pipefs-generator.c | 216 +++++++++++++++++++++++++++++++++++++++++
> >  systemd/rpc-svcgssd.service    |   3 +-
> >  systemd/rpc_pipefs.target      |   3 +
> >  8 files changed, 231 insertions(+), 9 deletions(-)
> >  create mode 100644 systemd/rpc-pipefs-generator.c
> >  create mode 100644 systemd/rpc_pipefs.target
> >
> > diff --git a/.gitignore b/.gitignore
> > index 126d12c..941aca0 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -70,6 +70,7 @@ tests/nsm_client/nlm_sm_inter_svc.c
> >  tests/nsm_client/nlm_sm_inter_xdr.c
> >  utils/nfsidmap/nfsidmap
> >  systemd/nfs-server-generator
> > +systemd/rpc-pipefs-generator
> >  systemd/nfs-config.service
> >  systemd/rpc-gssd.service
> >  # cscope database files
> > diff --git a/systemd/Makefile.am b/systemd/Makefile.am
> > index 0d15b9f..4becb77 100644
> > --- a/systemd/Makefile.am
> > +++ b/systemd/Makefile.am
> > @@ -4,6 +4,7 @@ MAINTAINERCLEANFILES = Makefile.in
> >  
> >  unit_files =  \
> >      nfs-client.target \
> > +    rpc_pipefs.target \
> >      \
> >      nfs-mountd.service \
> >      nfs-server.service \
> > @@ -42,12 +43,14 @@ EXTRA_DIST = $(unit_files) $(man5_MANS) $(man7_MANS)
> >  unit_dir = /usr/lib/systemd/system
> >  generator_dir = /usr/lib/systemd/system-generators
> >  
> > -EXTRA_PROGRAMS	= nfs-server-generator
> > +EXTRA_PROGRAMS	= nfs-server-generator rpc-pipefs-generator
> >  genexecdir = $(generator_dir)
> >  nfs_server_generator_LDADD = ../support/export/libexport.a \
> >  			     ../support/nfs/libnfs.a \
> >  			     ../support/misc/libmisc.a
> >  
> > +rpc_pipefs_generator_LDADD = ../support/nfs/libnfs.a
> > +
> >  if INSTALL_SYSTEMD
> >  genexec_PROGRAMS = nfs-server-generator
> >  install-data-hook: $(unit_files)
> > diff --git a/systemd/nfs-blkmap.service b/systemd/nfs-blkmap.service
> > index ddc324e..2bbcee6 100644
> > --- a/systemd/nfs-blkmap.service
> > +++ b/systemd/nfs-blkmap.service
> > @@ -2,8 +2,8 @@
> >  Description=pNFS block layout mapping daemon
> >  DefaultDependencies=no
> >  Conflicts=umount.target
> > -After=var-lib-nfs-rpc_pipefs.mount
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > +After=rpc_pipefs.target
> > +Requires=rpc_pipefs.target
> >  
> >  PartOf=nfs-utils.service
> >  
> > diff --git a/systemd/nfs-idmapd.service b/systemd/nfs-idmapd.service
> > index acca86b..f38fe52 100644
> > --- a/systemd/nfs-idmapd.service
> > +++ b/systemd/nfs-idmapd.service
> > @@ -1,8 +1,8 @@
> >  [Unit]
> >  Description=NFSv4 ID-name mapping service
> >  DefaultDependencies=no
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > -After=var-lib-nfs-rpc_pipefs.mount local-fs.target
> > +Requires=rpc_pipefs.target
> > +After=rpc_pipefs.target local-fs.target
> >  
> >  BindsTo=nfs-server.service
> >  
> > diff --git a/systemd/rpc-gssd.service.in b/systemd/rpc-gssd.service.in
> > index b353027..6807db3 100644
> > --- a/systemd/rpc-gssd.service.in
> > +++ b/systemd/rpc-gssd.service.in
> > @@ -2,8 +2,8 @@
> >  Description=RPC security service for NFS client and server
> >  DefaultDependencies=no
> >  Conflicts=umount.target
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > -After=var-lib-nfs-rpc_pipefs.mount
> > +Requires=rpc_pipefs.target
> > +After=rpc_pipefs.target
> >  
> >  ConditionPathExists=@_sysconfdir@/krb5.keytab
> >  
> > diff --git a/systemd/rpc-pipefs-generator.c b/systemd/rpc-pipefs-generator.c
> > new file mode 100644
> > index 0000000..b0a6c19
> > --- /dev/null
> > +++ b/systemd/rpc-pipefs-generator.c
> > @@ -0,0 +1,216 @@
> > +/*
> > + * rpc-pipefs-generator:
> > + *   systemd generator to create ordering dependencies between
> > + *   nfs services and the rpc_pipefs mountpoint
> > + */
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include <sys/stat.h>
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <ctype.h>
> > +#include <stdio.h>
> > +#include <mntent.h>
> > +
> > +#include "nfslib.h"
> > +#include "conffile.h"
> > +
> > +#define RPC_PIPEFS_DEFAULT "/var/lib/nfs/rpc_pipefs"
> > +char *conf_path = NFS_CONFFILE;
> > +
> > +int systemd_len(char *path)
> > +{
> > +	char *p;
> > +	int len = 0;
> > +
> > +	p = path;
> > +	while (*p == '/')
> > +		p++;
> > +
> > +	if (!*p)
> > +		/* "/" becomes "-", otherwise leading "/" is ignored */
> > +		return 1;
> > +
> > +	while (*p) {
> > +		unsigned char c = *p++;
> > +
> > +		if (c == '/') {
> > +			/* multiple non-trailing slashes become '-' */
> > +			while (*p == '/')
> > +				p++;
> > +			if (*p)
> > +				len++;
> > +		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
> > +			len++;
> > +		else {
> > +			len += 4;
> > +		}
> > +	}
> > +
> > +	return len;
> > +}
> > +
> > +char *systemd_escape(char *path)
> > +{
> > +	char *result = NULL;
> > +	char *p;
> > +	int len;
> > +
> > +	len = systemd_len(path);
> > +	if (!len)
> > +		goto out;
> > +
> > +	result = malloc(len + strlen(".mount") + 1);
> > +	if (!result)
> > +		goto out;
> > +
> > +	p = result;
> > +	while (*path == '/')
> > +		path++;
> > +	if (!*path) {
> > +		/* "/" becomes "-", otherwise leading "/" is ignored */
> > +		*p++ = '-';
> > +		goto out_append;
> > +	}
> > +	while (*path) {
> > +		unsigned char c = *path++;
> > +
> > +		if (c == '/') {
> > +			/* multiple non-trailing slashes become '-' */
> > +			while (*path == '/')
> > +				path++;
> > +			if (*path)
> > +				*p++ = '-';
> > +		} else if (isalnum(c) || c == ':' || c == '.' || c == '_')
> > +			*p++ = c;
> > +		else {
> > +			*p++ = '\\';
> > +			*p++ = 'x';
> > +			*p++ = '0' + ((c&0xf0)>>4) + (c>=0xa0)*('a'-'9'-1);
> > +			*p++ = '0' + (c&0x0f) + ((c&0x0f)>=0x0a)*('a'-'9'-1);
> > +		}
> > +	}
> > +
> > +out_append:
> > +	sprintf(p, ".mount");
> > +out:
> > +	return result;
> > +}
> > +
> > +static int generate_mount_unit(const char *pipefs_path, const char *pipefs_unit,
> > +			       const char *dirname)
> > +{
> > +	char	*path;
> > +	FILE	*f;
> > +
> > +	path = malloc(strlen(dirname) + 1 + strlen(pipefs_unit));
> > +	if (!path)
> > +		return 1;
> > +	sprintf(path, "%s/%s", dirname, pipefs_unit);
> > +	f = fopen(path, "w");
> > +	if (!f)
> > +		return 1;
> > +
> > +	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
> > +	fprintf(f, "Description=RPC Pipe File System\n");
> > +	fprintf(f, "DefaultDependencies=no\n");
> > +	fprintf(f, "After=systemd-tmpfiles-setup.service\n");
> > +	fprintf(f, "Conflicts=umount.target\n");
> > +	fprintf(f, "\n[Mount]\n");
> > +	fprintf(f, "What=sunrpc\n");
> > +	fprintf(f, "Where=%s\n", pipefs_path);
> > +	fprintf(f, "Type=rpc_pipefs\n");
> > +
> > +	fclose(f);
> > +	return 0;
> > +}
> > +
> > +static
> > +int generate_target(char *pipefs_path, const char *dirname)
> > +{
> > +	char	*path;
> > +	char	filebase[] = "/rpc_pipefs.target";
> > +	char	*pipefs_unit;
> > +	FILE	*f;
> > +	int 	ret = 0;
> > +
> > +	pipefs_unit = systemd_escape(pipefs_path);
> > +	if (!pipefs_unit)
> > +		return 1;
> > +
> > +	ret = generate_mount_unit(pipefs_path, pipefs_unit, dirname);
> > +	if (ret)
> > +		return ret;
> > +
> > +	path = malloc(strlen(dirname) + 1 + sizeof(filebase));
> > +	if (!path)
> > +		return 2;
> > +	sprintf(path, "%s", dirname);
> > +	mkdir(path, 0755);
> > +	strcat(path, filebase);
> > +	f = fopen(path, "w");
> > +	if (!f)
> > +		return 1;
> > +
> > +	fprintf(f, "# Automatically generated by rpc-pipefs-generator\n\n[Unit]\n");
> > +	fprintf(f, "Requires=%s\n", pipefs_unit);
> > +	fprintf(f, "After=%s\n", pipefs_unit);
> > +	fclose(f);
> > +
> > +	return 0;
> > +}
> > +
> > +static int is_non_pipefs_mountpoint(char *path)
> > +{
> > +	FILE		*mtab;
> > +	struct mntent	*mnt;
> > +
> > +	mtab = setmntent("/etc/mtab", "r");
> > +	if (!mtab)
> > +		return 0;
> > +
> > +	while ((mnt = getmntent(mtab)) != NULL) {
> > +		if (strlen(mnt->mnt_dir) != strlen(path))
> > +			continue;
> > +		if (strncmp(mnt->mnt_dir, path, strlen(mnt->mnt_dir)))
> > +			continue;
> > +		if (strncmp(mnt->mnt_type, "rpc_pipefs", strlen(mnt->mnt_type)))
> > +			break;
> > +	}
> > +	fclose(mtab);
> > +	return mnt != NULL;
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > +	int 	ret;
> > +	char	*s;
> > +
> > +	/* Avoid using any external services */
> > +	xlog_syslog(0);
> > +
> > +	if (argc != 4 || argv[1][0] != '/') {
> > +		fprintf(stderr, "rpc-pipefs-generator: create systemd dependencies for nfs services\n");
> > +		fprintf(stderr, "Usage: normal-dir early-dir late-dir\n");
> > +		exit(1);
> > +	}
> > +
> > +	conf_init();
> > +	s = conf_get_str("global", "pipefs-directory");
> > +	if (!s)
> > +		exit(0);
> > +	if (strlen(s) == strlen(RPC_PIPEFS_DEFAULT) &&
> > +			strcmp(s, RPC_PIPEFS_DEFAULT) == 0)
> > +		exit(0);
> > +
> > +	if (is_non_pipefs_mountpoint(s))
> > +		exit(1);
> > +
> > +	ret = generate_target(s, argv[1]);
> > +	exit(ret);
> > +}
> > diff --git a/systemd/rpc-svcgssd.service b/systemd/rpc-svcgssd.service
> > index 7187e3c..cb2bcd4 100644
> > --- a/systemd/rpc-svcgssd.service
> > +++ b/systemd/rpc-svcgssd.service
> > @@ -1,8 +1,7 @@
> >  [Unit]
> >  Description=RPC security service for NFS server
> >  DefaultDependencies=no
> > -Requires=var-lib-nfs-rpc_pipefs.mount
> > -After=var-lib-nfs-rpc_pipefs.mount local-fs.target
> > +After=local-fs.target
> >  PartOf=nfs-server.service
> >  PartOf=nfs-utils.service
> >  
> > diff --git a/systemd/rpc_pipefs.target b/systemd/rpc_pipefs.target
> > new file mode 100644
> > index 0000000..01d4d27
> > --- /dev/null
> > +++ b/systemd/rpc_pipefs.target
> > @@ -0,0 +1,3 @@
> > +[Unit]
> > +Requires=var-lib-nfs-rpc_pipefs.mount
> > +After=var-lib-nfs-rpc_pipefs.mount
> > -- 
> > 2.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [nfs-utils PATCH v2 4/4] systemd: add a generator for the rpc_pipefs mountpoint

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 3424 bytes --]

On Thu, Apr 06 2017, Scott Mayhew wrote:

> On Thu, 06 Apr 2017, NeilBrown wrote:
>
>> On Wed, Apr 05 2017, Scott Mayhew wrote:
>> 
>> > The nfs.conf has config options for the rpc_pipefs mountpoint.
>> > Currently, changing these from the default also requires manually
>> > overriding the systemd unit files that are hard-coded to mount the
>> > filesystem on /var/lib/nfs/rpc_pipefs.
>> >
>> > This patch adds a generator that creates a mount unit file for the
>> > rpc_pipefs when a non-default value is specified in /etc/nfs.conf, as
>> > well as a target unit file to override the dependencies for the systemd
>> > units using the rpc_pipefs.  The blkmapd, idmapd, and gssd service unit
>> > files have been modified to define their dependencies on the rpc_pipefs
>> > mountpoint indirectly via the rpc_pipefs target unit file.
>> >
>> > This patch also removes the dependency on the rpc_pipefs from the
>> > rpc-svcgssd.service unit file.  rpc.svcgssd uses the sunrpc cache
>> > mechanism to exchange data with the kernel, not the rpc_pipefs.
>> >
>> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
>> 
>> I was reading through this, which I really thought would be a good patch
>> that I would give an Ack to, and looking at some of the detail involved,
>> I started to wonder if we were really doing the right thing here.
>> You go to some trouble to make the name of the .mount unit file match
>> the name of the location of the mountpoint.  Is that really necessary?
>> 
>> Unit files created by systemd-fstab-generator do follow that pattern,
>> but they don't have to.
>
>> 
>> Maybe we should just:
>> 
>> 1/ rename var-lib-nfs-rpc_pipefs.mount to rpc_pipefs.mount,
>>    and modify the dependencies accordingly.
>> 
> The mount doesn't work when I change the name.
>
> [root@fedora25 ~]# systemctl status rpc_pipefs.mount
> ● rpc_pipefs.mount - RPC Pipe File System
>    Loaded: error (Reason: Invalid argument)
>    Active: inactive (dead)
>     Where: /var/lib/nfs/rpc_pipefs
>      What: sunrpc
>
> Apr 06 08:17:59 localhost.localdomain systemd[1]: rpc_pipefs.mount: Where= setting doesn't match unit name. Refusing.
>
>
> Looking back at systemd.mount(5), it looks like they do have to follow
> that pattern:
>
>        Mount units must be named after the mount point directories they control.
>        Example: the mount point /home/lennart must be configured in a unit file
>        home-lennart.mount. For details about the escaping logic used to convert a
>        file system path to a unit name, see systemd.unit(5). Note that mount units
>        cannot be templated, nor is possible to add multiple names to a mount unit
>        by creating additional symlinks to it.
>

My turn not to read to the end of the man page I see...

That's a strange restriction that seem inconsistent with everything else
systemd does.  Maybe it it helpful for implementing "RequiresMountsFor"
but I cannot think of any other justification.
Anyway, I guess we have to live with it.

I bothered me that you needed to include this:
>> > +	fprintf(f, "DefaultDependencies=no\n");
>> > +	fprintf(f, "After=systemd-tmpfiles-setup.service\n");
>> > +	fprintf(f, "Conflicts=umount.target\n");

in the code, but I can't see anything we can do about that.

So:
 Reviewed-by: NeilBrown <neilb@suse.com>

for this patch.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]