[PATCH] rpc-statd.service: Stop rpcbind and rpc.stat in an exit race

[PATCH] rpc-statd.service: Stop rpcbind and rpc.stat in an exit race

[Steve Dickson]

From: Benjamin Coddington <bcodding@redhat.com>

When `systemctl stop rpcbind.socket` is run, the dependency means
that systemd first sends SIGTERM to rpcbind, then sigterm to rpc.statd.

On SIGTERM, rpcbind tears down /var/run/rpcbind.sock.  However,
rpc-statd on SIGTERM attempts to unregister from rpcbind

systemd needs to wait for rpc.statd to exit before sending
SIGTERM to rpcbind

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2100395
Signed-off-by: Steve Dickson <steved@redhat.com>
---
 systemd/rpc-statd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
index 095629f2..392750da 100644
--- a/systemd/rpc-statd.service
+++ b/systemd/rpc-statd.service
@@ -5,7 +5,7 @@ Conflicts=umount.target
 Requires=nss-lookup.target rpcbind.socket
 Wants=network-online.target
 Wants=rpc-statd-notify.service
-After=network-online.target nss-lookup.target rpcbind.socket
+After=network-online.target nss-lookup.target rpcbind.service
 
 PartOf=nfs-utils.service
 IgnoreOnIsolate=yes
-- 
2.34.1

Re: [PATCH] rpc-statd.service: Stop rpcbind and rpc.stat in an exit race

[Benjamin Coddington]

On 27 Jul 2022, at 13:24, Steve Dickson wrote:

> From: Benjamin Coddington <bcodding@redhat.com>
>
> When `systemctl stop rpcbind.socket` is run, the dependency means
> that systemd first sends SIGTERM to rpcbind, then sigterm to rpc.statd.
>
> On SIGTERM, rpcbind tears down /var/run/rpcbind.sock.  However,
> rpc-statd on SIGTERM attempts to unregister from rpcbind
>
> systemd needs to wait for rpc.statd to exit before sending
> SIGTERM to rpcbind
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2100395
> Signed-off-by: Steve Dickson <steved@redhat.com>
> ---
>  systemd/rpc-statd.service | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
> index 095629f2..392750da 100644
> --- a/systemd/rpc-statd.service
> +++ b/systemd/rpc-statd.service
> @@ -5,7 +5,7 @@ Conflicts=umount.target
>  Requires=nss-lookup.target rpcbind.socket
>  Wants=network-online.target
>  Wants=rpc-statd-notify.service
> -After=network-online.target nss-lookup.target rpcbind.socket
> +After=network-online.target nss-lookup.target rpcbind.service
>
>  PartOf=nfs-utils.service
>  IgnoreOnIsolate=yes
> -- 
> 2.34.1

Hey Steve - thanks for patchifying this -  I should have sent a proper
patch, but the reason I didn't is that I didn't understand why we have the
rpcbind.socket unit, and why that unit isn't sufficient to enforce the
ordering.

I don't remember the history.  Will changing the After= here create a
problem where rpcbind.service is up, but the socket isn't there yet, and
then rpc.statd comes up and can't find the socket?

Ben

Re: [PATCH] rpc-statd.service: Stop rpcbind and rpc.stat in an exit race

[Steve Dickson]

Hey!

On 7/28/22 7:15 AM, Benjamin Coddington wrote:
> On 27 Jul 2022, at 13:24, Steve Dickson wrote:
> 
>> From: Benjamin Coddington <bcodding@redhat.com>
>>
>> When `systemctl stop rpcbind.socket` is run, the dependency means
>> that systemd first sends SIGTERM to rpcbind, then sigterm to rpc.statd.
>>
>> On SIGTERM, rpcbind tears down /var/run/rpcbind.sock.  However,
>> rpc-statd on SIGTERM attempts to unregister from rpcbind
>>
>> systemd needs to wait for rpc.statd to exit before sending
>> SIGTERM to rpcbind
>>
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2100395
>> Signed-off-by: Steve Dickson <steved@redhat.com>
>> ---
>>   systemd/rpc-statd.service | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
>> index 095629f2..392750da 100644
>> --- a/systemd/rpc-statd.service
>> +++ b/systemd/rpc-statd.service
>> @@ -5,7 +5,7 @@ Conflicts=umount.target
>>   Requires=nss-lookup.target rpcbind.socket
>>   Wants=network-online.target
>>   Wants=rpc-statd-notify.service
>> -After=network-online.target nss-lookup.target rpcbind.socket
>> +After=network-online.target nss-lookup.target rpcbind.service
>>
>>   PartOf=nfs-utils.service
>>   IgnoreOnIsolate=yes
>> -- 
>> 2.34.1
> 
> Hey Steve - thanks for patchifying this -  I should have sent a proper
> patch, but the reason I didn't is that I didn't understand why we have the
> rpcbind.socket unit, and why that unit isn't sufficient to enforce the
> ordering.
The whole idea be the rpcbind.socket unit was the Fedora or systemd
people did not want daemons running when the are not needed.
So they came up with "on-demand" daemons, meaning a daemon
would be started when a process connect to the socket.

> 
> I don't remember the history.  Will changing the After= here create a
> problem where rpcbind.service is up, but the socket isn't there yet, and
> then rpc.statd comes up and can't find the socket?
I don't think so... That After= line, in rpc-statd.service,
was a result of a bug fix I did for bz1419351 (commit 09e5c6c2a3).
Why I used rpcbind.socket instead of rpcbind.service is not
clear, since rpcbind.service Requires rpcbind.socket.

So I'm thinking this is a valid bug fix... Thanks
for doing the debugging!!!

steved.

Re: [PATCH] rpc-statd.service: Stop rpcbind and rpc.stat in an exit race

[Steve Dickson]

On 7/27/22 1:24 PM, Steve Dickson wrote:
> Subject:
> [PATCH] rpc-statd.service: Stop rpcbind and rpc.stat in an exit race
> From:
> Steve Dickson <steved@redhat.com>
> Date:
> 7/27/22, 1:24 PM
> 
> To:
> Linux NFS Mailing list <linux-nfs@vger.kernel.org>
> 
> 
> From: Benjamin Coddington<bcodding@redhat.com>
> 
> When `systemctl stop rpcbind.socket` is run, the dependency means
> that systemd first sends SIGTERM to rpcbind, then sigterm to rpc.statd.
> 
> On SIGTERM, rpcbind tears down /var/run/rpcbind.sock.  However,
> rpc-statd on SIGTERM attempts to unregister from rpcbind
> 
> systemd needs to wait for rpc.statd to exit before sending
> SIGTERM to rpcbind
> 
> Fixes:https://bugzilla.redhat.com/show_bug.cgi?id=2100395
> Signed-off-by: Steve Dickson<steved@redhat.com>
Committed...

steved.
> ---
>   systemd/rpc-statd.service | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
> index 095629f2..392750da 100644
> --- a/systemd/rpc-statd.service
> +++ b/systemd/rpc-statd.service
> @@ -5,7 +5,7 @@ Conflicts=umount.target
>   Requires=nss-lookup.target rpcbind.socket
>   Wants=network-online.target
>   Wants=rpc-statd-notify.service
> -After=network-online.target nss-lookup.target rpcbind.socket
> +After=network-online.target nss-lookup.target rpcbind.service
>   
>   PartOf=nfs-utils.service
>   IgnoreOnIsolate=yes
> -- 2.34.1
>