From: Olga Kornievskaia <aglo@umich.edu>
To: Jeff Layton <jlayton@kernel.org>
Cc: Olga Kornievskaia <okorniev@redhat.com>,
steved@redhat.com, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/1] nfsdctl: check for listeners before starting threads
Date: Mon, 23 Mar 2026 15:57:36 -0400 [thread overview]
Message-ID: <CAN-5tyEQe5KrqKNy_HtMfciyrKsxLm8Xu=ugUEBpH+gtf7OYGw@mail.gmail.com> (raw)
In-Reply-To: <24a15058d09d238ab627a6359ca9241e6aa499ce.camel@kernel.org>
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.
> Ack on the basic idea though!
>
> > {
> > - int i;
> > + int i, count = 0;
> > const char *res;
> >
> > for (i = 0; i < MAX_NFSD_SOCKETS; ++i) {
> > @@ -1098,26 +1120,34 @@ static void print_listeners(void)
> > res = inet_ntop(AF_INET, &((struct sockaddr_in *)(&sock->ss))->sin_addr,
> > addr, INET6_ADDRSTRLEN);
> > port = ((struct sockaddr_in *)(&sock->ss))->sin_port;
> > - if (res == NULL)
> > + if (res == NULL && output)
> > perror("inet_ntop");
> > - else
> > - printf("%s:%s:%hu\n", sock->name, addr, ntohs(port));
> > + else {
> > + count++;
> > + if (output)
> > + printf("%s:%s:%hu\n", sock->name, addr, ntohs(port));
> > + }
> > break;
> > case AF_INET6:
> > res = inet_ntop(AF_INET6, &((struct sockaddr_in6 *)(&sock->ss))->sin6_addr,
> > addr, INET6_ADDRSTRLEN);
> > port = ((struct sockaddr_in6 *)(&sock->ss))->sin6_port;
> > - if (res == NULL)
> > + if (res == NULL && output)
> > perror("inet_ntop");
> > - else
> > - printf("%s:[%s]:%hu\n", sock->name, addr, ntohs(port));
> > + else {
> > + count++;
> > + if (output)
> > + printf("%s:[%s]:%hu\n", sock->name, addr, ntohs(port));
> > + }
> > break;
> > default:
> > - snprintf(addr, INET6_ADDRSTRLEN, "Unknown address family: %d\n",
> > + if (output)
> > + snprintf(addr, INET6_ADDRSTRLEN, "Unknown address family: %d\n",
> > sock->ss.ss_family);
> > addr[INET6_ADDRSTRLEN - 1] = '\0';
> > }
> > }
> > + return count;
> > }
> >
> > static bool ipv6_is_enabled(void)
> > @@ -1394,7 +1424,7 @@ static int listener_func(struct nl_sock *sock, int argc, char ** argv)
> > return set_listeners(sock);
> > }
> >
> > - print_listeners();
> > + print_listeners(1);
> > return 0;
> > }
> >
>
> --
> Jeff Layton <jlayton@kernel.org>
>
next prev parent reply other threads:[~2026-03-23 19:57 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-23 18:21 [PATCH 1/1] nfsdctl: check for listeners before starting threads Olga Kornievskaia
2026-03-23 18:44 ` Jeff Layton
2026-03-23 19:57 ` Olga Kornievskaia [this message]
2026-03-23 20:08 ` Jeff Layton
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAN-5tyEQe5KrqKNy_HtMfciyrKsxLm8Xu=ugUEBpH+gtf7OYGw@mail.gmail.com' \
--to=aglo@umich.edu \
--cc=jlayton@kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=okorniev@redhat.com \
--cc=steved@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).