Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e7f8ce36-93bc-50f5-1abf-5db5ec2e193e@redhat.com>
Date:   Thu, 28 Jul 2022 09:09:44 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] rpc-statd.service: Stop rpcbind and rpc.stat in an exit
 race
Content-Language: en-US
To:     Benjamin Coddington <bcodding@redhat.com>
Cc:     Linux NFS Mailing list <linux-nfs@vger.kernel.org>
References: <20220727172441.6524-1-steved@redhat.com>
 <E4C11B71-CBAC-4F1B-B4FF-270FD0EDADA9@redhat.com>
From:   Steve Dickson <steved@redhat.com>
In-Reply-To: <E4C11B71-CBAC-4F1B-B4FF-270FD0EDADA9@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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.