Re: [PATCH v2 0/3] nfsdctl: add support for new lockd configuration interface

[Scott Mayhew <smayhew@redhat.com>]

On Wed, 15 Jan 2025, Jeff Layton wrote:

> On Wed, 2025-01-15 at 10:12 -0500, Steve Dickson wrote:
> > 
> > On 1/15/25 9:56 AM, Jeff Layton wrote:
> > > On Wed, 2025-01-15 at 09:44 -0500, Scott Mayhew wrote:
> > > > On Tue, 14 Jan 2025, Jeff Layton wrote:
> > > > 
> > > > > On Tue, 2025-01-14 at 16:09 -0500, Scott Mayhew wrote:
> > > > > > On Fri, 10 Jan 2025, Jeff Layton wrote:
> > > > > > 
> > > > > > > v2 is just a small update to fix the problems that Scott spotted.
> > > > > > > 
> > > > > > > This patch series adds support for the new lockd configuration interface
> > > > > > > that should fix this RH bug:
> > > > > > > 
> > > > > > >      https://issues.redhat.com/browse/RHEL-71698
> > > > > > > 
> > > > > > > There are some other improvements here too, notably a switch to xlog.
> > > > > > > Only lightly tested, but seems to do the right thing.
> > > > > > > 
> > > > > > > Port handling with lockd still needs more work. Currently that is
> > > > > > > usually configured by rpc.statd. I think we need to convert it to
> > > > > > > use netlink to configure the ports as well, when it's able.
> > > > > > > 
> > > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > > 
> > > > > > I think the read_nfsd_conf call should be moved out of autostart_func
> > > > > > and into main (right before the command-line options are parsed).  Right
> > > > > > now if you enable debugging in nfs.conf, you get the "configuring
> > > > > > listeners" and "nfsdctl exiting" messages, but not the "nfsdctl started"
> > > > > > message.  It's not a big deal though and could be done if additional
> > > > > > debug logging is added in the future.
> > > > > > 
> > > > > 
> > > > > That sounds good. We can do that in a separate patch.
> > > > > 
> > > > > > Reviewed-by: Scott Mayhew <smayhew@redhat.com>
> > > > > 
> > > > > Thanks!
> > > > 
> > > > Hey, Jeff.  I was testing this against a kernel without the lockd
> > > > netlink patch, and I get this:
> > > > 
> > > > Jan 15 09:39:16 systemd[1]: Starting nfs-server.service - NFS server and services...
> > > > Jan 15 09:39:17 sh[1603]: nfsdctl: nfsdctl started
> > > > Jan 15 09:39:17 sh[1603]: nfsdctl: nfsd not found
> > > > Jan 15 09:39:17 sh[1603]: nfsdctl: lockd configuration failure
> > > > Jan 15 09:39:17 sh[1603]: nfsdctl: nfsdctl exiting
> > > > Jan 15 09:39:17 sh[1601]: rpc.nfsd: knfsd is currently down
> > > > Jan 15 09:39:17 sh[1601]: rpc.nfsd: Writing version string to kernel: -2 +3 +4 +4.1 +4.2
> > > > Jan 15 09:39:17 sh[1601]: rpc.nfsd: Created AF_INET TCP socket.
> > > > Jan 15 09:39:17 sh[1601]: rpc.nfsd: Created AF_INET6 TCP socket.
> > > > 
> > > > Do we really want it falling back to rpc.nfsd if it can't configure
> > > > lockd?  Maybe it should emit a warning instead?
> > > > 
> > > 
> > > I thought about that, and I think it's better to error out here.
> > > 
> > > Falling back to rpc.nfsd is harmless, and only people who are trying to
> > > set the grace period or lockd ports will ever hit this. lockd
> > > configuration is a no-op if none of those settings are set.
> > > 
> > > > At the very least, NFSD_FAMILY_NAME should no longer be hard-coded in
> > > > that "not found" error message in netlink_msg_alloc().
> > > > 
> > > 
> > > Yeah, that would be good to fix.
> > > 
> > 
> > On a rawhide kernel (6.13.0-0.rc6) the server does
> > come up with 'nfsdctl autostart' but with the
> > new argument 'nlm' I'm getting
> > 
> > $ nfsdctl nlm
> > nfsdctl: nfsd not found
> > 
> 
> Yeah, that's what Scott pointed out too. We should make that error
> message a bit more friendly. It may be a bit before I can get to it. Do
> you guys want to propose a patch to fix that?

Sure, I can do that.
> 
> Thanks,
> -- 
> Jeff Layton <jlayton@kernel.org>
>