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

Steve Dickson
2025-01-15T17:47:35Z

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?

steved.