Re: [PATCH RFC v15 11/11] NFSD: Show state of courtesy clients in client info

[dai.ngo@oracle.com]

On 3/10/22 7:00 AM, Bruce Fields wrote:
> On Thu, Mar 10, 2022 at 03:27:58AM +0000, Chuck Lever III wrote:
>>> On Mar 9, 2022, at 10:09 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>>
>>> 
>>>> On 3/9/22 12:51 PM, dai.ngo@oracle.com wrote:
>>>>
>>>>> On 3/9/22 12:14 PM, Chuck Lever III wrote:
>>>>>
>>>>>> On Mar 4, 2022, at 7:37 PM, Dai Ngo <dai.ngo@oracle.com> wrote:
>>>>>>
>>>>>> Update client_info_show to show state of courtesy client and time
>>>>>> since last renew.
>>>>>>
>>>>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com> ---
>>>>>> fs/nfsd/nfs4state.c | 9 ++++++++- 1 file changed, 8
>>>>>> insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index
>>>>>> bced09014e6b..ed14e0b54537 100644 --- a/fs/nfsd/nfs4state.c +++
>>>>>> b/fs/nfsd/nfs4state.c @@ -2439,7 +2439,8 @@ static int
>>>>>> client_info_show(struct seq_file *m, void *v) { struct inode
>>>>>> *inode = m->private; struct nfs4_client *clp; -    u64 clid; +
>>>>>> u64 clid, hrs; +    u32 mins, secs;
>>>>>>
>>>>>>      clp = get_nfsdfs_clp(inode); if (!clp) @@ -2451,6 +2452,12 @@
>>>>>>      static int client_info_show(struct seq_file *m, void *v)
>>>>>>      seq_puts(m, "status: confirmed\n"); else seq_puts(m, "status:
>>>>>>      unconfirmed\n"); +    seq_printf(m, "courtesy client: %s\n",
>>>>>>      +        test_bit(NFSD4_CLIENT_COURTESY, &clp->cl_flags) ?
>>>>>>      "yes" : "no");
>>>>> I'm wondering if it would be more economical to combine this
>>>>> output with the status output just before it so we have only one
>>>>> of:
>>>>>
>>>>>      seq_puts(m, "status: unconfirmed\n");
>>>>>
>>>>>      seq_puts(m, "status: confirmed\n");
>>>>>
>>>>> or
>>>>>
>>>>>      seq_puts(m, "status: courtesy\n");
>>>> make sense, will fix.
>>> On second thought, I think it's safer to keep this the same since
>>> there might be scripts out there that depend on it.
>> I agree we should be sensitive to potential users of this information.
>> However…
>>
>> Without having one or two examples of such scripts in front of us,
>> it’s hard to say whether my suggestion (a new keyword after “status:”)
>> or your original (a new line in the file) would be more disruptive.
>>
>> Also I’m not seeing exactly how the output format is versioned… so
>> what’s the safest way to make changes to the output format of this
>> file? Anyone?
> It's not versioned.  It'd be good to document some rules; nfsd(7) seems
> like the logical place to put that, though probably knows about it.
> Pointers to it from kernel comments and elsewhere might help?
>
> I suppose the absolute safest option would be adding a new line, but I
> like the idea of adding the possibility of "courtesy" to the existing
> line as you suggest, and that seems very low risk.
>
> There is one utility, see nfs-utils/tools/nfsdclnt.  I'd forgotten about
> it untill I looked just now....

I think nfsdclnt does not parse the 'status' field so we're safe to
update the status to show 'confirmed/unconfirmed/courtesy'.

-Dai

>
> --b.