Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:48906 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750892AbcCOPNh (ORCPT ); Tue, 15 Mar 2016 11:13:37 -0400 Date: Tue, 15 Mar 2016 11:13:33 -0400 (EDT) From: Benjamin Coddington To: "J. Bruce Fields" cc: steved@redhat.com, linux-nfs@vger.kernel.org, neilb@suse.com Subject: Re: [PATCH nfs-utils] systemd: remove the nfs-config service In-Reply-To: <20160315142432.GB419@fieldses.org> Message-ID: References: <20160315142432.GB419@fieldses.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-nfs-owner@vger.kernel.org List-ID: On Tue, 15 Mar 2016, J. Bruce Fields wrote: > Also cc'ing Neil, as he added nfs-config originally.--b. Ah, thanks Bruce! I should have looked up the original author. For a point of reference on the work needed within a distro: here's the smallest change required for Fedora's nfs-utils_env.sh: --- /usr/lib/systemd/scripts/nfs-utils_env.sh 2016-03-15 11:08:34.641326549 -0400 +++ /usr/lib/systemd/scripts/nfs-utils_env.sh.orig 2016-03-15 11:07:10.974514047 -0400 @@ -67,4 +67,4 @@ echo SVCGSSDARGS=\"$RPCSVCGSSDARGS\" echo BLKMAPDARGS=\"$BLKMAPDARGS\" echo GSS_USE_PROXY=\"$GSS_USE_PROXY\" -} > /run/sysconfig/nfs-utils +} > /run/sysconfig/nfs-utils-${SERVICE} This small change just generates the entire set of variables into a separate file for each service, so it is quite dumb. More sensible would be to only generate the required set of environment variables for each service. Ben > On Sat, Mar 12, 2016 at 07:05:48AM -0500, Benjamin Coddington wrote: > > The nfs-config service exists to translate distro-specific startup > > configuration into the command line arguments used by systemd to start nfs > > utilities. Unfortunately, it is not clear to most users that this service > > must be recycled every time startup configurations have been modified, as > > this requirement is a change for at least one distro. > > > > We can get rid of nfs-config by generating the startup arguments in an > > ExecStartPre option for each service that needs them. We'll also have to > > break out the EnvironmentFile into a separate file for each service to > > avoid races overwriting this file. > > > > A distro taking this change should also modify their > > /usr/lib/systemd/scripts/nfs-utils_env.sh script to include the new > > EnvironmentFile location for each service. > > > > Signed-off-by: Benjamin Coddington > > --- > > systemd/Makefile.am | 1 - > > systemd/README | 8 +++++--- > > systemd/nfs-blkmap.service | 4 +++- > > systemd/nfs-idmapd.service | 7 +++---- > > systemd/nfs-mountd.service | 7 +++---- > > systemd/nfs-server.service | 7 +++---- > > systemd/rpc-gssd.service | 7 +++---- > > systemd/rpc-statd-notify.service | 7 +++---- > > systemd/rpc-statd.service | 7 +++---- > > systemd/rpc-svcgssd.service | 7 +++---- > > 10 files changed, 29 insertions(+), 33 deletions(-) > > > > diff --git a/systemd/Makefile.am b/systemd/Makefile.am > > index 03f96e9..6f59dfc 100644 > > --- a/systemd/Makefile.am > > +++ b/systemd/Makefile.am > > @@ -5,7 +5,6 @@ MAINTAINERCLEANFILES = Makefile.in > > unit_files = \ > > nfs-client.target \ > > \ > > - nfs-config.service \ > > nfs-mountd.service \ > > nfs-server.service \ > > nfs-utils.service \ > > diff --git a/systemd/README b/systemd/README > > index bbd7790..1508ea7 100644 > > --- a/systemd/README > > +++ b/systemd/README > > @@ -54,9 +54,11 @@ client and systemd cannot specify is two-pronged reverse dependency. > > > > Distro specific commandline configuration can be provided by > > installing a script /usr/lib/systemd/scripts/nfs-utils_env.sh > > -This should write /run/sysconfig/nfs-utils based on configuration > > -information such as in /etc/sysconfig/nfs or /etc/defaults/nfs. > > -It is run once by nfs-config.service. > > +It will be provided the environment variable SERVICE with the short > > +name of the service being started, and should write > > +/run/sysconfig/nfs-utils-${SERVICE} based on configuration > > +information such as in /etc/sysconfig/nfs or /etc/defaults/nfs. It > > +is run just prior to starting each service. > > > > rpc.gssd and rpc.svcgssd are assumed to be needed if /etc/krb5.keytab > > is present. > > diff --git a/systemd/nfs-blkmap.service b/systemd/nfs-blkmap.service > > index ddbf4e9..254b005 100644 > > --- a/systemd/nfs-blkmap.service > > +++ b/systemd/nfs-blkmap.service > > @@ -10,7 +10,9 @@ PartOf=nfs-utils.service > > [Service] > > Type=forking > > PIDFile=/var/run/blkmapd.pid > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=blkmapd" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-blkmapd > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStart=/usr/sbin/blkmapd $BLKMAPDARGS > > > > [Install] > > diff --git a/systemd/nfs-idmapd.service b/systemd/nfs-idmapd.service > > index df3dd9d..f58a58d 100644 > > --- a/systemd/nfs-idmapd.service > > +++ b/systemd/nfs-idmapd.service > > @@ -6,10 +6,9 @@ After=var-lib-nfs-rpc_pipefs.mount local-fs.target > > > > BindsTo=nfs-server.service > > > > -Wants=nfs-config.service > > -After=nfs-config.service > > - > > [Service] > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=idmapd" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-idmapd > > Type=forking > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStart=/usr/sbin/rpc.idmapd $RPCIDMAPDARGS > > diff --git a/systemd/nfs-mountd.service b/systemd/nfs-mountd.service > > index 8a39f3e..5392429 100644 > > --- a/systemd/nfs-mountd.service > > +++ b/systemd/nfs-mountd.service > > @@ -6,10 +6,9 @@ After=proc-fs-nfsd.mount > > After=network.target local-fs.target > > BindsTo=nfs-server.service > > > > -Wants=nfs-config.service > > -After=nfs-config.service > > - > > [Service] > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=mountd" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-mountd > > Type=forking > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStart=/usr/sbin/rpc.mountd $RPCMOUNTDARGS > > diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service > > index 317e5d6..493e41e 100644 > > --- a/systemd/nfs-server.service > > +++ b/systemd/nfs-server.service > > @@ -18,14 +18,13 @@ After=rpc-gssd.service gssproxy.service rpc-svcgssd.service > > # start/stop server before/after client > > Before=remote-fs-pre.target > > > > -Wants=nfs-config.service > > -After=nfs-config.service > > - > > [Service] > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=nfsd" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-nfsd > > > > Type=oneshot > > RemainAfterExit=yes > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStartPre=/usr/sbin/exportfs -r > > ExecStart=/usr/sbin/rpc.nfsd $RPCNFSDARGS > > ExecStop=/usr/sbin/rpc.nfsd 0 > > diff --git a/systemd/rpc-gssd.service b/systemd/rpc-gssd.service > > index d4a3819..f945661 100644 > > --- a/systemd/rpc-gssd.service > > +++ b/systemd/rpc-gssd.service > > @@ -9,11 +9,10 @@ ConditionPathExists=/etc/krb5.keytab > > > > PartOf=nfs-utils.service > > > > -Wants=nfs-config.service > > -After=nfs-config.service > > - > > [Service] > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=gssd" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-gssd > > > > Type=forking > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStart=/usr/sbin/rpc.gssd $GSSDARGS > > diff --git a/systemd/rpc-statd-notify.service b/systemd/rpc-statd-notify.service > > index 89ba36c..b33c92e 100644 > > --- a/systemd/rpc-statd-notify.service > > +++ b/systemd/rpc-statd-notify.service > > @@ -10,10 +10,9 @@ After=nfs-server.service > > > > PartOf=nfs-utils.service > > > > -Wants=nfs-config.service > > -After=nfs-config.service > > - > > [Service] > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=sm-notify" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-sm-notify > > Type=forking > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStart=-/usr/sbin/sm-notify $SMNOTIFYARGS > > diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service > > index f16ea42..1af3bc8 100644 > > --- a/systemd/rpc-statd.service > > +++ b/systemd/rpc-statd.service > > @@ -7,11 +7,10 @@ After=network.target nss-lookup.target rpcbind.service > > > > PartOf=nfs-utils.service > > > > -Wants=nfs-config.service > > -After=nfs-config.service > > - > > [Service] > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=statd" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-statd > > Type=forking > > PIDFile=/var/run/rpc.statd.pid > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStart=/usr/sbin/rpc.statd --no-notify $STATDARGS > > diff --git a/systemd/rpc-svcgssd.service b/systemd/rpc-svcgssd.service > > index 41177b6..ace6ec5 100644 > > --- a/systemd/rpc-svcgssd.service > > +++ b/systemd/rpc-svcgssd.service > > @@ -11,10 +11,9 @@ ConditionPathExists=|!/run/gssproxy.pid > > ConditionPathExists=|!/proc/net/rpc/use-gss-proxy > > ConditionPathExists=/etc/krb5.keytab > > > > -Wants=nfs-config.service > > -After=nfs-config.service > > - > > [Service] > > -EnvironmentFile=-/run/sysconfig/nfs-utils > > +Environment="SERVICE=svcgssd" > > +EnvironmentFile=-/run/sysconfig/nfs-utils-svcgssd > > Type=forking > > +ExecStartPre=-/usr/lib/systemd/scripts/nfs-utils_env.sh > > ExecStart=/usr/sbin/rpc.svcgssd $SVCGSSDARGS > > -- > > 1.7.1 > > > > -- > > 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 >