Re: [PATCH 1/1] nfsdctl: check for listeners before starting threads

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2026-03-23 at 15:57 -0400, Olga Kornievskaia wrote:
> On Mon, Mar 23, 2026 at 2:52 PM Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > On Mon, 2026-03-23 at 14:21 -0400, Olga Kornievskaia wrote:
> > > When a thread command is executed and yet no listeners have been
> > > added prior to it, instead of failing with EIO error print an
> > > informative error. Also, "thread 0" command should not error
> > > regardless of the listener status.
> > > 
> > > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > > ---
> > >  utils/nfsdctl/nfsdctl.c | 52 ++++++++++++++++++++++++++++++++---------
> > >  1 file changed, 41 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
> > > index 6a20d180..d03e2c9f 100644
> > > --- a/utils/nfsdctl/nfsdctl.c
> > > +++ b/utils/nfsdctl/nfsdctl.c
> > > @@ -162,6 +162,9 @@ static const char *nfsd4_ops[] = {
> > >       [OP_REMOVEXATTR]        = "OP_REMOVEXATTR",
> > >  };
> > > 
> > > +static int fetch_current_listeners(struct nl_sock *sock);
> > > +static int print_listeners(int output);
> > > +
> > >  static int error_handler(struct sockaddr_nl *nla, struct nlmsgerr *err,
> > >                        void *arg)
> > >  {
> > > @@ -720,7 +723,7 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
> > >       uint8_t cmd = NFSD_CMD_THREADS_GET;
> > >       int *pool_threads = NULL;
> > >       int minthreads = -1;
> > > -     int opt, pools = 0;
> > > +     int opt, pools = 0, zero_threads = 0;
> > > 
> > >       optind = 1;
> > >       while ((opt = getopt_long(argc, argv, "hm:", threads_options, NULL)) != -1) {
> > > @@ -762,12 +765,31 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
> > >                       }
> > > 
> > >                       pool_threads[i] = strtol(targv[i], &endptr, 0);
> > > +                     if (!pool_threads[i])
> > > +                             zero_threads++;
> > >                       if (!endptr || *endptr != '\0') {
> > >                               xlog(L_ERROR, "Invalid threads value %s.", argv[1]);
> > >                               return 1;
> > >                       }
> > >               }
> > >       }
> > > +     /* check if there are listeners added */
> > > +     if (fetch_current_listeners(sock)) {
> > > +             xlog(L_ERROR, "Unable to determine if listeners were added\n");
> > > +             return 1;
> > > +     }
> > > +     if (!print_listeners(0)) {
> > > +             if (zero_threads && zero_threads == pools) {
> > > +                     /* Note: we can never have a server with threads and no
> > > +                      * listeners. If we ever add functionality to remove
> > > +                      * listeners on an active server, we need to revisit this.
> > > +                      */
> > > +                     return 0;
> > > +             }
> > > +             xlog(L_ERROR, "No listeners added, not starting threads\n");
> > > +             return 1;
> > > +     }
> > > +
> > >       return threads_doit(sock, cmd, 0, 0, pools, pool_threads, NULL, minthreads);
> > >  }
> > > 
> > > @@ -1077,9 +1099,9 @@ out:
> > >       return ret;
> > >  }
> > > 
> > > -static void print_listeners(void)
> > > +static int print_listeners(int output)
> > 
> > Maybe instead of adding the "output" parameter, you could just add a
> > count_listeners() function?
> 
> Well, I thought would need to do all the work of print_listener(), I
> would assume we'd want to do all the checks done by print_listener
> before it prints the value or in my case count it as a listener. And
> thus I opted for re-using the function but adding the count.
> 
> If you think it's sufficient to just check if sock->name is non-empty
> and active and do no further checking, I can.
> 

The "active" flag is only used on the set side of things. Anything read
by "get" is considered active. If you fetch_current_listeners() first,
then I think you can just check that nfsd_socket_count is non-zero.
-- 
Jeff Layton <jlayton@kernel.org>