To aid in debugging problems and verifying general
functionality, this patch set adds error logging
and a -v verbosity flag.
Steve Dickson (2):
nfsidmap: Added Error Logging
nfsidmap: Added -v flag
utils/nfsidmap/Makefile.am | 2 +-
utils/nfsidmap/nfsidmap.c | 46 +++++++++++++++++++++++++++++++++++++++---
utils/nfsidmap/nfsidmap.man | 15 +++++++++++--
3 files changed, 55 insertions(+), 8 deletions(-)
--
1.7.6.4
On 11/10/2011 07:23 PM, Jim Rees wrote:
> Steve Dickson wrote:
>
>
>
> On 11/10/2011 04:11 PM, Jim Rees wrote:
> > Steve Dickson wrote:
> >
> > To aid in debugging, the -v flag can now be specified
> > on the command to enable verbose logging in both
> > the nfsidmap command and libnfsidmap library routines.
> >
> > Signed-off-by: Steve Dickson <[email protected]>
> > ---
> > utils/nfsidmap/nfsidmap.c | 12 ++++++++++++
> > utils/nfsidmap/nfsidmap.man | 15 ++++++++++++---
> > 2 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
> > index 134d9bc..d74189a 100644
> > --- a/utils/nfsidmap/nfsidmap.c
> > +++ b/utils/nfsidmap/nfsidmap.c
> > @@ -12,6 +12,7 @@
> > #include <syslog.h>
> > #include "xlog.h"
> >
> > +int verbose = 0;
> > /* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
> >
> > #define MAX_ID_LEN 11
> > @@ -108,6 +109,12 @@ int main(int argc, char **argv)
> > xlog_syslog(1);
> > xlog_stderr(0);
> >
> > + if (argc > 1 && strcmp(argv[1], "-v") == 0) {
> > + verbose = 1;
> > + nfs4_set_debug(1, NULL);
> > + argc--, argv++;
> > + }
> > +
> >
> > Ugh. Is there some reason not to use getopt() like all the other utils do?
> I was waiting for this comment ;-)
>
> I'm always happy to play your straight man.
>
> I decided to go with how the command was originally written
> because this command is only call from the kernel so a user
> should execute it (except for debugging).
>
> The arguments are vary static on where they need to be
> no command line. So its either going to work or not,
> which means there is no real need for a usage error (expect
> for the one I added).
>
> Finally, is there real need for a while loop and switch statement
> for on simple case? I thought not...
>
> It's more work for the next guy who comes along and wants to add another
> option, especially if the new option takes an argument.
I still think its overkill but I'm always a fan
of making easier for the next guys down the line...
I'll re-spin it...
steved.
To aid in debugging, the -v flag can now be specified
on the command to enable verbose logging in both
the nfsidmap command and libnfsidmap library routines.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/nfsidmap/nfsidmap.c | 12 ++++++++++++
utils/nfsidmap/nfsidmap.man | 15 ++++++++++++---
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 134d9bc..d74189a 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -12,6 +12,7 @@
#include <syslog.h>
#include "xlog.h"
+int verbose = 0;
/* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
#define MAX_ID_LEN 11
@@ -108,6 +109,12 @@ int main(int argc, char **argv)
xlog_syslog(1);
xlog_stderr(0);
+ if (argc > 1 && strcmp(argv[1], "-v") == 0) {
+ verbose = 1;
+ nfs4_set_debug(1, NULL);
+ argc--, argv++;
+ }
+
if (argc < 3) {
xlog_err("Bad arg count. Check /etc/request-key.conf");
return 1;
@@ -126,6 +133,11 @@ int main(int argc, char **argv)
key = strtol(argv[1], NULL, 10);
+ if (verbose) {
+ xlog_warn("key: %ld type: %s value: %s timeout %ld",
+ key, type, value, timeout);
+ }
+
if (strcmp(type, "uid") == 0)
rc = id_lookup(value, key, USER);
else if (strcmp(type, "gid") == 0)
diff --git a/utils/nfsidmap/nfsidmap.man b/utils/nfsidmap/nfsidmap.man
index 2381908..aa4d94b 100644
--- a/utils/nfsidmap/nfsidmap.man
+++ b/utils/nfsidmap/nfsidmap.man
@@ -5,6 +5,8 @@
.TH nfsidmap 5 "1 October 2010"
.SH NAME
nfsidmap \- The NFS idmapper upcall program
+.SH SYNOPSIS
+.B "nfsidmap [-v] key user [timeout]"
.SH DESCRIPTION
The file
.I /usr/sbin/nfsidmap
@@ -14,9 +16,16 @@ the upcall and cache the result.
.I /usr/sbin/nfsidmap
should only be called by request-key, and will perform the translation and
initialize a key with the resulting information.
-.PP
-NFS_USE_NEW_IDMAPPER must be selected when configuring the kernel to use this
-feature.
+.SH OPTIONS
+.TP
+.B -v
+Enables verbose logging in both the
+.B nfsidmap
+binary and in the library routines
+that are used.
+.B Note,
+the -v flag has to be the first argument on the
+command to enable the logging.
.SH CONFIGURING
The file
.I /etc/request-key.conf
--
1.7.6.4
Steve Dickson wrote:
To aid in debugging, the -v flag can now be specified
on the command to enable verbose logging in both
the nfsidmap command and libnfsidmap library routines.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/nfsidmap/nfsidmap.c | 12 ++++++++++++
utils/nfsidmap/nfsidmap.man | 15 ++++++++++++---
2 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 134d9bc..d74189a 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -12,6 +12,7 @@
#include <syslog.h>
#include "xlog.h"
+int verbose = 0;
/* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
#define MAX_ID_LEN 11
@@ -108,6 +109,12 @@ int main(int argc, char **argv)
xlog_syslog(1);
xlog_stderr(0);
+ if (argc > 1 && strcmp(argv[1], "-v") == 0) {
+ verbose = 1;
+ nfs4_set_debug(1, NULL);
+ argc--, argv++;
+ }
+
Ugh. Is there some reason not to use getopt() like all the other utils do?
On 11/10/2011 04:19 PM, Bryan Schumaker wrote:
> On 11/10/2011 03:26 PM, Steve Dickson wrote:
>> To aid in debugging, the -v flag can now be specified
>> on the command to enable verbose logging in both
>> the nfsidmap command and libnfsidmap library routines.
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> utils/nfsidmap/nfsidmap.c | 12 ++++++++++++
>> utils/nfsidmap/nfsidmap.man | 15 ++++++++++++---
>> 2 files changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
>> index 134d9bc..d74189a 100644
>> --- a/utils/nfsidmap/nfsidmap.c
>> +++ b/utils/nfsidmap/nfsidmap.c
>> @@ -12,6 +12,7 @@
>> #include <syslog.h>
>> #include "xlog.h"
>
> Is the syslog.h still needed here? I don't think I see anything that uses it.
Probably not...
> If I knew about xlog, I probably would have used that while writing nfsidmap.
No biggie... :-)
steved.
>
> - Bryan
>
>>
>> +int verbose = 0;
>> /* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
>>
>> #define MAX_ID_LEN 11
>> @@ -108,6 +109,12 @@ int main(int argc, char **argv)
>> xlog_syslog(1);
>> xlog_stderr(0);
>>
>> + if (argc > 1 && strcmp(argv[1], "-v") == 0) {
>> + verbose = 1;
>> + nfs4_set_debug(1, NULL);
>> + argc--, argv++;
>> + }
>> +
>> if (argc < 3) {
>> xlog_err("Bad arg count. Check /etc/request-key.conf");
>> return 1;
>> @@ -126,6 +133,11 @@ int main(int argc, char **argv)
>>
>> key = strtol(argv[1], NULL, 10);
>>
>> + if (verbose) {
>> + xlog_warn("key: %ld type: %s value: %s timeout %ld",
>> + key, type, value, timeout);
>> + }
>> +
>> if (strcmp(type, "uid") == 0)
>> rc = id_lookup(value, key, USER);
>> else if (strcmp(type, "gid") == 0)
>> diff --git a/utils/nfsidmap/nfsidmap.man b/utils/nfsidmap/nfsidmap.man
>> index 2381908..aa4d94b 100644
>> --- a/utils/nfsidmap/nfsidmap.man
>> +++ b/utils/nfsidmap/nfsidmap.man
>> @@ -5,6 +5,8 @@
>> .TH nfsidmap 5 "1 October 2010"
>> .SH NAME
>> nfsidmap \- The NFS idmapper upcall program
>> +.SH SYNOPSIS
>> +.B "nfsidmap [-v] key user [timeout]"
>> .SH DESCRIPTION
>> The file
>> .I /usr/sbin/nfsidmap
>> @@ -14,9 +16,16 @@ the upcall and cache the result.
>> .I /usr/sbin/nfsidmap
>> should only be called by request-key, and will perform the translation and
>> initialize a key with the resulting information.
>> -.PP
>> -NFS_USE_NEW_IDMAPPER must be selected when configuring the kernel to use this
>> -feature.
>> +.SH OPTIONS
>> +.TP
>> +.B -v
>> +Enables verbose logging in both the
>> +.B nfsidmap
>> +binary and in the library routines
>> +that are used.
>> +.B Note,
>> +the -v flag has to be the first argument on the
>> +command to enable the logging.
>> .SH CONFIGURING
>> The file
>> .I /etc/request-key.conf
>
Since this binary is being called by the kernel,
errors need to be logged to the syslog for
help in debugging problems.
Signed-off-by: Steve Dickson <[email protected]>
---
utils/nfsidmap/Makefile.am | 2 +-
utils/nfsidmap/nfsidmap.c | 34 ++++++++++++++++++++++++++++++----
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/utils/nfsidmap/Makefile.am b/utils/nfsidmap/Makefile.am
index f837b91..037aa79 100644
--- a/utils/nfsidmap/Makefile.am
+++ b/utils/nfsidmap/Makefile.am
@@ -4,6 +4,6 @@ man8_MANS = nfsidmap.man
sbin_PROGRAMS = nfsidmap
nfsidmap_SOURCES = nfsidmap.c
-nfsidmap_LDADD = -lnfsidmap -lkeyutils
+nfsidmap_LDADD = -lnfsidmap -lkeyutils ../../support/nfs/libnfs.a
MAINTAINERCLEANFILES = Makefile.in
diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
index 2d87381..134d9bc 100644
--- a/utils/nfsidmap/nfsidmap.c
+++ b/utils/nfsidmap/nfsidmap.c
@@ -10,6 +10,7 @@
#include <nfsidmap.h>
#include <syslog.h>
+#include "xlog.h"
/* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
@@ -36,9 +37,15 @@ int id_lookup(char *name_at_domain, key_serial_t key, int type)
rc = nfs4_group_owner_to_gid(name_at_domain, &gid);
sprintf(id, "%u", gid);
}
+ if (rc < 0)
+ xlog_err("id_lookup: %s: failed: %m",
+ (type == USER ? "nfs4_owner_to_uid" : "nfs4_group_owner_to_gid"));
- if (rc == 0)
+ if (rc == 0) {
rc = keyctl_instantiate(key, id, strlen(id) + 1, 0);
+ if (rc < 0)
+ xlog_err("id_lookup: keyctl_instantiate failed: %m");
+ }
return rc;
}
@@ -57,6 +64,7 @@ int name_lookup(char *id, key_serial_t key, int type)
rc = nfs4_get_default_domain(NULL, domain, NFS4_MAX_DOMAIN_LEN);
if (rc != 0) {
rc = -1;
+ xlog_err("name_lookup: nfs4_get_default_domain failed: %m");
goto out;
}
@@ -67,10 +75,15 @@ int name_lookup(char *id, key_serial_t key, int type)
gid = atoi(id);
rc = nfs4_gid_to_name(gid, domain, name, IDMAP_NAMESZ);
}
+ if (rc < 0)
+ xlog_err("name_lookup: %s: failed: %m",
+ (type == USER ? "nfs4_uid_to_name" : "nfs4_gid_to_name"));
- if (rc == 0)
+ if (rc == 0) {
rc = keyctl_instantiate(key, &name, strlen(name), 0);
-
+ if (rc < 0)
+ xlog_err("name_lookup: keyctl_instantiate failed: %m");
+ }
out:
return rc;
}
@@ -83,9 +96,22 @@ int main(int argc, char **argv)
int rc = 1;
int timeout = 600;
key_serial_t key;
+ char *progname;
+
+ /* Set the basename */
+ if ((progname = strrchr(argv[0], '/')) != NULL)
+ progname++;
+ else
+ progname = argv[0];
- if (argc < 3)
+ xlog_open(progname);
+ xlog_syslog(1);
+ xlog_stderr(0);
+
+ if (argc < 3) {
+ xlog_err("Bad arg count. Check /etc/request-key.conf");
return 1;
+ }
arg = malloc(sizeof(char) * strlen(argv[2]) + 1);
strcpy(arg, argv[2]);
--
1.7.6.4
Steve Dickson wrote:
On 11/10/2011 04:11 PM, Jim Rees wrote:
> Steve Dickson wrote:
>
> To aid in debugging, the -v flag can now be specified
> on the command to enable verbose logging in both
> the nfsidmap command and libnfsidmap library routines.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/nfsidmap/nfsidmap.c | 12 ++++++++++++
> utils/nfsidmap/nfsidmap.man | 15 ++++++++++++---
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
> index 134d9bc..d74189a 100644
> --- a/utils/nfsidmap/nfsidmap.c
> +++ b/utils/nfsidmap/nfsidmap.c
> @@ -12,6 +12,7 @@
> #include <syslog.h>
> #include "xlog.h"
>
> +int verbose = 0;
> /* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
>
> #define MAX_ID_LEN 11
> @@ -108,6 +109,12 @@ int main(int argc, char **argv)
> xlog_syslog(1);
> xlog_stderr(0);
>
> + if (argc > 1 && strcmp(argv[1], "-v") == 0) {
> + verbose = 1;
> + nfs4_set_debug(1, NULL);
> + argc--, argv++;
> + }
> +
>
> Ugh. Is there some reason not to use getopt() like all the other utils do?
I was waiting for this comment ;-)
I'm always happy to play your straight man.
I decided to go with how the command was originally written
because this command is only call from the kernel so a user
should execute it (except for debugging).
The arguments are vary static on where they need to be
no command line. So its either going to work or not,
which means there is no real need for a usage error (expect
for the one I added).
Finally, is there real need for a while loop and switch statement
for on simple case? I thought not...
It's more work for the next guy who comes along and wants to add another
option, especially if the new option takes an argument.
On 11/10/2011 04:11 PM, Jim Rees wrote:
> Steve Dickson wrote:
>
> To aid in debugging, the -v flag can now be specified
> on the command to enable verbose logging in both
> the nfsidmap command and libnfsidmap library routines.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/nfsidmap/nfsidmap.c | 12 ++++++++++++
> utils/nfsidmap/nfsidmap.man | 15 ++++++++++++---
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
> index 134d9bc..d74189a 100644
> --- a/utils/nfsidmap/nfsidmap.c
> +++ b/utils/nfsidmap/nfsidmap.c
> @@ -12,6 +12,7 @@
> #include <syslog.h>
> #include "xlog.h"
>
> +int verbose = 0;
> /* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
>
> #define MAX_ID_LEN 11
> @@ -108,6 +109,12 @@ int main(int argc, char **argv)
> xlog_syslog(1);
> xlog_stderr(0);
>
> + if (argc > 1 && strcmp(argv[1], "-v") == 0) {
> + verbose = 1;
> + nfs4_set_debug(1, NULL);
> + argc--, argv++;
> + }
> +
>
> Ugh. Is there some reason not to use getopt() like all the other utils do?
I was waiting for this comment ;-)
I decided to go with how the command was originally written
because this command is only call from the kernel so a user
should execute it (except for debugging).
The arguments are vary static on where they need to be
no command line. So its either going to work or not,
which means there is no real need for a usage error (expect
for the one I added).
Finally, is there real need for a while loop and switch statement
for on simple case? I thought not...
Those are my thoughts..
steved.
On 11/10/2011 03:26 PM, Steve Dickson wrote:
> To aid in debugging, the -v flag can now be specified
> on the command to enable verbose logging in both
> the nfsidmap command and libnfsidmap library routines.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/nfsidmap/nfsidmap.c | 12 ++++++++++++
> utils/nfsidmap/nfsidmap.man | 15 ++++++++++++---
> 2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/utils/nfsidmap/nfsidmap.c b/utils/nfsidmap/nfsidmap.c
> index 134d9bc..d74189a 100644
> --- a/utils/nfsidmap/nfsidmap.c
> +++ b/utils/nfsidmap/nfsidmap.c
> @@ -12,6 +12,7 @@
> #include <syslog.h>
> #include "xlog.h"
Is the syslog.h still needed here? I don't think I see anything that uses it. If I knew about xlog, I probably would have used that while writing nfsidmap.
- Bryan
>
> +int verbose = 0;
> /* gcc nfsidmap.c -o nfsidmap -l nfsidmap -l keyutils */
>
> #define MAX_ID_LEN 11
> @@ -108,6 +109,12 @@ int main(int argc, char **argv)
> xlog_syslog(1);
> xlog_stderr(0);
>
> + if (argc > 1 && strcmp(argv[1], "-v") == 0) {
> + verbose = 1;
> + nfs4_set_debug(1, NULL);
> + argc--, argv++;
> + }
> +
> if (argc < 3) {
> xlog_err("Bad arg count. Check /etc/request-key.conf");
> return 1;
> @@ -126,6 +133,11 @@ int main(int argc, char **argv)
>
> key = strtol(argv[1], NULL, 10);
>
> + if (verbose) {
> + xlog_warn("key: %ld type: %s value: %s timeout %ld",
> + key, type, value, timeout);
> + }
> +
> if (strcmp(type, "uid") == 0)
> rc = id_lookup(value, key, USER);
> else if (strcmp(type, "gid") == 0)
> diff --git a/utils/nfsidmap/nfsidmap.man b/utils/nfsidmap/nfsidmap.man
> index 2381908..aa4d94b 100644
> --- a/utils/nfsidmap/nfsidmap.man
> +++ b/utils/nfsidmap/nfsidmap.man
> @@ -5,6 +5,8 @@
> .TH nfsidmap 5 "1 October 2010"
> .SH NAME
> nfsidmap \- The NFS idmapper upcall program
> +.SH SYNOPSIS
> +.B "nfsidmap [-v] key user [timeout]"
> .SH DESCRIPTION
> The file
> .I /usr/sbin/nfsidmap
> @@ -14,9 +16,16 @@ the upcall and cache the result.
> .I /usr/sbin/nfsidmap
> should only be called by request-key, and will perform the translation and
> initialize a key with the resulting information.
> -.PP
> -NFS_USE_NEW_IDMAPPER must be selected when configuring the kernel to use this
> -feature.
> +.SH OPTIONS
> +.TP
> +.B -v
> +Enables verbose logging in both the
> +.B nfsidmap
> +binary and in the library routines
> +that are used.
> +.B Note,
> +the -v flag has to be the first argument on the
> +command to enable the logging.
> .SH CONFIGURING
> The file
> .I /etc/request-key.conf