Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:44378 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751963AbdDCSvR (ORCPT ); Mon, 3 Apr 2017 14:51:17 -0400 Date: Mon, 3 Apr 2017 14:51:15 -0400 From: Scott Mayhew To: NeilBrown Cc: steved@redhat.com, linux-nfs@vger.kernel.org Subject: Re: [RFC nfs-utils PATCH 0/2] add systemd generator for the rpc_pipefs mountpoint Message-ID: <20170403185115.tvouk4k3nrfoxmiu@tonberry.usersys.redhat.com> References: <20170331215654.31570-1-smayhew@redhat.com> <87lgrikuue.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 In-Reply-To: <87lgrikuue.fsf@notabene.neil.brown.name> Sender: linux-nfs-owner@vger.kernel.org List-ID: 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