* [PATCH nfs-utils] systemd: remove the nfs-config service @ 2016-03-12 12:05 Benjamin Coddington 2016-03-15 14:24 ` J. Bruce Fields 2016-03-15 20:59 ` NeilBrown 0 siblings, 2 replies; 11+ messages in thread From: Benjamin Coddington @ 2016-03-12 12:05 UTC (permalink / raw) To: steved; +Cc: linux-nfs 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 <bcodding@redhat.com> --- 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-12 12:05 [PATCH nfs-utils] systemd: remove the nfs-config service Benjamin Coddington @ 2016-03-15 14:24 ` J. Bruce Fields 2016-03-15 15:13 ` Benjamin Coddington 2016-03-15 23:33 ` Malahal Naineni 2016-03-15 20:59 ` NeilBrown 1 sibling, 2 replies; 11+ messages in thread From: J. Bruce Fields @ 2016-03-15 14:24 UTC (permalink / raw) To: Benjamin Coddington; +Cc: steved, linux-nfs, neilb Also cc'ing Neil, as he added nfs-config originally.--b. 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 <bcodding@redhat.com> > --- > 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-15 14:24 ` J. Bruce Fields @ 2016-03-15 15:13 ` Benjamin Coddington 2016-03-15 23:33 ` Malahal Naineni 1 sibling, 0 replies; 11+ messages in thread From: Benjamin Coddington @ 2016-03-15 15:13 UTC (permalink / raw) To: J. Bruce Fields; +Cc: steved, linux-nfs, neilb 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 <bcodding@redhat.com> > > --- > > 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-15 14:24 ` J. Bruce Fields 2016-03-15 15:13 ` Benjamin Coddington @ 2016-03-15 23:33 ` Malahal Naineni 2016-03-16 0:19 ` NeilBrown 2016-03-16 0:23 ` Benjamin Coddington 1 sibling, 2 replies; 11+ messages in thread From: Malahal Naineni @ 2016-03-15 23:33 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Benjamin Coddington, steved, linux-nfs, neilb Ben, if I understand, EnvironmentFile must be sourced after running ExecStartPre for this method to work, correct? I was planning on doing something like this for ganesha project but I couldn't find enough documentation that says this. Regards, Malahal. J. Bruce Fields [bfields@fieldses.org] wrote: > Also cc'ing Neil, as he added nfs-config originally.--b. > > 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 <bcodding@redhat.com> > > --- > > 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 > -- > 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-15 23:33 ` Malahal Naineni @ 2016-03-16 0:19 ` NeilBrown 2016-03-16 0:23 ` Benjamin Coddington 1 sibling, 0 replies; 11+ messages in thread From: NeilBrown @ 2016-03-16 0:19 UTC (permalink / raw) To: Malahal Naineni, J. Bruce Fields; +Cc: Benjamin Coddington, steved, linux-nfs [-- Attachment #1: Type: text/plain, Size: 356 bytes --] On Wed, Mar 16 2016, Malahal Naineni wrote: > Ben, if I understand, EnvironmentFile must be sourced after running > ExecStartPre for this method to work, correct? I was planning on doing > something like this for ganesha project but I couldn't find enough > documentation that says this. The only documentation I found was the source code :-) NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-15 23:33 ` Malahal Naineni 2016-03-16 0:19 ` NeilBrown @ 2016-03-16 0:23 ` Benjamin Coddington 1 sibling, 0 replies; 11+ messages in thread From: Benjamin Coddington @ 2016-03-16 0:23 UTC (permalink / raw) To: Malahal Naineni; +Cc: J. Bruce Fields, steved, linux-nfs, neilb On Tue, 15 Mar 2016, Malahal Naineni wrote: > Ben, if I understand, EnvironmentFile must be sourced after running > ExecStartPre for this method to work, correct? I was planning on doing > something like this for ganesha project but I couldn't find enough > documentation that says this. > > Regards, Malahal. Right, the EnvironmentFile would need to be sourced after running ExecStartPre. Regarding EnvironmentFile, systemd.service(5) states: The files listed with this directive will be read shortly before the process is executed (more specifically, after all processes from a previous unit state terminated. This means you can generate these files in one unit state, and read it with this option in the next). I suspect that the EnvironmentFile is sourced before every exec, including each ExecStartPre. It looks now like we'll perhaps not use this method after all, as Neil Brown has found that making the nfs-config.service into a RemainAfterExit=no, and Type=oneshot means it will get re-started before each service that has it listed in Wants=. That's nicer because it means we can change less, use less files, and systemd (I expect) handles the state transitions such that it isn't run concurrently. Ben > > J. Bruce Fields [bfields@fieldses.org] wrote: > > Also cc'ing Neil, as he added nfs-config originally.--b. > > > > 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 <bcodding@redhat.com> > > > --- > > > 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 > > -- > > 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 > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-12 12:05 [PATCH nfs-utils] systemd: remove the nfs-config service Benjamin Coddington 2016-03-15 14:24 ` J. Bruce Fields @ 2016-03-15 20:59 ` NeilBrown 2016-03-15 21:10 ` Benjamin Coddington 1 sibling, 1 reply; 11+ messages in thread From: NeilBrown @ 2016-03-15 20:59 UTC (permalink / raw) To: Benjamin Coddington, steved; +Cc: linux-nfs [-- Attachment #1: Type: text/plain, Size: 8410 bytes --] On Sat, Mar 12 2016, 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 <bcodding@redhat.com> I can't say I really like this, but it does make sense and solves a real problem. Maybe: I would never have written it myself, but I'm kind-a glad you did :-) Acked-by: NeilBrown <neilb@suse.com> Thanks, NeilBrown > --- > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-15 20:59 ` NeilBrown @ 2016-03-15 21:10 ` Benjamin Coddington 2016-03-15 21:52 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Benjamin Coddington @ 2016-03-15 21:10 UTC (permalink / raw) To: NeilBrown; +Cc: steved, linux-nfs On Wed, 16 Mar 2016, NeilBrown wrote: > On Sat, Mar 12 2016, 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 <bcodding@redhat.com> > > I can't say I really like this, but it does make sense and solves a real > problem. > Maybe: I would never have written it myself, but I'm kind-a glad you did > :-) Which bits are unlikeable? I am guessing that all these little intermediate files are the issue; that is what I hate. I admit that it is a least-effort approach to fixing the problem. What does the ideal fix look like to you? Ben > > Acked-by: NeilBrown <neilb@suse.com> > > Thanks, > NeilBrown > > > > --- > > 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-15 21:10 ` Benjamin Coddington @ 2016-03-15 21:52 ` NeilBrown 2016-03-16 0:16 ` Benjamin Coddington 0 siblings, 1 reply; 11+ messages in thread From: NeilBrown @ 2016-03-15 21:52 UTC (permalink / raw) To: Benjamin Coddington; +Cc: steved, linux-nfs [-- Attachment #1: Type: text/plain, Size: 2898 bytes --] On Wed, Mar 16 2016, Benjamin Coddington wrote: > On Wed, 16 Mar 2016, NeilBrown wrote: > >> On Sat, Mar 12 2016, 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 <bcodding@redhat.com> >> >> I can't say I really like this, but it does make sense and solves a real >> problem. >> Maybe: I would never have written it myself, but I'm kind-a glad you did >> :-) > > Which bits are unlikeable? I am guessing that all these little intermediate > files are the issue; that is what I hate. I admit that it is a least-effort > approach to fixing the problem. What does the ideal fix look like to you? I don't like having multiple intermediate config files that are all identical, and I don't like having to run the config script every time any daemon is (re)started. I'm not sure what "ideal" would be. One improvement would be if systemd could be given an executable in place of an environment file with the implication that it would run the executable and read environment variables from the stdout. That would remove the multiple config files. It would still mean the script is run every time, but I could be convinced that isn't so bad. An alternate idea is that maybe systemd could have a directive so that in any transaction were "this" unit is started, "that" unit will also be started. Then we could replace "Wants=nfs-config.server" with "Prepare=nfs-config.service" so if systemd ever needed to start any nfs-related service, it would make preparations by first running nfs-config.service - only once per transaction. But unless you want to hack on systemd, I think what you have is good enough. .... hmmm. I wonder what would happen if we just removed "RemainAfterExit=y" from nfs-config.service. Would it cause things to fail, or would it cause nfs-config.service to be run every time. Hey, that looks like it works! Can you double check for me? i.e. revert your patch, remove "RemainAfterExit=y" from nfs-config.service, and then see if nfs-config.service gets run any time any nfs server is started. Thanks, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-15 21:52 ` NeilBrown @ 2016-03-16 0:16 ` Benjamin Coddington 2016-03-16 1:35 ` NeilBrown 0 siblings, 1 reply; 11+ messages in thread From: Benjamin Coddington @ 2016-03-16 0:16 UTC (permalink / raw) To: NeilBrown; +Cc: steved, linux-nfs > On Wed, Mar 16 2016, Benjamin Coddington wrote: > > > On Wed, 16 Mar 2016, NeilBrown wrote: > > > >> On Sat, Mar 12 2016, 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 <bcodding@redhat.com> > >> > >> I can't say I really like this, but it does make sense and solves a real > >> problem. > >> Maybe: I would never have written it myself, but I'm kind-a glad you did > >> :-) > > > > Which bits are unlikeable? I am guessing that all these little intermediate > > files are the issue; that is what I hate. I admit that it is a least-effort > > approach to fixing the problem. What does the ideal fix look like to you? > > I don't like having multiple intermediate config files that are all > identical, and I don't like having to run the config script every time > any daemon is (re)started. > > I'm not sure what "ideal" would be. > > One improvement would be if systemd could be given an executable in > place of an environment file with the implication that it would run the > executable and read environment variables from the stdout. > That would remove the multiple config files. > It would still mean the script is run every time, but I could be > convinced that isn't so bad. > > An alternate idea is that maybe systemd could have a directive so that > in any transaction were "this" unit is started, "that" unit will also be > started. Then we could replace "Wants=nfs-config.server" with > "Prepare=nfs-config.service" so if systemd ever needed to start any > nfs-related service, it would make preparations by first running > nfs-config.service - only once per transaction. > > But unless you want to hack on systemd, I think what you have is good > enough. > > > .... hmmm. I wonder what would happen if we just removed > "RemainAfterExit=y" from nfs-config.service. Would it cause things to > fail, or would it cause nfs-config.service to be run every time. > > Hey, that looks like it works! Can you double check for me? > i.e. revert your patch, remove "RemainAfterExit=y" from > nfs-config.service, and then see if nfs-config.service gets run any time > any nfs server is started. Yep.. that appears to work just fine. Seems like a simpler solution than this change. It appears to me ( from systemd.service(5) ) that systemd's state transitions for Type=Oneshot will avoid races that might overwrite the environment file. Nice find! Ben ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH nfs-utils] systemd: remove the nfs-config service 2016-03-16 0:16 ` Benjamin Coddington @ 2016-03-16 1:35 ` NeilBrown 0 siblings, 0 replies; 11+ messages in thread From: NeilBrown @ 2016-03-16 1:35 UTC (permalink / raw) To: Benjamin Coddington; +Cc: steved, linux-nfs [-- Attachment #1: Type: text/plain, Size: 3523 bytes --] On Wed, Mar 16 2016, Benjamin Coddington wrote: >> On Wed, Mar 16 2016, Benjamin Coddington wrote: >> >> > On Wed, 16 Mar 2016, NeilBrown wrote: >> > >> >> On Sat, Mar 12 2016, 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 <bcodding@redhat.com> >> >> >> >> I can't say I really like this, but it does make sense and solves a real >> >> problem. >> >> Maybe: I would never have written it myself, but I'm kind-a glad you did >> >> :-) >> > >> > Which bits are unlikeable? I am guessing that all these little intermediate >> > files are the issue; that is what I hate. I admit that it is a least-effort >> > approach to fixing the problem. What does the ideal fix look like to you? >> >> I don't like having multiple intermediate config files that are all >> identical, and I don't like having to run the config script every time >> any daemon is (re)started. >> >> I'm not sure what "ideal" would be. >> >> One improvement would be if systemd could be given an executable in >> place of an environment file with the implication that it would run the >> executable and read environment variables from the stdout. >> That would remove the multiple config files. >> It would still mean the script is run every time, but I could be >> convinced that isn't so bad. >> >> An alternate idea is that maybe systemd could have a directive so that >> in any transaction were "this" unit is started, "that" unit will also be >> started. Then we could replace "Wants=nfs-config.server" with >> "Prepare=nfs-config.service" so if systemd ever needed to start any >> nfs-related service, it would make preparations by first running >> nfs-config.service - only once per transaction. >> >> But unless you want to hack on systemd, I think what you have is good >> enough. >> >> >> .... hmmm. I wonder what would happen if we just removed >> "RemainAfterExit=y" from nfs-config.service. Would it cause things to >> fail, or would it cause nfs-config.service to be run every time. >> >> Hey, that looks like it works! Can you double check for me? >> i.e. revert your patch, remove "RemainAfterExit=y" from >> nfs-config.service, and then see if nfs-config.service gets run any time >> any nfs server is started. > > Yep.. that appears to work just fine. Seems like a simpler solution than > this change. It appears to me ( from systemd.service(5) ) that systemd's > state transitions for Type=Oneshot will avoid races that might overwrite the > environment file. Nice find! > Great. Thanks for testing and thanks for prodding me to explain myself. Very happy with the outcome! I've posted a patch to Steve. NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 818 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-03-16 1:35 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-12 12:05 [PATCH nfs-utils] systemd: remove the nfs-config service Benjamin Coddington 2016-03-15 14:24 ` J. Bruce Fields 2016-03-15 15:13 ` Benjamin Coddington 2016-03-15 23:33 ` Malahal Naineni 2016-03-16 0:19 ` NeilBrown 2016-03-16 0:23 ` Benjamin Coddington 2016-03-15 20:59 ` NeilBrown 2016-03-15 21:10 ` Benjamin Coddington 2016-03-15 21:52 ` NeilBrown 2016-03-16 0:16 ` Benjamin Coddington 2016-03-16 1:35 ` NeilBrown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).