From: Scott Mayhew <smayhew@redhat.com>
To: Jeff Layton <jlayton@kernel.org>
Cc: Steve Dickson <steved@redhat.com>,
Yongcheng Yang <yoyang@redhat.com>,
linux-nfs@vger.kernel.org
Subject: Re: [PATCH 1/3] nfsdctl: convert to xlog()
Date: Thu, 9 Jan 2025 15:29:46 -0500 [thread overview]
Message-ID: <Z4Axui-NRCuI0Epi@aion> (raw)
In-Reply-To: <20250109-lockd-nl-v1-1-108548ab0b6b@kernel.org>
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.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> 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.
> 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
>
next prev parent reply other threads:[~2025-01-09 20:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-09 19:40 [PATCH 0/3] nfsdctl: add support for new lockd configuration interface Jeff Layton
2025-01-09 19:40 ` [PATCH 1/3] nfsdctl: convert to xlog() Jeff Layton
2025-01-09 20:29 ` Scott Mayhew [this message]
2025-01-09 20:39 ` Jeff Layton
2025-01-09 19:40 ` [PATCH 2/3] nfsdctl: fix the --version option Jeff Layton
2025-01-09 19:40 ` [PATCH 3/3] nfsdctl: add necessary bits to configure lockd 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=Z4Axui-NRCuI0Epi@aion \
--to=smayhew@redhat.com \
--cc=jlayton@kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=steved@redhat.com \
--cc=yoyang@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.