All of lore.kernel.org
 help / color / mirror / Atom feed
* [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint
@ 2017-04-06 16:31 Scott Mayhew
  2017-04-06 16:31 ` [nfs-utils PATCH v3 1/4] idmapd: move the pipefs-directory config option to nfs.conf Scott Mayhew
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Scott Mayhew @ 2017-04-06 16:31 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

Changes since v2: retained the name "general" for the global section of
the /etc/nfs.conf file.

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 /etc/nfs.conf.

The second patch adds a deprecation warning to  rpc.gssd if it reads its
pipefs-directory setting from the [gssd] section of /etc/nfs.conf
instead of from the [general] section.

The third patch allows blkmapd to read its pipefs-directory setting from
the [general] 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: add a deprecation warning for pipefs-directory in gssd section
  blkmapd:  allow the rpc_pipefs mountpoint to be overridden
  systemd: add a generator for the rpc_pipefs mountpoint

 .gitignore                       |   1 +
 nfs.conf                         |   6 +-
 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            |  36 +++++--
 utils/idmapd/idmapd.man          |  21 +++-
 16 files changed, 360 insertions(+), 36 deletions(-)
 create mode 100644 systemd/rpc-pipefs-generator.c
 create mode 100644 systemd/rpc_pipefs.target

-- 
2.9.3


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

* [nfs-utils PATCH v3 1/4] idmapd: move the pipefs-directory config option to nfs.conf
  2017-04-06 16:31 [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint Scott Mayhew
@ 2017-04-06 16:31 ` Scott Mayhew
  2017-04-06 16:31 ` [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section Scott Mayhew
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Scott Mayhew @ 2017-04-06 16:31 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

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.

Corrected a misspelling of 'configuration' in nfs.conf.

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

diff --git a/nfs.conf b/nfs.conf
index 81ece06..0828bdd 100644
--- a/nfs.conf
+++ b/nfs.conf
@@ -1,7 +1,10 @@
 #
-# This is a general conifguration for the 
+# This is a general configuration for the
 # NFS daemons and tools
 #
+#[general]
+# pipefs-directory=/var/lib/nfs/rpc_pipefs
+#
 #[exportfs]
 # debug=0
 #
diff --git a/systemd/nfs.conf.man b/systemd/nfs.conf.man
index bdc0988..e493ea3 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 general
+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..56bf67e 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"));
 		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"));
 	}
diff --git a/utils/idmapd/idmapd.man b/utils/idmapd/idmapd.man
index d4ab894..5f34d2b 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 [general]
+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


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

* [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section
  2017-04-06 16:31 [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint Scott Mayhew
  2017-04-06 16:31 ` [nfs-utils PATCH v3 1/4] idmapd: move the pipefs-directory config option to nfs.conf Scott Mayhew
@ 2017-04-06 16:31 ` Scott Mayhew
  2017-04-10 13:36   ` Steve Dickson
  2017-04-06 16:31 ` [nfs-utils PATCH v3 3/4] blkmapd: allow the rpc_pipefs mountpoint to be overridden Scott Mayhew
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Scott Mayhew @ 2017-04-06 16:31 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

All the daemons should use the same rpc_pipefs, so pipefs-directory
should be specified in the [general] 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, 14 insertions(+), 6 deletions(-)

diff --git a/nfs.conf b/nfs.conf
index 0828bdd..10d383d 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 e493ea3..1a72da0 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..28f9649 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -857,6 +857,10 @@ read_gss_conf(void)
 	s = conf_get_str("gssd", "pipefs-directory");
 	if (!s)
 		s = conf_get_str("general", "pipefs-directory");
+	else
+		printerr(0, "WARNING: Specifying pipefs-directory in the [gssd] "
+			 "section of %s is deprecated.  Use the [general] "
+			 "section instead.", NFS_CONFFILE);
 	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..e620f0d 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 [general]
+section:
+.TP
+.B pipefs-directory
+Equivalent to
+.BR -p .
 
 .SH SEE ALSO
 .BR rpc.svcgssd (8),
-- 
2.9.3


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

* [nfs-utils PATCH v3 3/4] blkmapd:  allow the rpc_pipefs mountpoint to be overridden
  2017-04-06 16:31 [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint Scott Mayhew
  2017-04-06 16:31 ` [nfs-utils PATCH v3 1/4] idmapd: move the pipefs-directory config option to nfs.conf Scott Mayhew
  2017-04-06 16:31 ` [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section Scott Mayhew
@ 2017-04-06 16:31 ` Scott Mayhew
  2017-04-10 13:36   ` Steve Dickson
  2017-04-06 16:31 ` [nfs-utils PATCH v3 4/4] systemd: add a generator for the rpc_pipefs mountpoint Scott Mayhew
  2017-04-11  0:40 ` [nfs-utils PATCH v3 0/4] add systemd " NeilBrown
  4 siblings, 1 reply; 12+ messages in thread
From: Scott Mayhew @ 2017-04-06 16:31 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

Allow the rpc_pipefs mountpoint to be overriden via the pipefs-directory
variable in the [general] 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 1a72da0..189b052 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..4b3d3f0 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 [general]
+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..d2da764 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("general", "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


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

* [nfs-utils PATCH v3 4/4] systemd: add a generator for the rpc_pipefs mountpoint
  2017-04-06 16:31 [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint Scott Mayhew
                   ` (2 preceding siblings ...)
  2017-04-06 16:31 ` [nfs-utils PATCH v3 3/4] blkmapd: allow the rpc_pipefs mountpoint to be overridden Scott Mayhew
@ 2017-04-06 16:31 ` Scott Mayhew
  2017-04-06 18:14   ` Steve Dickson
  2017-04-11  0:40 ` [nfs-utils PATCH v3 0/4] add systemd " NeilBrown
  4 siblings, 1 reply; 12+ messages in thread
From: Scott Mayhew @ 2017-04-06 16:31 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

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..512a464
--- /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("general", "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


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

* Re: [nfs-utils PATCH v3 4/4] systemd: add a generator for the rpc_pipefs mountpoint
  2017-04-06 16:31 ` [nfs-utils PATCH v3 4/4] systemd: add a generator for the rpc_pipefs mountpoint Scott Mayhew
@ 2017-04-06 18:14   ` Steve Dickson
  2017-04-07 15:04     ` Scott Mayhew
  2017-04-07 17:02     ` [nfs-utils PATCH v4] " Scott Mayhew
  0 siblings, 2 replies; 12+ messages in thread
From: Steve Dickson @ 2017-04-06 18:14 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: linux-nfs

Hello,

On 04/06/2017 12:31 PM, 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>
> ---
>  .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..512a464
> --- /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)
We already have one of these in nfs-server-generator.c
Is there any reason we can not used that one? 

I'm just concern that with every generator we'll get
another one of these routines... It is not a huge deal,
but hopefully we can start reusing this routines. 

At lease ad a header comment saying what this routine
is doing.... This systemd-generator stuff is all black 
magic IMHO..The more comments the better... 

> +{
> +	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);
Talk about black magic... What in world is going on here. 8-)

I'm sure I could stare at it for a while to figure it out but,
I would much rather have a comment telling what happening
or rewrite it in more straightforward way or both.

> +		}
> +	}
> +
> +out_append:
> +	sprintf(p, ".mount");
> +out:
> +	return result;
Finally the output of these two routines are different using the 
same path (/var/lib/nfs/rpc_pipefs) 

rpc-pipefs-generator generates 'var-lib-nfs-rpc_pipefs.mount'
nfs-server-generator generates 'var-lib-nfs-rpc\x5fpipefs.mount'

Should the '_' be turned into a \x5 or the other way around?

Note, since the rest of the patches are fine just repost
this patch with the appropriated  Message-Id using --in-reply-to
to keep it in the same thread. 

steved.
> +}
> +
> +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("general", "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
> 

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

* Re: [nfs-utils PATCH v3 4/4] systemd: add a generator for the rpc_pipefs mountpoint
  2017-04-06 18:14   ` Steve Dickson
@ 2017-04-07 15:04     ` Scott Mayhew
  2017-04-07 17:02     ` [nfs-utils PATCH v4] " Scott Mayhew
  1 sibling, 0 replies; 12+ messages in thread
From: Scott Mayhew @ 2017-04-07 15:04 UTC (permalink / raw)
  To: Steve Dickson; +Cc: linux-nfs

On Thu, 06 Apr 2017, Steve Dickson wrote:

> Hello,
> 
> On 04/06/2017 12:31 PM, 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>
> > ---
> >  .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..512a464
> > --- /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)
> We already have one of these in nfs-server-generator.c
> Is there any reason we can not used that one? 

The one in nfs-server-generator.c writes the value directly to a file.
I need the escaped value to be returned as a string because in addition
to writing the value out to an existing file, I need need to create a
file with that as the filename.
> 
> I'm just concern that with every generator we'll get
> another one of these routines... It is not a huge deal,
> but hopefully we can start reusing this routines. 

I'll make nfs-server-generator.c use the new function.
> 
> At lease ad a header comment saying what this routine
> is doing.... This systemd-generator stuff is all black 
> magic IMHO..The more comments the better... 

Will do.
> 
> > +{
> > +	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);
> Talk about black magic... What in world is going on here. 8-)
> 
> I'm sure I could stare at it for a while to figure it out but,
> I would much rather have a comment telling what happening
> or rewrite it in more straightforward way or both.

That's converting a character to a "\x2d" escape sequence.  FWIW I
pretty much lifted that from qword_addhex().  I'll make it clearer.
> 
> > +		}
> > +	}
> > +
> > +out_append:
> > +	sprintf(p, ".mount");
> > +out:
> > +	return result;
> Finally the output of these two routines are different using the 
> same path (/var/lib/nfs/rpc_pipefs) 
> 
> rpc-pipefs-generator generates 'var-lib-nfs-rpc_pipefs.mount'
> nfs-server-generator generates 'var-lib-nfs-rpc\x5fpipefs.mount'
> 
> Should the '_' be turned into a \x5 or the other way around?

The string generated by rpc-pipefs-generator is correct.  You can
compare it to the output of the systemd-escape(1) command 

$ systemd-escape -p --suffix=mount /var/lib/nfs/rpc_pipefs
var-lib-nfs-rpc_pipefs.mount

Also the function isn't properly escaping paths with a leading '.' --
I'll fix that.

-Scott
> 
> Note, since the rest of the patches are fine just repost
> this patch with the appropriated  Message-Id using --in-reply-to
> to keep it in the same thread. 
> 
> steved.
> > +}
> > +
> > +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("general", "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
> > 

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

* [nfs-utils PATCH v4] systemd: add a generator for the rpc_pipefs mountpoint
  2017-04-06 18:14   ` Steve Dickson
  2017-04-07 15:04     ` Scott Mayhew
@ 2017-04-07 17:02     ` Scott Mayhew
  2017-04-10 13:35       ` Steve Dickson
  1 sibling, 1 reply; 12+ messages in thread
From: Scott Mayhew @ 2017-04-07 17:02 UTC (permalink / raw)
  To: steved; +Cc: linux-nfs

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.  Since both
rpc-pipefs-generator.c and nfs-server-generator.c need to convert path
names to unit file names, that functionality has been moved to
systemd.c.

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            |  14 ++++-
 systemd/nfs-blkmap.service     |   4 +-
 systemd/nfs-idmapd.service     |   4 +-
 systemd/nfs-server-generator.c |  33 +---------
 systemd/rpc-gssd.service.in    |   4 +-
 systemd/rpc-pipefs-generator.c | 138 +++++++++++++++++++++++++++++++++++++++++
 systemd/rpc-svcgssd.service    |   3 +-
 systemd/rpc_pipefs.target      |   3 +
 systemd/systemd.c              | 133 +++++++++++++++++++++++++++++++++++++++
 systemd/systemd.h              |   6 ++
 11 files changed, 302 insertions(+), 41 deletions(-)
 create mode 100644 systemd/rpc-pipefs-generator.c
 create mode 100644 systemd/rpc_pipefs.target
 create mode 100644 systemd/systemd.c
 create mode 100644 systemd/systemd.h

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..eef53c4 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,14 +43,23 @@ 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)
+
+COMMON_SRCS = systemd.c systemd.h
+
+nfs_server_generator_SOURCES = $(COMMON_SRCS) nfs-server-generator.c
+
+rpc_pipefs_generator_SOURCES = $(COMMON_SRCS) rpc-pipefs-generator.c
+
 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
+genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
 install-data-hook: $(unit_files)
 	mkdir -p $(DESTDIR)/$(unitdir)
 	cp $(unit_files) $(DESTDIR)/$(unitdir)
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/nfs-server-generator.c b/systemd/nfs-server-generator.c
index 4aa6509..ce3d7fd 100644
--- a/systemd/nfs-server-generator.c
+++ b/systemd/nfs-server-generator.c
@@ -29,6 +29,7 @@
 #include "misc.h"
 #include "nfslib.h"
 #include "exportfs.h"
+#include "systemd.h"
 
 /* A simple "set of strings" to remove duplicates
  * found in /etc/exports
@@ -55,35 +56,6 @@ static int is_unique(struct list **lp, char *path)
 	return 1;
 }
 
-/* We need to convert a path name to a systemd unit
- * name.  This requires some translation ('/' -> '-')
- * and some escaping.
- */
-static void systemd_escape(FILE *f, char *path)
-{
-	while (*path == '/')
-		path++;
-	if (!*path) {
-		/* "/" becomes "-", otherwise leading "/" is ignored */
-		fputs("-", f);
-		return;
-	}
-	while (*path) {
-		char c = *path++;
-
-		if (c == '/') {
-			/* multiple non-trailing slashes become '-' */
-			while (*path == '/')
-				path++;
-			if (*path)
-				fputs("-", f);
-		} else if (isalnum(c) || c == ':' || c == '.')
-			fputc(c, f);
-		else
-			fprintf(f, "\\x%02x", c & 0xff);
-	}
-}
-
 static int has_noauto_flag(char *path)
 {
 	FILE		*fstab;
@@ -168,8 +140,7 @@ int main(int argc, char *argv[])
 		    strcmp(mnt->mnt_type, "nfs4") != 0)
 			continue;
 		fprintf(f, "Before= ");
-		systemd_escape(f, mnt->mnt_dir);
-		fprintf(f, ".mount\n");
+		fprintf(f, "%s\n", systemd_escape(mnt->mnt_dir, ".mount"));
 	}
 
 	fclose(fstab);
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..66addb9
--- /dev/null
+++ b/systemd/rpc-pipefs-generator.c
@@ -0,0 +1,138 @@
+/*
+ * 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"
+#include "systemd.h"
+
+#define RPC_PIPEFS_DEFAULT "/var/lib/nfs/rpc_pipefs"
+char *conf_path = NFS_CONFFILE;
+
+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, ".mount");
+	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("general", "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
diff --git a/systemd/systemd.c b/systemd/systemd.c
new file mode 100644
index 0000000..e10da52
--- /dev/null
+++ b/systemd/systemd.c
@@ -0,0 +1,133 @@
+/*
+ * Helper functions for systemd generators in nfs-utils.
+ *
+ * Currently just systemd_escape().
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <string.h>
+
+static const char hex[16] =
+{
+  '0', '1', '2', '3', '4', '5', '6', '7',
+  '8', '9', 'a', 'b', 'c', 'd', 'e', 'f',
+};
+
+/*
+ * determine length of the string that systemd_escape() needs to allocate
+ */
+static int systemd_len(char *path)
+{
+	char *p;
+	int len = 0;
+
+	p = path;
+	while (*p == '/')
+		/* multiple leading "/" are ignored */
+		p++;
+
+	if (!*p)
+		/* root directory "/" becomes is encoded as a single "-" */
+		return 1;
+
+	if (*p == '.')
+		/*
+		 * replace "." with "\x2d" escape sequence if
+		 * it's the first character in escaped path
+		 * */
+		len += 4;
+
+	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 == '_')
+			/* these characters are not replaced */
+			len++;
+		else
+			/* replace with "\x2d" escape sequence */
+			len += 4;
+	}
+
+	return len;
+}
+
+/*
+ * convert c to "\x2d" escape sequence and append to string
+ * at position p, advancing p
+ */
+static char *hexify(unsigned char c, char *p)
+{
+	*p++ = '\\';
+	*p++ = 'x';
+	*p++ = hex[c >> 4];
+	*p++ = hex[c & 0xf];
+	return p;
+}
+
+/*
+ * convert a path to a unit name according to the logic in systemd.unit(5):
+ *
+ *     Basically, given a path, "/" is replaced by "-", and all other
+ *     characters which are not ASCII alphanumerics are replaced by C-style
+ *     "\x2d" escapes (except that "_" is never replaced and "." is only
+ *     replaced when it would be the first character in the escaped path).
+ *     The root directory "/" is encoded as single dash, while otherwise the
+ *     initial and ending "/" are removed from all paths during
+ *     transformation.
+ *
+ * NB: Although the systemd.unit(5) doesn't mention it, the ':' character
+ * is not escaped.
+ */
+char *systemd_escape(char *path, char *suffix)
+{
+	char *result;
+	char *p;
+	int len;
+
+	len = systemd_len(path);
+	result = malloc(len + strlen(suffix) + 1);
+	p = result;
+	while (*path == '/')
+		/* multiple leading "/" are ignored */
+		path++;
+	if (!*path) {
+		/* root directory "/" becomes is encoded as a single "-" */
+		*p++ = '-';
+		goto out;
+	}
+	if (*path == '.')
+		/*
+		 * replace "." with "\x2d" escape sequence if
+		 * it's the first character in escaped path
+		 * */
+		p = hexify(*path++, p);
+
+	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 == '_')
+			/* these characters are not replaced */
+			*p++ = c;
+		else
+			/* replace with "\x2d" escape sequence */
+			p = hexify(c, p);
+	}
+
+out:
+	sprintf(p, suffix);
+	return result;
+}
diff --git a/systemd/systemd.h b/systemd/systemd.h
new file mode 100644
index 0000000..25235ec
--- /dev/null
+++ b/systemd/systemd.h
@@ -0,0 +1,6 @@
+#ifndef SYSTEMD_H
+#define SYSTEMD_H
+
+char *systemd_escape(char *path, char *suffix);
+
+#endif /* SYSTEMD_H */
-- 
2.9.3


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

* Re: [nfs-utils PATCH v4] systemd: add a generator for the rpc_pipefs mountpoint
  2017-04-07 17:02     ` [nfs-utils PATCH v4] " Scott Mayhew
@ 2017-04-10 13:35       ` Steve Dickson
  0 siblings, 0 replies; 12+ messages in thread
From: Steve Dickson @ 2017-04-10 13:35 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: linux-nfs



On 04/07/2017 01:02 PM, 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.  Since both
> rpc-pipefs-generator.c and nfs-server-generator.c need to convert path
> names to unit file names, that functionality has been moved to
> systemd.c.
> 
> 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>
Committed with a couple corrections... 

steved.
> ---
>  .gitignore                     |   1 +
>  systemd/Makefile.am            |  14 ++++-
>  systemd/nfs-blkmap.service     |   4 +-
>  systemd/nfs-idmapd.service     |   4 +-
>  systemd/nfs-server-generator.c |  33 +---------
>  systemd/rpc-gssd.service.in    |   4 +-
>  systemd/rpc-pipefs-generator.c | 138 +++++++++++++++++++++++++++++++++++++++++
>  systemd/rpc-svcgssd.service    |   3 +-
>  systemd/rpc_pipefs.target      |   3 +
>  systemd/systemd.c              | 133 +++++++++++++++++++++++++++++++++++++++
>  systemd/systemd.h              |   6 ++
>  11 files changed, 302 insertions(+), 41 deletions(-)
>  create mode 100644 systemd/rpc-pipefs-generator.c
>  create mode 100644 systemd/rpc_pipefs.target
>  create mode 100644 systemd/systemd.c
>  create mode 100644 systemd/systemd.h
> 
> 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..eef53c4 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,14 +43,23 @@ 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)
> +
> +COMMON_SRCS = systemd.c systemd.h
> +
> +nfs_server_generator_SOURCES = $(COMMON_SRCS) nfs-server-generator.c
> +
> +rpc_pipefs_generator_SOURCES = $(COMMON_SRCS) rpc-pipefs-generator.c
> +
>  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
> +genexec_PROGRAMS = nfs-server-generator rpc-pipefs-generator
>  install-data-hook: $(unit_files)
>  	mkdir -p $(DESTDIR)/$(unitdir)
>  	cp $(unit_files) $(DESTDIR)/$(unitdir)
> 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/nfs-server-generator.c b/systemd/nfs-server-generator.c
> index 4aa6509..ce3d7fd 100644
> --- a/systemd/nfs-server-generator.c
> +++ b/systemd/nfs-server-generator.c
> @@ -29,6 +29,7 @@
>  #include "misc.h"
>  #include "nfslib.h"
>  #include "exportfs.h"
> +#include "systemd.h"
>  
>  /* A simple "set of strings" to remove duplicates
>   * found in /etc/exports
> @@ -55,35 +56,6 @@ static int is_unique(struct list **lp, char *path)
>  	return 1;
>  }
>  
> -/* We need to convert a path name to a systemd unit
> - * name.  This requires some translation ('/' -> '-')
> - * and some escaping.
> - */
> -static void systemd_escape(FILE *f, char *path)
> -{
> -	while (*path == '/')
> -		path++;
> -	if (!*path) {
> -		/* "/" becomes "-", otherwise leading "/" is ignored */
> -		fputs("-", f);
> -		return;
> -	}
> -	while (*path) {
> -		char c = *path++;
> -
> -		if (c == '/') {
> -			/* multiple non-trailing slashes become '-' */
> -			while (*path == '/')
> -				path++;
> -			if (*path)
> -				fputs("-", f);
> -		} else if (isalnum(c) || c == ':' || c == '.')
> -			fputc(c, f);
> -		else
> -			fprintf(f, "\\x%02x", c & 0xff);
> -	}
> -}
> -
>  static int has_noauto_flag(char *path)
>  {
>  	FILE		*fstab;
> @@ -168,8 +140,7 @@ int main(int argc, char *argv[])
>  		    strcmp(mnt->mnt_type, "nfs4") != 0)
>  			continue;
>  		fprintf(f, "Before= ");
> -		systemd_escape(f, mnt->mnt_dir);
> -		fprintf(f, ".mount\n");
> +		fprintf(f, "%s\n", systemd_escape(mnt->mnt_dir, ".mount"));
>  	}
>  
>  	fclose(fstab);
> 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..66addb9
> --- /dev/null
> +++ b/systemd/rpc-pipefs-generator.c
> @@ -0,0 +1,138 @@
> +/*
> + * 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"
> +#include "systemd.h"
> +
> +#define RPC_PIPEFS_DEFAULT "/var/lib/nfs/rpc_pipefs"
> +char *conf_path = NFS_CONFFILE;
> +
> +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, ".mount");
> +	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("general", "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
> diff --git a/systemd/systemd.c b/systemd/systemd.c
> new file mode 100644
> index 0000000..e10da52
> --- /dev/null
> +++ b/systemd/systemd.c
> @@ -0,0 +1,133 @@
> +/*
> + * Helper functions for systemd generators in nfs-utils.
> + *
> + * Currently just systemd_escape().
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <ctype.h>
> +#include <string.h>
> +
> +static const char hex[16] =
> +{
> +  '0', '1', '2', '3', '4', '5', '6', '7',
> +  '8', '9', 'a', 'b', 'c', 'd', 'e', 'f',
> +};
> +
> +/*
> + * determine length of the string that systemd_escape() needs to allocate
> + */
> +static int systemd_len(char *path)
> +{
> +	char *p;
> +	int len = 0;
> +
> +	p = path;
> +	while (*p == '/')
> +		/* multiple leading "/" are ignored */
> +		p++;
> +
> +	if (!*p)
> +		/* root directory "/" becomes is encoded as a single "-" */
> +		return 1;
> +
> +	if (*p == '.')
> +		/*
> +		 * replace "." with "\x2d" escape sequence if
> +		 * it's the first character in escaped path
> +		 * */
> +		len += 4;
> +
> +	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 == '_')
> +			/* these characters are not replaced */
> +			len++;
> +		else
> +			/* replace with "\x2d" escape sequence */
> +			len += 4;
> +	}
> +
> +	return len;
> +}
> +
> +/*
> + * convert c to "\x2d" escape sequence and append to string
> + * at position p, advancing p
> + */
> +static char *hexify(unsigned char c, char *p)
> +{
> +	*p++ = '\\';
> +	*p++ = 'x';
> +	*p++ = hex[c >> 4];
> +	*p++ = hex[c & 0xf];
> +	return p;
> +}
> +
> +/*
> + * convert a path to a unit name according to the logic in systemd.unit(5):
> + *
> + *     Basically, given a path, "/" is replaced by "-", and all other
> + *     characters which are not ASCII alphanumerics are replaced by C-style
> + *     "\x2d" escapes (except that "_" is never replaced and "." is only
> + *     replaced when it would be the first character in the escaped path).
> + *     The root directory "/" is encoded as single dash, while otherwise the
> + *     initial and ending "/" are removed from all paths during
> + *     transformation.
> + *
> + * NB: Although the systemd.unit(5) doesn't mention it, the ':' character
> + * is not escaped.
> + */
> +char *systemd_escape(char *path, char *suffix)
> +{
> +	char *result;
> +	char *p;
> +	int len;
> +
> +	len = systemd_len(path);
> +	result = malloc(len + strlen(suffix) + 1);
> +	p = result;
> +	while (*path == '/')
> +		/* multiple leading "/" are ignored */
> +		path++;
> +	if (!*path) {
> +		/* root directory "/" becomes is encoded as a single "-" */
> +		*p++ = '-';
> +		goto out;
> +	}
> +	if (*path == '.')
> +		/*
> +		 * replace "." with "\x2d" escape sequence if
> +		 * it's the first character in escaped path
> +		 * */
> +		p = hexify(*path++, p);
> +
> +	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 == '_')
> +			/* these characters are not replaced */
> +			*p++ = c;
> +		else
> +			/* replace with "\x2d" escape sequence */
> +			p = hexify(c, p);
> +	}
> +
> +out:
> +	sprintf(p, suffix);
> +	return result;
> +}
> diff --git a/systemd/systemd.h b/systemd/systemd.h
> new file mode 100644
> index 0000000..25235ec
> --- /dev/null
> +++ b/systemd/systemd.h
> @@ -0,0 +1,6 @@
> +#ifndef SYSTEMD_H
> +#define SYSTEMD_H
> +
> +char *systemd_escape(char *path, char *suffix);
> +
> +#endif /* SYSTEMD_H */
> 

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

* Re: [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section
  2017-04-06 16:31 ` [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section Scott Mayhew
@ 2017-04-10 13:36   ` Steve Dickson
  0 siblings, 0 replies; 12+ messages in thread
From: Steve Dickson @ 2017-04-10 13:36 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: linux-nfs



On 04/06/2017 12:31 PM, Scott Mayhew wrote:
> All the daemons should use the same rpc_pipefs, so pipefs-directory
> should be specified in the [general] section.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Committed... 

steved.
> ---
>  nfs.conf             |  1 -
>  systemd/nfs.conf.man |  3 ++-
>  utils/gssd/gssd.c    |  4 ++++
>  utils/gssd/gssd.man  | 12 ++++++++----
>  4 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/nfs.conf b/nfs.conf
> index 0828bdd..10d383d 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 e493ea3..1a72da0 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..28f9649 100644
> --- a/utils/gssd/gssd.c
> +++ b/utils/gssd/gssd.c
> @@ -857,6 +857,10 @@ read_gss_conf(void)
>  	s = conf_get_str("gssd", "pipefs-directory");
>  	if (!s)
>  		s = conf_get_str("general", "pipefs-directory");
> +	else
> +		printerr(0, "WARNING: Specifying pipefs-directory in the [gssd] "
> +			 "section of %s is deprecated.  Use the [general] "
> +			 "section instead.", NFS_CONFFILE);
>  	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..e620f0d 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 [general]
> +section:
> +.TP
> +.B pipefs-directory
> +Equivalent to
> +.BR -p .
>  
>  .SH SEE ALSO
>  .BR rpc.svcgssd (8),
> 

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

* Re: [nfs-utils PATCH v3 3/4] blkmapd: allow the rpc_pipefs mountpoint to be overridden
  2017-04-06 16:31 ` [nfs-utils PATCH v3 3/4] blkmapd: allow the rpc_pipefs mountpoint to be overridden Scott Mayhew
@ 2017-04-10 13:36   ` Steve Dickson
  0 siblings, 0 replies; 12+ messages in thread
From: Steve Dickson @ 2017-04-10 13:36 UTC (permalink / raw)
  To: Scott Mayhew; +Cc: linux-nfs



On 04/06/2017 12:31 PM, Scott Mayhew wrote:
> Allow the rpc_pipefs mountpoint to be overriden via the pipefs-directory
> variable in the [general] section of /etc/nfs.conf.
> 
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Committed...

steved.
> ---
>  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 1a72da0..189b052 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..4b3d3f0 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 [general]
> +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..d2da764 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("general", "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 */
> 

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

* Re: [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint
  2017-04-06 16:31 [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint Scott Mayhew
                   ` (3 preceding siblings ...)
  2017-04-06 16:31 ` [nfs-utils PATCH v3 4/4] systemd: add a generator for the rpc_pipefs mountpoint Scott Mayhew
@ 2017-04-11  0:40 ` NeilBrown
  4 siblings, 0 replies; 12+ messages in thread
From: NeilBrown @ 2017-04-11  0:40 UTC (permalink / raw)
  To: Scott Mayhew, steved; +Cc: linux-nfs

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

On Thu, Apr 06 2017, Scott Mayhew wrote:

> Changes since v2: retained the name "general" for the global section of
> the /etc/nfs.conf file.
>
> 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 /etc/nfs.conf.
>
> The second patch adds a deprecation warning to  rpc.gssd if it reads its
> pipefs-directory setting from the [gssd] section of /etc/nfs.conf
> instead of from the [general] section.
>
> The third patch allows blkmapd to read its pipefs-directory setting from
> the [general] 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: add a deprecation warning for pipefs-directory in gssd section
>   blkmapd:  allow the rpc_pipefs mountpoint to be overridden
>   systemd: add a generator for the rpc_pipefs mountpoint

Looks good.  Thanks for persisting with this!

NeilBrown

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

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

end of thread, other threads:[~2017-04-11  0:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-06 16:31 [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint Scott Mayhew
2017-04-06 16:31 ` [nfs-utils PATCH v3 1/4] idmapd: move the pipefs-directory config option to nfs.conf Scott Mayhew
2017-04-06 16:31 ` [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section Scott Mayhew
2017-04-10 13:36   ` Steve Dickson
2017-04-06 16:31 ` [nfs-utils PATCH v3 3/4] blkmapd: allow the rpc_pipefs mountpoint to be overridden Scott Mayhew
2017-04-10 13:36   ` Steve Dickson
2017-04-06 16:31 ` [nfs-utils PATCH v3 4/4] systemd: add a generator for the rpc_pipefs mountpoint Scott Mayhew
2017-04-06 18:14   ` Steve Dickson
2017-04-07 15:04     ` Scott Mayhew
2017-04-07 17:02     ` [nfs-utils PATCH v4] " Scott Mayhew
2017-04-10 13:35       ` Steve Dickson
2017-04-11  0:40 ` [nfs-utils PATCH v3 0/4] add systemd " NeilBrown

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