Re: [nfs-utils PATCH] nfsdctl: debug logging fixups

Jeff Layton
2025-01-15T18:33:41Z

On Wed, 2025-01-15 at 12:47 -0500, Steve Dickson wrote:
>
> On 1/15/25 12:35 PM, Jeff Layton wrote:
> > On Wed, 2025-01-15 at 12:32 -0500, Steve Dickson wrote:
> > >
> > > On 1/15/25 12:00 PM, Scott Mayhew wrote:
> > > > Move read_nfsd_conf() out of autostart_func() and into main(). Remove
> > > > hard-coded NFSD_FAMILY_NAME in the first error message in
> > > > netlink_msg_alloc() and make the error messages in netlink_msg_alloc()
> > > > more descriptive/unique.
> > > >
> > > > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > > > ---
> > > > SteveD - this would go on top of Jeff's "nfsdctl: add support for new
> > > > lockd configuration interface" patches.
> > > Got it...
> > >
> > > >
> > > > utils/nfsdctl/nfsdctl.c | 8 ++++----
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
> > > > index 003daba5..f81c78ae 100644
> > > > --- a/utils/nfsdctl/nfsdctl.c
> > > > +++ b/utils/nfsdctl/nfsdctl.c
> > > > @@ -436,7 +436,7 @@ static struct nl_msg *netlink_msg_alloc(struct nl_sock *sock, const char *family
> > > >
> > > > id = genl_ctrl_resolve(sock, family);
> > > > if (id < 0) {
> > > > - xlog(L_ERROR, "%s not found", NFSD_FAMILY_NAME);
> > > > + xlog(L_ERROR, "failed to resolve %s generic netlink family", family);
> > > > return NULL;
> > > > }
> > > >
> > > > @@ -447,7 +447,7 @@ static struct nl_msg *netlink_msg_alloc(struct nl_sock *sock, const char *family
> > > > }
> > > >
> > > > if (!genlmsg_put(msg, 0, 0, id, 0, 0, 0, 0)) {
> > > > - xlog(L_ERROR, "failed to allocate netlink message");
> > > > + xlog(L_ERROR, "failed to add generic netlink headers to netlink message");
> > > > nlmsg_free(msg);
> > > > return NULL;
> > > > }
> > > > @@ -1509,8 +1509,6 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
> > > > }
> > > > }
> > > >
> > > > - read_nfsd_conf();
> > > > -
> > > > grace = conf_get_num("nfsd", "grace-time", 0);
> > > > ret = lockd_configure(sock, grace);
> > > > if (ret) {
> > > > @@ -1728,6 +1726,8 @@ int main(int argc, char **argv)
> > > > xlog_syslog(0);
> > > > xlog_stderr(1);
> > > >
> > > > + read_nfsd_conf();
> > > > +
> > > > /* Parse the preliminary options */
> > > > while ((opt = getopt_long(argc, argv, "+hdsV", pre_options, NULL)) != -1) {
> > > > switch (opt) {
> > > Ok... at this point we a prettier error message
> > > $ nfsdctl nlm
> > > nfsdctl: failed to resolve lockd generic netlink family
> > >
> > > But the point of this argument is:
> > >
> > > Get information about NLM (lockd) settings in the current net
> > > namespace. This subcommand takes no arguments.
> > >
> > > How is that giving information from the running lockd?
> > >
> > > What am I missing??
> > >
> >
> > You're missing a kernel that has the required netlink interface. To
> > test this properly, you'll need to patch your kernel, until that patch
> > makes it upstream.
> Okay... I figured it was something like that. But doesn't make sense to
> wait until the patch is in upstream so the argument can be properly
> tested? Why add an argument that will always fail?
>

Why can't it be properly tested? It's just a matter of running a more
recent kernel that has the right interfaces. That should be in linux-
next soon (if not already).

I think the question is whether we want to wait until the kernel
interfaces trickle out into downstream distro kernels before we ship
any userland support in an upstream project (nfs-utils).

If you want to wait until it hits Fedora Rawhide kernels, then you're
looking at about 10-12 weeks from now. If you want to wait until it
makes it into a stable Fedora release kernel then we're looking at
about 6 months from now.

I'll note that that it took 6 months to get the original nfsdctl
patches merged because of the lag on kernel patches making it into
distros, and I think that was way too long.
--
Jeff Layton <jlayton@kernel.org>