[PATCH nfs-utils] systemd unit files: fix up dependencies on rpcbind.

[PATCH nfs-utils] systemd unit files: fix up dependencies on rpcbind.

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 4193 bytes --]


The dependencies on rpcbind have been changed a few times and I think
they are still wrong.  So I'll go into some detail to justify this change.

Firstly: rpcbind.target rpcbind.socket or rpcbind.service?

The systemd documentation talks about targets as "synchronization
points" and likens them to SysV init run levels.  Run levels are about
ordering but not dependencies.

The systemd.special man page describes rpcbind.target as intended
explicitly for ordering sysvinit scripts, with "After=" dependencies.

So while I think it is valid to use rpcbind.target for ordering
(before/after) it shouldn't be used for dependencies (Wants/Requires).
The rpcbind.target file included in systemd does not "Require" the
actual service, so requiring rpcbind.target itself is pointless.

I think we shouldn't use rpcbind.target at all.  Leave it for sysvinit
synchronization.

So: .socket or .service?

I think nfs only needs the socket to be active.  On first connection
the service will be started.  But nfs does not need to wait for the
service to start, only the socket.  So I think we should exclusively
use rpcbind.socket.

Next: Wants or Requires.

rpc.statd definitely Requires rpcbind.  It needs to register to be
useful, and without rpcbind it cannot register.

nfs-server does not necesarily require rpcbind.  Specifically if
configured for NFSv4 only, nfs-server will work quite happily without
rpcbind.

Someone with an NFSv4 only setup who wants rpcbind to not run can use
  systemctl mask rpcbind.socket
to ensure it never runs.
So nfs-server should only "Wants: rpcbind.socket".
I think
 Commit: 4fabfcd08206 ("systemd: Decouple the starting and stopping of rpcbind/nfs-server")
should have changed "Requires" to "Wants" rather than "server" to "target"
to fix the dependency problem.

Finally: After?

It only makes sense to declare an ordering relation as "After:"
something that will actually be started.  If "foo.service" is not part
of the systemd transaction, then "After: foo.service" has no effect.
So having:
  Requires: rpcbind.target
  After: rpcbind.socket

doesn't make much sense unless there is some relationship between
rpcbind.target and rpcbind.socket, and there is no general guarantee
of that (though what individual distros do, I don't know).
So the "After" should match the "Wants" or "Requires".

It might make sense to
  Requires: rpcbind.socket
  After: rpcbind.target

as it is reasonable to assume that rpcbind.target will be ordered with
rpcbind.socket, but as we can use rpcbind.socket explictly, that is
clearer.

So my conclusion is that nfs-server should:
  Wants: rpcbind.socket
  After: rpcbind.socket

and rpc-statd should
  Requires: rpcbind.socket
  After: rpcbind.socket

which is what this patch puts into effect.

Signed-off-by: NeilBrown <neilb@suse.com>

diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
index 317e5d689767..2ccdc6344cd5 100644
--- a/systemd/nfs-server.service
+++ b/systemd/nfs-server.service
@@ -1,13 +1,14 @@
 [Unit]
 Description=NFS server and services
 DefaultDependencies=no
-Requires= network.target proc-fs-nfsd.mount rpcbind.target
+Requires= network.target proc-fs-nfsd.mount
 Requires= nfs-mountd.service
+Wants=rpcbind.socket
 Wants=rpc-statd.service nfs-idmapd.service
 Wants=rpc-statd-notify.service
 
 After= local-fs.target
-After= network.target proc-fs-nfsd.mount rpcbind.service nfs-mountd.service
+After= network.target proc-fs-nfsd.mount rpcbind.socket nfs-mountd.service
 After= nfs-idmapd.service rpc-statd.service
 Before= rpc-statd-notify.service
 
diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
index f16ea425dc77..a02f5c41a424 100644
--- a/systemd/rpc-statd.service
+++ b/systemd/rpc-statd.service
@@ -2,8 +2,8 @@
 Description=NFS status monitor for NFSv2/3 locking.
 DefaultDependencies=no
 Conflicts=umount.target
-Requires=nss-lookup.target rpcbind.target
-After=network.target nss-lookup.target rpcbind.service
+Requires=nss-lookup.target rpcbind.socket
+After=network.target nss-lookup.target rpcbind.socket
 
 PartOf=nfs-utils.service
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

Re: [PATCH nfs-utils] systemd unit files: fix up dependencies on rpcbind.

[Steve Dickson]

On 28/04/16 22:59, NeilBrown wrote:
> 
> The dependencies on rpcbind have been changed a few times and I think
> they are still wrong.  So I'll go into some detail to justify this change.
> 
> Firstly: rpcbind.target rpcbind.socket or rpcbind.service?
> 
> The systemd documentation talks about targets as "synchronization
> points" and likens them to SysV init run levels.  Run levels are about
> ordering but not dependencies.
> 
> The systemd.special man page describes rpcbind.target as intended
> explicitly for ordering sysvinit scripts, with "After=" dependencies.
> 
> So while I think it is valid to use rpcbind.target for ordering
> (before/after) it shouldn't be used for dependencies (Wants/Requires).
> The rpcbind.target file included in systemd does not "Require" the
> actual service, so requiring rpcbind.target itself is pointless.
> 
> I think we shouldn't use rpcbind.target at all.  Leave it for sysvinit
> synchronization.
> 
> So: .socket or .service?
> 
> I think nfs only needs the socket to be active.  On first connection
> the service will be started.  But nfs does not need to wait for the
> service to start, only the socket.  So I think we should exclusively
> use rpcbind.socket.
> 
> Next: Wants or Requires.
> 
> rpc.statd definitely Requires rpcbind.  It needs to register to be
> useful, and without rpcbind it cannot register.
> 
> nfs-server does not necesarily require rpcbind.  Specifically if
> configured for NFSv4 only, nfs-server will work quite happily without
> rpcbind.
> 
> Someone with an NFSv4 only setup who wants rpcbind to not run can use
>   systemctl mask rpcbind.socket
> to ensure it never runs.
> So nfs-server should only "Wants: rpcbind.socket".
> I think
>  Commit: 4fabfcd08206 ("systemd: Decouple the starting and stopping of rpcbind/nfs-server")
> should have changed "Requires" to "Wants" rather than "server" to "target"
> to fix the dependency problem.
> 
> Finally: After?
> 
> It only makes sense to declare an ordering relation as "After:"
> something that will actually be started.  If "foo.service" is not part
> of the systemd transaction, then "After: foo.service" has no effect.
> So having:
>   Requires: rpcbind.target
>   After: rpcbind.socket
> 
> doesn't make much sense unless there is some relationship between
> rpcbind.target and rpcbind.socket, and there is no general guarantee
> of that (though what individual distros do, I don't know).
> So the "After" should match the "Wants" or "Requires".
> 
> It might make sense to
>   Requires: rpcbind.socket
>   After: rpcbind.target
> 
> as it is reasonable to assume that rpcbind.target will be ordered with
> rpcbind.socket, but as we can use rpcbind.socket explictly, that is
> clearer.
> 
> So my conclusion is that nfs-server should:
>   Wants: rpcbind.socket
>   After: rpcbind.socket
> 
> and rpc-statd should
>   Requires: rpcbind.socket
>   After: rpcbind.socket
> 
> which is what this patch puts into effect.
Thanks for making sense of this.... Committed... 

steved.

> 
> Signed-off-by: NeilBrown <neilb@suse.com>
> 
> diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
> index 317e5d689767..2ccdc6344cd5 100644
> --- a/systemd/nfs-server.service
> +++ b/systemd/nfs-server.service
> @@ -1,13 +1,14 @@
>  [Unit]
>  Description=NFS server and services
>  DefaultDependencies=no
> -Requires= network.target proc-fs-nfsd.mount rpcbind.target
> +Requires= network.target proc-fs-nfsd.mount
>  Requires= nfs-mountd.service
> +Wants=rpcbind.socket
>  Wants=rpc-statd.service nfs-idmapd.service
>  Wants=rpc-statd-notify.service
>  
>  After= local-fs.target
> -After= network.target proc-fs-nfsd.mount rpcbind.service nfs-mountd.service
> +After= network.target proc-fs-nfsd.mount rpcbind.socket nfs-mountd.service
>  After= nfs-idmapd.service rpc-statd.service
>  Before= rpc-statd-notify.service
>  
> diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
> index f16ea425dc77..a02f5c41a424 100644
> --- a/systemd/rpc-statd.service
> +++ b/systemd/rpc-statd.service
> @@ -2,8 +2,8 @@
>  Description=NFS status monitor for NFSv2/3 locking.
>  DefaultDependencies=no
>  Conflicts=umount.target
> -Requires=nss-lookup.target rpcbind.target
> -After=network.target nss-lookup.target rpcbind.service
> +Requires=nss-lookup.target rpcbind.socket
> +After=network.target nss-lookup.target rpcbind.socket
>  
>  PartOf=nfs-utils.service
>  
>