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

[Bruce Fields <bfields@fieldses.org>]

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....

--b.