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

Jeff Layton
2025-01-15T14:56:46Z

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.

>
> >
> > > > ---
> > > > Changes in v2:
> > > > - properly regenerate manpages
> > > > - fix up bogus merge conflict
> > > > - add D_GENERAL xlog messages when nfsdctl starts and exits
> > > > - Link to v1: https://lore.kernel.org/r/20250109-lockd-nl-v1-0-108548ab0b6b@kernel.org
> > > >
> > > > ---
> > > > Jeff Layton (3):
> > > > nfsdctl: convert to xlog()
> > > > nfsdctl: fix the --version option
> > > > nfsdctl: add necessary bits to configure lockd
> > > >
> > > > configure.ac | 4 +
> > > > utils/nfsdctl/lockd_netlink.h | 29 ++++
> > > > utils/nfsdctl/nfsdctl.8 | 15 +-
> > > > utils/nfsdctl/nfsdctl.adoc | 8 +
> > > > utils/nfsdctl/nfsdctl.c | 331 ++++++++++++++++++++++++++++++++++--------
> > > > 5 files changed, 321 insertions(+), 66 deletions(-)
> > > > ---
> > > > base-commit: 65f4cc3a6ce1472ee4092c4bbf4b19beb0a8217b
> > > > change-id: 20250109-lockd-nl-6272fa9e8a5d
> > > >
> > > > Best regards,
> > > > --
> > > > Jeff Layton <jlayton@kernel.org>
> > > >
> > >
> >
> > --
> > Jeff Layton <jlayton@kernel.org>
> >
>

--
Jeff Layton <jlayton@kernel.org>