LinuxLists.cc - [nfs-utils PATCH v3 0/4] add systemd generator for the rpc

2017-04-06 16:31:05

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

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

2017-04-06 16:31:05

by Scott Mayhew

[permalink] [raw]

Subject: [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section

All the daemons should use the same rpc_pipefs, so pipefs-directory
should be specified in the [general] section.

Signed-off-by: Scott Mayhew <[email protected]>
---
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

2017-04-06 16:31:05

by Scott Mayhew

[permalink] [raw]

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

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 <[email protected]>
---
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

2017-04-06 16:31:05

by Scott Mayhew

[permalink] [raw]

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

2017-04-06 16:31:05

by Scott Mayhew

[permalink] [raw]

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

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 <[email protected]>
---
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

2017-04-06 18:14:13

by Steve Dickson

[permalink] [raw]

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

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 <[email protected]>
> ---
> .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
>

2017-04-07 15:04:37

by Scott Mayhew

[permalink] [raw]

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

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 <[email protected]>
> > ---
> > .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
> >

2017-04-07 17:02:18

by Scott Mayhew

[permalink] [raw]

Subject: [nfs-utils PATCH v4] systemd: add a generator for the rpc_pipefs mountpoint

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 <[email protected]>
---
.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

2017-04-10 13:35:50

by Steve Dickson

[permalink] [raw]

Subject: Re: [nfs-utils PATCH v4] systemd: add a generator for the rpc_pipefs mountpoint

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 <[email protected]>
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 */
>

2017-04-10 13:36:30

by Steve Dickson

[permalink] [raw]

Subject: Re: [nfs-utils PATCH v3 2/4] gssd: add a deprecation warning for pipefs-directory in gssd section

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 <[email protected]>
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),
>

2017-04-10 13:36:50

by Steve Dickson

[permalink] [raw]

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

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 <[email protected]>
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 */
>

2017-04-11 00:40:19

by NeilBrown

[permalink] [raw]

Subject: Re: [nfs-utils PATCH v3 0/4] add systemd generator for the rpc_pipefs mountpoint

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

Attachments:

signature.asc (832.00 B)