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

Commit Message

Scott Mayhew April 6, 2017, 4:31 p.m. UTC

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 <[e-mail]>
---
 .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

Comments

Steve Dickson April 6, 2017, 6:14 p.m. UTC | #1

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 <[e-mail]>
> ---
>  .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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Scott Mayhew April 7, 2017, 3:04 p.m. UTC | #2

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 <[e-mail]>
> > ---
> >  .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
> > 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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