On Thu, 2025-01-09 at 15:29 -0500, Scott Mayhew wrote:
> On Thu, 09 Jan 2025, Jeff Layton wrote:
>
> > Convert all of the fprintf(stderr, ...) calls to xlog(L_ERROR, ...)
> > calls. Change the -d option to not take an argument, and add a -s
> > option that will make nfsdctl log to syslog instead of stderr.
>
> I think there should be few xlog(D_GENERAL) calls, otherwise we'll wind
> up getting bug reports from users saying they turned on debug logging
> and they're not getting any output. I was working on a similar patch,
> with before each of the nl_send_auto() calls.
>
> xlog(D_GENERAL, "%s: sending netlink message", __func__);
>
> Even a single "nfsdctl started" log message would be better than
> nothing.
>
Good point. The autostart one, in particular probably needs something.
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > utils/nfsdctl/nfsdctl.adoc | 3 ++
> > utils/nfsdctl/nfsdctl.c | 103 ++++++++++++++++++++++++---------------------
> > 2 files changed, 59 insertions(+), 47 deletions(-)
> >
> > diff --git a/utils/nfsdctl/nfsdctl.adoc b/utils/nfsdctl/nfsdctl.adoc
> > index c5921458a8e81e749d264cc7dd8344889ec71ac5..2114693042594297b7c5d8600ca16813a0f2356c 100644
> > --- a/utils/nfsdctl/nfsdctl.adoc
> > +++ b/utils/nfsdctl/nfsdctl.adoc
> > @@ -30,6 +30,9 @@ To get information about different subcommand usage, pass the subcommand the
> > *-h, --help*::
> > Print program help text
> >
> > +*-s, --syslog*::
> > + Log to syslog instead of to stderr (the default)
> > +
> > *-V, --version*::
> > Print program version
> >
>
> You updated the adoc file but didn't regenerate the man page.
>
Yeah, forgot to git add it. I'll include it in the next version.
> > diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
> > index b138f06d8b5504e4bf0362041ba36a68aeb12508..11fa4363907a0897ddf79f21916f9e25b9e88dbb 100644
> > --- a/utils/nfsdctl/nfsdctl.c
> > +++ b/utils/nfsdctl/nfsdctl.c
> > @@ -45,8 +45,6 @@
> > * gcc -I/usr/include/libnl3/ -o <prog-name> <prog-name>.c -lnl-3 -lnl-genl-3
> > */
> >
> > -static int debug_level;
> > -
> > struct nfs_version {
> > uint8_t major;
> > uint8_t minor;
> > @@ -390,7 +388,7 @@ static struct nl_sock *netlink_sock_alloc(void)
> > return NULL;
> >
> > if (genl_connect(sock)) {
> > - fprintf(stderr, "Failed to connect to generic netlink\n");
> > + xlog(L_ERROR, "Failed to connect to generic netlink");
> > nl_socket_free(sock);
> > return NULL;
> > }
> > @@ -409,18 +407,18 @@ static struct nl_msg *netlink_msg_alloc(struct nl_sock *sock)
> >
> > id = genl_ctrl_resolve(sock, NFSD_FAMILY_NAME);
> > if (id < 0) {
> > - fprintf(stderr, "%s not found\n", NFSD_FAMILY_NAME);
> > + xlog(L_ERROR, "%s not found", NFSD_FAMILY_NAME);
> > return NULL;
> > }
> >
> > msg = nlmsg_alloc();
> > if (!msg) {
> > - fprintf(stderr, "failed to allocate netlink message\n");
> > + xlog(L_ERROR, "failed to allocate netlink message");
> > return NULL;
> > }
> >
> > if (!genlmsg_put(msg, 0, 0, id, 0, 0, 0, 0)) {
> > - fprintf(stderr, "failed to allocate netlink message\n");
> > + xlog(L_ERROR, "failed to allocate netlink message");
> > nlmsg_free(msg);
> > return NULL;
> > }
> > @@ -462,7 +460,7 @@ static int status_func(struct nl_sock *sock, int argc, char ** argv)
> >
> > cb = nl_cb_alloc(NL_CB_CUSTOM);
> > if (!cb) {
> > - fprintf(stderr, "failed to allocate netlink callbacks\n");
> > + xlog(L_ERROR, "failed to allocate netlink callbacks");
> > ret = 1;
> > goto out;
> > }
> > @@ -480,7 +478,7 @@ static int status_func(struct nl_sock *sock, int argc, char ** argv)
> > while (ret > 0)
> > nl_recvmsgs(sock, cb);
> > if (ret < 0) {
> > - fprintf(stderr, "Error: %s\n", strerror(-ret));
> > + xlog(L_ERROR, "Error: %s", strerror(-ret));
> > ret = 1;
> > }
> > out_cb:
> > @@ -521,14 +519,14 @@ static int threads_doit(struct nl_sock *sock, int cmd, int grace, int lease,
> >
> > cb = nl_cb_alloc(NL_CB_CUSTOM);
> > if (!cb) {
> > - fprintf(stderr, "failed to allocate netlink callbacks\n");
> > + xlog(L_ERROR, "failed to allocate netlink callbacks");
> > ret = 1;
> > goto out;
> > }
> >
> > ret = nl_send_auto(sock, msg);
> > if (ret < 0) {
> > - fprintf(stderr, "send failed (%d)!\n", ret);
> > + xlog(L_ERROR, "send failed (%d)!", ret);
> > goto out_cb;
> > }
> >
> > @@ -541,7 +539,7 @@ static int threads_doit(struct nl_sock *sock, int cmd, int grace, int lease,
> > while (ret > 0)
> > nl_recvmsgs(sock, cb);
> > if (ret < 0) {
> > - fprintf(stderr, "Error: %s\n", strerror(-ret));
> > + xlog(L_ERROR, "Error: %s", strerror(-ret));
> > ret = 1;
> > }
> > out_cb:
> > @@ -586,13 +584,13 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
> >
> > /* empty string? */
> > if (targv[i][0] == '\0') {
> > - fprintf(stderr, "Invalid threads value %s.\n", targv[i]);
> > + xlog(L_ERROR, "Invalid threads value %s.", targv[i]);
> > return 1;
> > }
> >
> > pool_threads[i] = strtol(targv[i], &endptr, 0);
> > if (!endptr || *endptr != '\0') {
> > - fprintf(stderr, "Invalid threads value %s.\n", argv[1]);
> > + xlog(L_ERROR, "Invalid threads value %s.", argv[1]);
> > return 1;
> > }
> > }
> > @@ -621,14 +619,14 @@ static int fetch_nfsd_versions(struct nl_sock *sock)
> >
> > cb = nl_cb_alloc(NL_CB_CUSTOM);
> > if (!cb) {
> > - fprintf(stderr, "failed to allocate netlink callbacks\n");
> > + xlog(L_ERROR, "failed to allocate netlink callbacks");
> > ret = 1;
> > goto out;
> > }
> >
> > ret = nl_send_auto(sock, msg);
> > if (ret < 0) {
> > - fprintf(stderr, "send failed: %d\n", ret);
> > + xlog(L_ERROR, "send failed: %d", ret);
> > goto out_cb;
> > }
> >
> > @@ -641,7 +639,7 @@ static int fetch_nfsd_versions(struct nl_sock *sock)
> > while (ret > 0)
> > nl_recvmsgs(sock, cb);
> > if (ret < 0) {
> > - fprintf(stderr, "Error: %s\n", strerror(-ret));
> > + xlog(L_ERROR, "Error: %s", strerror(-ret));
> > ret = 1;
> > }
> > out_cb:
> > @@ -690,7 +688,7 @@ static int set_nfsd_versions(struct nl_sock *sock)
> >
> > a = nla_nest_start(msg, NLA_F_NESTED | NFSD_A_SERVER_PROTO_VERSION);
> > if (!a) {
> > - fprintf(stderr, "Unable to allocate version nest!\n");
> > + xlog(L_ERROR, "Unable to allocate version nest!");
> > ret = 1;
> > goto out;
> > }
> > @@ -707,14 +705,14 @@ static int set_nfsd_versions(struct nl_sock *sock)
> >
> > cb = nl_cb_alloc(NL_CB_CUSTOM);
> > if (!cb) {
> > - fprintf(stderr, "Failed to allocate netlink callbacks\n");
> > + xlog(L_ERROR, "Failed to allocate netlink callbacks");
> > ret = 1;
> > goto out;
> > }
> >
> > ret = nl_send_auto(sock, msg);
> > if (ret < 0) {
> > - fprintf(stderr, "Send failed: %d\n", ret);
> > + xlog(L_ERROR, "Send failed: %d", ret);
> > goto out_cb;
> > }
> >
> > @@ -727,7 +725,7 @@ static int set_nfsd_versions(struct nl_sock *sock)
> > while (ret > 0)
> > nl_recvmsgs(sock, cb);
> > if (ret < 0) {
> > - fprintf(stderr, "Error: %s\n", strerror(-ret));
> > + xlog(L_ERROR, "Error: %s", strerror(-ret));
> > ret = 1;
> > }
> > out_cb:
> > @@ -752,7 +750,7 @@ static int update_nfsd_version(int major, int minor, bool enabled)
> > /* the kernel doesn't support this version */
> > if (!enabled)
> > return 0;
> > - fprintf(stderr, "This kernel does not support NFS version %d.%d\n", major, minor);
> > + xlog(L_ERROR, "This kernel does not support NFS version %d.%d", major, minor);
> > return -EINVAL;
> > }
> >
> > @@ -794,7 +792,7 @@ static int version_func(struct nl_sock *sock, int argc, char ** argv)
> >
> > ret = sscanf(str, "%c%d.%d\n", &sign, &major, &minor);
> > if (ret < 2) {
> > - fprintf(stderr, "Invalid version string (%d) %s\n", ret, str);
> > + xlog(L_ERROR, "Invalid version string (%d) %s", ret, str);
> > return -EINVAL;
> > }
> >
> > @@ -806,7 +804,7 @@ static int version_func(struct nl_sock *sock, int argc, char ** argv)
> > enabled = false;
> > break;
> > default:
> > - fprintf(stderr, "Invalid version string %s\n", str);
> > + xlog(L_ERROR, "Invalid version string %s", str);
> > return -EINVAL;
> > }
> >
> > @@ -839,14 +837,14 @@ static int fetch_current_listeners(struct nl_sock *sock)
> >
> > cb = nl_cb_alloc(NL_CB_CUSTOM);
> > if (!cb) {
> > - fprintf(stderr, "failed to allocate netlink callbacks\n");
> > + xlog(L_ERROR, "failed to allocate netlink callbacks");
> > ret = 1;
> > goto out;
> > }
> >
> > ret = nl_send_auto(sock, msg);
> > if (ret < 0) {
> > - fprintf(stderr, "send failed: %d\n", ret);
> > + xlog(L_ERROR, "send failed: %d", ret);
> > goto out_cb;
> > }
> >
> > @@ -859,7 +857,7 @@ static int fetch_current_listeners(struct nl_sock *sock)
> > while (ret > 0)
> > nl_recvmsgs(sock, cb);
> > if (ret < 0) {
> > - fprintf(stderr, "Error: %s\n", strerror(-ret));
> > + xlog(L_ERROR, "Error: %s", strerror(-ret));
> > ret = 1;
> > }
> > out_cb:
> > @@ -994,7 +992,7 @@ static int update_listeners(const char *str)
> > */
> > ret = getaddrinfo(addr, port, &hints, &res);
> > if (ret) {
> > - fprintf(stderr, "getaddrinfo of \"%s\" failed: %s\n",
> > + xlog(L_ERROR, "getaddrinfo of \"%s\" failed: %s",
> > addr, gai_strerror(ret));
> > return -EINVAL;
> > }
> > @@ -1046,7 +1044,7 @@ static int update_listeners(const char *str)
> > }
> > return 0;
> > out_inval:
> > - fprintf(stderr, "Invalid listener update string: %s", str);
> > + xlog(L_ERROR, "Invalid listener update string: %s", str);
> > return -EINVAL;
> > }
> >
> > @@ -1076,7 +1074,7 @@ static int set_listeners(struct nl_sock *sock)
> >
> > a = nla_nest_start(msg, NLA_F_NESTED | NFSD_A_SERVER_SOCK_ADDR);
> > if (!a) {
> > - fprintf(stderr, "Unable to allocate listener nest!\n");
> > + xlog(L_ERROR, "Unable to allocate listener nest!");
> > ret = 1;
> > goto out;
> > }
> > @@ -1091,14 +1089,14 @@ static int set_listeners(struct nl_sock *sock)
> >
> > cb = nl_cb_alloc(NL_CB_CUSTOM);
> > if (!cb) {
> > - fprintf(stderr, "Failed to allocate netlink callbacks\n");
> > + xlog(L_ERROR, "Failed to allocate netlink callbacks");
> > ret = 1;
> > goto out;
> > }
> >
> > ret = nl_send_auto(sock, msg);
> > if (ret < 0) {
> > - fprintf(stderr, "Send failed: %d\n", ret);
> > + xlog(L_ERROR, "Send failed: %d", ret);
> > goto out_cb;
> > }
> >
> > @@ -1111,7 +1109,7 @@ static int set_listeners(struct nl_sock *sock)
> > while (ret > 0)
> > nl_recvmsgs(sock, cb);
> > if (ret < 0) {
> > - fprintf(stderr, "Error: %s\n", strerror(-ret));
> > + xlog(L_ERROR, "Error: %s", strerror(-ret));
> > ret = 1;
> > }
> > out_cb:
> > @@ -1188,14 +1186,14 @@ static int pool_mode_doit(struct nl_sock *sock, int cmd, const char *pool_mode)
> >
> > cb = nl_cb_alloc(NL_CB_CUSTOM);
> > if (!cb) {
> > - fprintf(stderr, "failed to allocate netlink callbacks\n");
> > + xlog(L_ERROR, "failed to allocate netlink callbacks");
> > ret = 1;
> > goto out;
> > }
> >
> > ret = nl_send_auto(sock, msg);
> > if (ret < 0) {
> > - fprintf(stderr, "send failed (%d)!\n", ret);
> > + xlog(L_ERROR, "send failed (%d)!", ret);
> > goto out_cb;
> > }
> >
> > @@ -1208,7 +1206,7 @@ static int pool_mode_doit(struct nl_sock *sock, int cmd, const char *pool_mode)
> > while (ret > 0)
> > nl_recvmsgs(sock, cb);
> > if (ret < 0) {
> > - fprintf(stderr, "Error: %s\n", strerror(-ret));
> > + xlog(L_ERROR, "Error: %s", strerror(-ret));
> > ret = 1;
> > }
> > out_cb:
> > @@ -1245,7 +1243,7 @@ static int pool_mode_func(struct nl_sock *sock, int argc, char **argv)
> >
> > /* empty string? */
> > if (*targv[0] == '\0') {
> > - fprintf(stderr, "Invalid threads value %s.\n", targv[0]);
> > + xlog(L_ERROR, "Invalid threads value %s.", targv[0]);
> > return 1;
> > }
> > pool_mode = targv[0];
> > @@ -1397,7 +1395,7 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
> >
> > threads[idx++] = strtol(n->field, &endptr, 0);
> > if (!endptr || *endptr != '\0') {
> > - fprintf(stderr, "Invalid threads value %s.\n", n->field);
> > + xlog(L_ERROR, "Invalid threads value %s.", n->field);
> > ret = -EINVAL;
> > goto out;
> > }
> > @@ -1470,7 +1468,8 @@ static void usage(void)
>
> You missed some other changes in the usage() function (some boilerplate
> stuff in the printf's).
>
> -Scott
> > /* Options given before the command string */
> > static const struct option pre_options[] = {
> > { "help", no_argument, NULL, 'h' },
> > - { "debug", required_argument, NULL, 'd' },
> > + { "debug", no_argument, NULL, 'd' },
> > + { "syslog", no_argument, NULL, 's' },
> > { "version", no_argument, NULL, 'V' },
> > { },
> > };
> > @@ -1524,7 +1523,7 @@ static int run_commandline(struct nl_sock *sock)
> > if (!ret)
> > ret = run_one_command(sock, argc, argv);
> > if (ret)
> > - fprintf(stderr, "Error: %s\n", strerror(ret));
> > + xlog(L_ERROR, "Error: %s", strerror(ret));
> > free(line);
> > }
> > return 0;
> > @@ -1533,23 +1532,25 @@ static int run_commandline(struct nl_sock *sock)
> > int main(int argc, char **argv)
> > {
> > int opt, ret;
> > - struct nl_sock *sock = netlink_sock_alloc();
> > -
> > - if (!sock) {
> > - fprintf(stderr, "Unable to allocate netlink socket!");
> > - return 1;
> > - }
> > + struct nl_sock *sock;
> >
> > taskname = argv[0];
> >
> > + xlog_syslog(0);
> > + xlog_stderr(1);
> > +
> > /* Parse the preliminary options */
> > - while ((opt = getopt_long(argc, argv, "+hd:V", pre_options, NULL)) != -1) {
> > + while ((opt = getopt_long(argc, argv, "+hdsV", pre_options, NULL)) != -1) {
> > switch (opt) {
> > case 'h':
> > usage();
> > return 0;
> > case 'd':
> > - debug_level = atoi(optarg);
> > + xlog_config(D_ALL, 1);
> > + break;
> > + case 's':
> > + xlog_syslog(1);
> > + xlog_stderr(0);
> > break;
> > case 'V':
> > // FIXME: print_version();
> > @@ -1557,6 +1558,14 @@ int main(int argc, char **argv)
> > }
> > }
> >
> > + xlog_open(basename(argv[0]));
> > +
> > + sock = netlink_sock_alloc();
> > + if (!sock) {
> > + xlog(L_ERROR, "Unable to allocate netlink socket!");
> > + return 1;
> > + }
> > +
> > if (optind > argc) {
> > usage();
> > return 1;
> >
> > --
> > 2.47.1
> >
>
--
Jeff Layton <jlayton@xxxxxxxxxx>
