Re: [RFC nfs-utils PATCH 0/2] add systemd generator for the rpc_pipefs mountpoint

[Scott Mayhew <smayhew@redhat.com>]

On Mon, 03 Apr 2017, NeilBrown wrote:

> On Fri, Mar 31 2017, Scott Mayhew wrote:
> 
> > 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, which rpc.gssd
> > already can use for it's pipefs-directory configuration.
> >
> > The second patch adds a systemd generator that reads the
> > pipefs-directory configurations from /etc/nfs.conf, and if they differ
> > from the default it will automatically 1) create a systemd mount unit
> > file for the pipefs mountpoint and 2) it will create a drop-in
> > configuration file to override the Requires= and After= directives for
> > that service.
> >
> > I did run into a bit of a snag though.  Depsite overriding the
> > dependencies for both idmapd and gssd, I wind up with two pipefs
> > filesystems mounted:
> >
> > [root@coeurl ~]# grep pipefs /proc/mounts
> > sunrpc /var/lib/nfs/rpc_pipefs rpc_pipefs rw,relatime 0 0
> > sunrpc /run/rpc_pipefs rpc_pipefs rw,relatime 0 0
> >
> > systemd still shows the dependency on the default pipefs mountpoint:
> >
> > [root@coeurl ~]# systemctl list-dependencies --before var-lib-nfs-rpc_pipefs.mount
> > var-lib-nfs-rpc_pipefs.mount
> > ● ├─nfs-idmapd.service
> > ● └─rpc-gssd.service
> >
> > as well as the new one:
> >
> > [root@coeurl ~]# systemctl list-dependencies --before run-rpc_pipefs.mount
> > run-rpc_pipefs.mount
> > ● ├─nfs-idmapd.service
> > ● └─rpc-gssd.service
> >
> > The drop-in configs to override the pipefs mountpoint look correct.  I'm
> > clearing both Requires= and After= before setting them:
> >
> > [root@coeurl ~]# cat /run/systemd/generator/nfs-idmapd.service.d/10-pipefs.conf 
> > # Automatically generated by rpc-pipefs-generator
> >
> > [Unit]
> > Requires=
> > Requires=run-rpc_pipefs.mount
> > After=
> > After=run-rpc_pipefs.mount local-fs.target
> >
> > [root@coeurl ~]# cat /run/systemd/generator/rpc-gssd.service.d/10-pipefs.conf 
> > # Automatically generated by rpc-pipefs-generator
> >
> > [Unit]
> > Requires=
> > Requires=run-rpc_pipefs.mount
> > After=
> > After=run-rpc_pipefs.mount
> >
> > The generated mount unit file also looks correct:
> >
> > [root@coeurl ~]# cat /run/systemd/generator/run-rpc_pipefs.mount 
> > # Automatically generated by rpc-pipefs-generator
> >
> > [Unit]
> > Description=RPC Pipe File System
> > DefaultDependencies=no
> > After=systemd-tmpfiles-setup.service
> > Conflicts=umount.target
> >
> > [Mount]
> > What=sunrpc
> > Where=/run/rpc_pipefs
> > Type=rpc_pipefs
> >
> > systemd shows that the drop-in config was picked up:
> >
> > [root@coeurl ~]# systemctl status nfs-idmapd
> > ● nfs-idmapd.service - NFSv4 ID-name mapping service
> >    Loaded: loaded (/usr/lib/systemd/system/nfs-idmapd.service; static; vendor preset: disabled)
> >   Drop-In: /run/systemd/generator/nfs-idmapd.service.d
> >            └─10-pipefs.conf
> >    Active: active (running) since Fri 2017-03-31 16:54:24 EDT; 5min ago
> >   Process: 27831 ExecStart=/usr/sbin/rpc.idmapd $RPCIDMAPDARGS (code=exited, status=0/SUCCESS)
> >  Main PID: 27832 (rpc.idmapd)
> >     Tasks: 1 (limit: 4915)
> >    CGroup: /system.slice/nfs-idmapd.service
> >            └─27832 /usr/sbin/rpc.idmapd
> >
> > and lsof shows that the correct mountpoint is being used:
> >
> > [root@coeurl ~]# lsof -p 27832 2>/dev/null | grep pipefs
> > rpc.idmap 27832 root   10r      DIR               0,42        0        103 /run/rpc_pipefs/nfs
> >
> > The same for gssd:
> >
> > [root@coeurl ~]# systemctl status rpc-gssd
> > ● rpc-gssd.service - RPC security service for NFS client and server
> >    Loaded: loaded (/usr/lib/systemd/system/rpc-gssd.service; static; vendor preset: disabled)
> >   Drop-In: /run/systemd/generator/rpc-gssd.service.d
> >            └─10-pipefs.conf
> >    Active: active (running) since Fri 2017-03-31 16:54:29 EDT; 6min ago
> >   Process: 27839 ExecStart=/usr/sbin/rpc.gssd $RPCGSSDARGS (code=exited, status=0/SUCCESS)
> >  Main PID: 27840 (rpc.gssd)
> >     Tasks: 1 (limit: 4915)
> >    CGroup: /system.slice/rpc-gssd.service
> >            └─27840 /usr/sbin/rpc.gssd
> >
> > [root@coeurl ~]# lsof -p 27840 2>/dev/null | grep pipefs
> > rpc.gssd 27840 root  cwd       DIR               0,42        0   24637 /run/rpc_pipefs
> > rpc.gssd 27840 root    7r      DIR               0,42        0   24637 /run/rpc_pipefs
> > rpc.gssd 27840 root   11u     FIFO               0,42      0t0     112 /run/rpc_pipefs/gssd/clntXX/gssd
> >
> > So it looks like systemd is using both sets of dependencies, even though
> > the programs themselves are only looking for what's specified in
> > /etc/nfs.conf.  I'm not sure what to do about that.  Maybe remove the
> > var-lib-nfs-rpc_pipefs.mount unit as well as the dependencies in the
> > nfs-idmapd.service and rpc-gssd.service files, and have the generator
> > create those automatically as well?
> >
> 
> Towards the end of the systemd.unit man page is the text:
> 
>       Note that dependencies (After=, etc.) cannot be reset to an empty list,
>        so dependencies can only be added in drop-ins. If you want to remove
>        dependencies, you have to override the entire unit.
> 
> 
> which is consistent with what you discovered.

Doh, that's what I get for not reading all the way to the end.

> 
> Maybe create a "rpc_pipefs.target" which
>   Requires=var-lib-nfs-rpc_pipefs.mount
>   After=var-lib-nfs-rpc_pipefs.mount
> and have all the other unit files specified their dependencies
> against this target.
> 
> Then your generator would conditionally create a new "rpc_pipefs.target"
> and matching foo.mount.  The new .target would depend in the foo.mount,
> and the service files would already depend on that.
> 
> Might work.

That looks like it works, but what should happen then if programs
were to intentionally specify two different values for the pipefs-directory
in nfs.conf?  Or is that something that wouldn't make sense to do and
should be prevented?  I can't think of a reason why you'd want more than
one rpc_pipefs filesystem mounted...

Maybe it would make sense to create a 'global' section and have the
programs all look at that instead of looking in their specific config
sections... or maybe have the program check global section and then a
program-specific section, but include a big fat warning that if you
specify it in the program-specific section then you need to override all
of them with the same value.

-Scott
> 
> Thanks,
> NeilBrown
> 
> 
> > -Scott
> >
> > Scott Mayhew (2):
> >   idmapd: move the pipefs-directory config option to nfs.conf
> >   systemd: add a generator for the rpc_pipefs mountpoint
> >
> >  .gitignore                     |   1 +
> >  nfs.conf                       |   3 +
> >  systemd/Makefile.am            |   4 +-
> >  systemd/nfs.conf.man           |   9 ++
> >  systemd/rpc-pipefs-generator.c | 256 +++++++++++++++++++++++++++++++++++++++++
> >  systemd/rpc-svcgssd.service    |   3 +-
> >  utils/idmapd/idmapd.c          |  35 +++---
> >  utils/idmapd/idmapd.man        |  19 ++-
> >  8 files changed, 305 insertions(+), 25 deletions(-)
> >  create mode 100644 systemd/rpc-pipefs-generator.c
> >
> > -- 
> > 2.9.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html