* [PATCH 01/10] nfs-utils: don't link libexport.a and libmisc.a to nfsd
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-03 19:52 ` [PATCH 02/10] nfs-utils: clean up option parsing in nfsd.c Jeff Layton
` (8 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
...they aren't needed.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsd/Makefile.am | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/utils/nfsd/Makefile.am b/utils/nfsd/Makefile.am
index 445e3fd..ea85cd5 100644
--- a/utils/nfsd/Makefile.am
+++ b/utils/nfsd/Makefile.am
@@ -8,9 +8,7 @@ KPREFIX = @kprefix@
sbin_PROGRAMS = nfsd
nfsd_SOURCES = nfsd.c
-nfsd_LDADD = ../../support/export/libexport.a \
- ../../support/nfs/libnfs.a \
- ../../support/misc/libmisc.a
+nfsd_LDADD = ../../support/nfs/libnfs.a
MAINTAINERCLEANFILES = Makefile.in
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 02/10] nfs-utils: clean up option parsing in nfsd.c
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
2009-06-03 19:52 ` [PATCH 01/10] nfs-utils: don't link libexport.a and libmisc.a to nfsd Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-03 19:52 ` [PATCH 03/10] nfs-utils: convert rpc.nfsd to use xlog() Jeff Layton
` (7 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
Minor formatting nits.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsd/nfsd.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index e3c0094..c7bd003 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -97,11 +97,11 @@ main(int argc, char **argv)
}
break;
case 'T':
- NFSCTL_TCPUNSET(protobits);
- break;
+ NFSCTL_TCPUNSET(protobits);
+ break;
case 'U':
- NFSCTL_UDPUNSET(protobits);
- break;
+ NFSCTL_UDPUNSET(protobits);
+ break;
default:
fprintf(stderr, "Invalid argument: '%c'\n", c);
case 'h':
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 03/10] nfs-utils: convert rpc.nfsd to use xlog()
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
2009-06-03 19:52 ` [PATCH 01/10] nfs-utils: don't link libexport.a and libmisc.a to nfsd Jeff Layton
2009-06-03 19:52 ` [PATCH 02/10] nfs-utils: clean up option parsing in nfsd.c Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-03 20:01 ` Chuck Lever
[not found] ` <7968D2B9-3E31-4E0D-9D0B-309D757A0014@oracle.com>
2009-06-03 19:52 ` [PATCH 04/10] nfs-utils: clean up NFSCTL_* macros for handling protocol bits Jeff Layton
` (6 subsequent siblings)
9 siblings, 2 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
...and add a --debug option to make nfsd log messages to stderr.
rpc.nfsd is usually run out of init scripts in which case logging to
syslog makes more sense. Make that the default and add a switch that
makes log messages go to stderr. Eventually however nfsd has to close
stderr, at which point the program switches to logging to syslog
unconditionally.
For now, stderr gets closed rather early, so --debug isn't terribly
helpful. Later patches will make rpc.nfsd delay closing of stderr longer
which should make it more useful.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
support/nfs/nfssvc.c | 46 +++++++++++++++++-------------------
utils/nfsd/nfsd.c | 63 +++++++++++++++++++++++++++++++-------------------
2 files changed, 61 insertions(+), 48 deletions(-)
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 33c15a7..3e6bd31 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -16,10 +16,9 @@
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
-#include <syslog.h>
-
#include "nfslib.h"
+#include "xlog.h"
#define NFSD_PORTS_FILE "/proc/fs/nfsd/portlist"
#define NFSD_VERS_FILE "/proc/fs/nfsd/versions"
@@ -57,13 +56,13 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
if (NFSCTL_UDPISSET(ctlbits)) {
udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (udpfd < 0) {
- syslog(LOG_ERR, "nfssvc: unable to create UPD socket: "
- "errno %d (%s)\n", errno, strerror(errno));
+ xlog(L_ERROR, "unable to create UDP socket: "
+ "errno %d (%m)", errno);
exit(1);
}
if (bind(udpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
- syslog(LOG_ERR, "nfssvc: unable to bind UPD socket: "
- "errno %d (%s)\n", errno, strerror(errno));
+ xlog(L_ERROR, "unable to bind UDP socket: "
+ "errno %d (%m)", errno);
exit(1);
}
}
@@ -71,32 +70,32 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
if (NFSCTL_TCPISSET(ctlbits)) {
tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
if (tcpfd < 0) {
- syslog(LOG_ERR, "nfssvc: unable to createt tcp socket: "
- "errno %d (%s)\n", errno, strerror(errno));
+ xlog(L_ERROR, "unable to createt tcp socket: "
+ "errno %d (%m)", errno);
exit(1);
}
if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) < 0) {
- syslog(LOG_ERR, "nfssvc: unable to set SO_REUSEADDR: "
- "errno %d (%s)\n", errno, strerror(errno));
+ xlog(L_ERROR, "unable to set SO_REUSEADDR: "
+ "errno %d (%m)", errno);
exit(1);
}
if (bind(tcpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
- syslog(LOG_ERR, "nfssvc: unable to bind TCP socket: "
- "errno %d (%s)\n", errno, strerror(errno));
+ xlog(L_ERROR, "unable to bind TCP socket: "
+ "errno %d (%m)", errno);
exit(1);
}
if (listen(tcpfd, 64) < 0){
- syslog(LOG_ERR, "nfssvc: unable to create listening socket: "
- "errno %d (%s)\n", errno, strerror(errno));
+ xlog(L_ERROR, "unable to create listening socket: "
+ "errno %d (%m)", errno);
exit(1);
}
}
if (udpfd >= 0) {
snprintf(buf, BUFSIZ,"%d\n", udpfd);
if (write(fd, buf, strlen(buf)) != strlen(buf)) {
- syslog(LOG_ERR,
- "nfssvc: writing fds to kernel failed: errno %d (%s)",
- errno, strerror(errno));
+ xlog(L_ERROR,
+ "writing fds to kernel failed: errno %d (%m)",
+ errno);
}
close(fd);
fd = -1;
@@ -106,9 +105,9 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
fd = open(NFSD_PORTS_FILE, O_WRONLY);
snprintf(buf, BUFSIZ,"%d\n", tcpfd);
if (write(fd, buf, strlen(buf)) != strlen(buf)) {
- syslog(LOG_ERR,
- "nfssvc: writing fds to kernel failed: errno %d (%s)",
- errno, strerror(errno));
+ xlog(L_ERROR,
+ "writing fds to kernel failed: errno %d (%m)",
+ errno);
}
}
close(fd);
@@ -139,10 +138,9 @@ nfssvc_versbits(unsigned int ctlbits, int minorvers4)
minorvers4 > 0 ? '+' : '-',
n);
snprintf(ptr+off, BUFSIZ - off, "\n");
- if (write(fd, buf, strlen(buf)) != strlen(buf)) {
- syslog(LOG_ERR, "nfssvc: Setting version failed: errno %d (%s)",
- errno, strerror(errno));
- }
+ if (write(fd, buf, strlen(buf)) != strlen(buf))
+ xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
+
close(fd);
return;
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index c7bd003..6dfea67 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -18,13 +18,13 @@
#include <string.h>
#include <errno.h>
#include <getopt.h>
-#include <syslog.h>
#include <netdb.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include "nfslib.h"
+#include "xlog.h"
static void usage(const char *);
@@ -37,6 +37,7 @@ static struct option longopts[] =
{ "no-udp", 0, 0, 'U' },
{ "port", 1, 0, 'P' },
{ "port", 1, 0, 'p' },
+ { "debug", 0, 0, 'd' },
{ NULL, 0, 0, 0 }
};
unsigned int protobits = NFSCTL_ALLBITS;
@@ -50,7 +51,7 @@ main(int argc, char **argv)
int count = 1, c, error, port, fd, found_one;
struct servent *ent;
struct hostent *hp;
- char *p;
+ char *p, *progname;
ent = getservbyname ("nfs", "udp");
if (ent != NULL)
@@ -58,8 +59,22 @@ main(int argc, char **argv)
else
port = 2049;
- while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", longopts, NULL)) != EOF) {
+ progname = strdup(basename(argv[0]));
+ if (!progname) {
+ fprintf(stderr, "%s: unable to allocate memory.\n", argv[0]);
+ exit(1);
+ }
+
+ xlog_syslog(1);
+ xlog_stderr(0);
+
+ while ((c = getopt_long(argc, argv, "dH:hN:p:P:TU", longopts, NULL)) != EOF) {
switch(c) {
+ case 'd':
+ xlog_config(D_ALL, 1);
+ xlog_syslog(0);
+ xlog_stderr(1);
+ break;
case 'H':
if (inet_addr(optarg) != INADDR_NONE) {
haddr = strdup(optarg);
@@ -67,8 +82,8 @@ main(int argc, char **argv)
haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
} else {
fprintf(stderr, "%s: Unknown hostname: %s\n",
- argv[0], optarg);
- usage(argv [0]);
+ progname, optarg);
+ usage(progname);
}
break;
case 'P': /* XXX for nfs-server compatibility */
@@ -76,8 +91,8 @@ main(int argc, char **argv)
port = atoi(optarg);
if (port <= 0 || port > 65535) {
fprintf(stderr, "%s: bad port number: %s\n",
- argv[0], optarg);
- usage(argv [0]);
+ progname, optarg);
+ usage(progname);
}
break;
case 'N':
@@ -105,14 +120,17 @@ main(int argc, char **argv)
default:
fprintf(stderr, "Invalid argument: '%c'\n", c);
case 'h':
- usage(argv[0]);
+ usage(progname);
}
}
+
+ xlog_open(progname);
+
/*
* Do some sanity checking, if the ctlbits are set
*/
if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
- fprintf(stderr, "invalid protocol specified\n");
+ xlog(L_ERROR, "invalid protocol specified");
exit(1);
}
found_one = 0;
@@ -121,12 +139,12 @@ main(int argc, char **argv)
found_one = 1;
}
if (!found_one) {
- fprintf(stderr, "no version specified\n");
+ xlog(L_ERROR, "no version specified");
exit(1);
}
if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
- fprintf(stderr, "version 4 requires the TCP protocol\n");
+ xlog(L_ERROR, "version 4 requires the TCP protocol");
exit(1);
}
if (haddr == NULL) {
@@ -135,17 +153,15 @@ main(int argc, char **argv)
}
if (chdir(NFS_STATEDIR)) {
- fprintf(stderr, "%s: chdir(%s) failed: %s\n",
- argv [0], NFS_STATEDIR, strerror(errno));
+ xlog(L_ERROR, "chdir(%s) failed: %m", NFS_STATEDIR);
exit(1);
}
if (optind < argc) {
if ((count = atoi(argv[optind])) < 0) {
/* insane # of servers */
- fprintf(stderr,
- "%s: invalid server count (%d), using 1\n",
- argv[0], count);
+ xlog(L_ERROR, "invalid server count (%d), using 1",
+ count);
count = 1;
}
}
@@ -155,21 +171,20 @@ main(int argc, char **argv)
everything before spawning kernel threads. --Chip */
fd = open("/dev/null", O_RDWR);
if (fd == -1)
- perror("/dev/null");
+ xlog(L_ERROR, "Unable to open /dev/null: %m");
else {
(void) dup2(fd, 0);
(void) dup2(fd, 1);
(void) dup2(fd, 2);
}
+ xlog_syslog(1);
+ xlog_stderr(0);
closeall(3);
- openlog("nfsd", LOG_PID, LOG_DAEMON);
- if ((error = nfssvc(port, count, versbits, minorvers4, protobits, haddr)) < 0) {
- int e = errno;
- syslog(LOG_ERR, "nfssvc: %s", strerror(e));
- closelog();
- }
+ if ((error = nfssvc(port, count, versbits, minorvers4, protobits, haddr)) < 0)
+ xlog(L_ERROR, "nfssvc (%m)");
+ free(progname);
return (error != 0);
}
@@ -177,7 +192,7 @@ static void
usage(const char *prog)
{
fprintf(stderr, "Usage:\n"
- "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
+ "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-T|--no-tcp] [-U|--no-udp] [-d|--debug] nrservs\n",
prog);
exit(2);
}
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 03/10] nfs-utils: convert rpc.nfsd to use xlog()
2009-06-03 19:52 ` [PATCH 03/10] nfs-utils: convert rpc.nfsd to use xlog() Jeff Layton
@ 2009-06-03 20:01 ` Chuck Lever
2009-06-03 20:22 ` Jeff Layton
[not found] ` <7968D2B9-3E31-4E0D-9D0B-309D757A0014@oracle.com>
1 sibling, 1 reply; 26+ messages in thread
From: Chuck Lever @ 2009-06-03 20:01 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs, steved
On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
> ...and add a --debug option to make nfsd log messages to stderr.
>
> rpc.nfsd is usually run out of init scripts in which case logging to
> syslog makes more sense. Make that the default and add a switch that
> makes log messages go to stderr. Eventually however nfsd has to close
> stderr, at which point the program switches to logging to syslog
> unconditionally.
>
> For now, stderr gets closed rather early, so --debug isn't terribly
> helpful. Later patches will make rpc.nfsd delay closing of stderr
> longer
> which should make it more useful.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> support/nfs/nfssvc.c | 46 +++++++++++++++++-------------------
> utils/nfsd/nfsd.c | 63 ++++++++++++++++++++++++++++++
> +-------------------
> 2 files changed, 61 insertions(+), 48 deletions(-)
>
> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> index 33c15a7..3e6bd31 100644
> --- a/support/nfs/nfssvc.c
> +++ b/support/nfs/nfssvc.c
> @@ -16,10 +16,9 @@
> #include <unistd.h>
> #include <fcntl.h>
> #include <errno.h>
> -#include <syslog.h>
> -
>
> #include "nfslib.h"
> +#include "xlog.h"
>
> #define NFSD_PORTS_FILE "/proc/fs/nfsd/portlist"
> #define NFSD_VERS_FILE "/proc/fs/nfsd/versions"
> @@ -57,13 +56,13 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> char *haddr)
> if (NFSCTL_UDPISSET(ctlbits)) {
> udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> if (udpfd < 0) {
> - syslog(LOG_ERR, "nfssvc: unable to create UPD socket: "
> - "errno %d (%s)\n", errno, strerror(errno));
> + xlog(L_ERROR, "unable to create UDP socket: "
> + "errno %d (%m)", errno);
> exit(1);
> }
> if (bind(udpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> - syslog(LOG_ERR, "nfssvc: unable to bind UPD socket: "
> - "errno %d (%s)\n", errno, strerror(errno));
> + xlog(L_ERROR, "unable to bind UDP socket: "
> + "errno %d (%m)", errno);
> exit(1);
> }
> }
> @@ -71,32 +70,32 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> char *haddr)
> if (NFSCTL_TCPISSET(ctlbits)) {
> tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> if (tcpfd < 0) {
> - syslog(LOG_ERR, "nfssvc: unable to createt tcp socket: "
> - "errno %d (%s)\n", errno, strerror(errno));
> + xlog(L_ERROR, "unable to createt tcp socket: "
> + "errno %d (%m)", errno);
> exit(1);
> }
> if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) <
> 0) {
> - syslog(LOG_ERR, "nfssvc: unable to set SO_REUSEADDR: "
> - "errno %d (%s)\n", errno, strerror(errno));
> + xlog(L_ERROR, "unable to set SO_REUSEADDR: "
> + "errno %d (%m)", errno);
> exit(1);
> }
> if (bind(tcpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> - syslog(LOG_ERR, "nfssvc: unable to bind TCP socket: "
> - "errno %d (%s)\n", errno, strerror(errno));
> + xlog(L_ERROR, "unable to bind TCP socket: "
> + "errno %d (%m)", errno);
> exit(1);
> }
> if (listen(tcpfd, 64) < 0){
> - syslog(LOG_ERR, "nfssvc: unable to create listening socket: "
> - "errno %d (%s)\n", errno, strerror(errno));
> + xlog(L_ERROR, "unable to create listening socket: "
> + "errno %d (%m)", errno);
> exit(1);
> }
> }
> if (udpfd >= 0) {
> snprintf(buf, BUFSIZ,"%d\n", udpfd);
> if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> - syslog(LOG_ERR,
> - "nfssvc: writing fds to kernel failed: errno %d (%s)",
> - errno, strerror(errno));
> + xlog(L_ERROR,
> + "writing fds to kernel failed: errno %d (%m)",
> + errno);
> }
> close(fd);
> fd = -1;
> @@ -106,9 +105,9 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> char *haddr)
> fd = open(NFSD_PORTS_FILE, O_WRONLY);
> snprintf(buf, BUFSIZ,"%d\n", tcpfd);
> if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> - syslog(LOG_ERR,
> - "nfssvc: writing fds to kernel failed: errno %d (%s)",
> - errno, strerror(errno));
> + xlog(L_ERROR,
> + "writing fds to kernel failed: errno %d (%m)",
> + errno);
> }
> }
> close(fd);
> @@ -139,10 +138,9 @@ nfssvc_versbits(unsigned int ctlbits, int
> minorvers4)
> minorvers4 > 0 ? '+' : '-',
> n);
> snprintf(ptr+off, BUFSIZ - off, "\n");
> - if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> - syslog(LOG_ERR, "nfssvc: Setting version failed: errno %d (%s)",
> - errno, strerror(errno));
> - }
> + if (write(fd, buf, strlen(buf)) != strlen(buf))
> + xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
> +
> close(fd);
>
> return;
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index c7bd003..6dfea67 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -18,13 +18,13 @@
> #include <string.h>
> #include <errno.h>
> #include <getopt.h>
> -#include <syslog.h>
> #include <netdb.h>
> #include <sys/socket.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
>
> #include "nfslib.h"
> +#include "xlog.h"
>
> static void usage(const char *);
>
> @@ -37,6 +37,7 @@ static struct option longopts[] =
> { "no-udp", 0, 0, 'U' },
> { "port", 1, 0, 'P' },
> { "port", 1, 0, 'p' },
> + { "debug", 0, 0, 'd' },
> { NULL, 0, 0, 0 }
> };
> unsigned int protobits = NFSCTL_ALLBITS;
> @@ -50,7 +51,7 @@ main(int argc, char **argv)
> int count = 1, c, error, port, fd, found_one;
> struct servent *ent;
> struct hostent *hp;
> - char *p;
> + char *p, *progname;
>
> ent = getservbyname ("nfs", "udp");
> if (ent != NULL)
> @@ -58,8 +59,22 @@ main(int argc, char **argv)
> else
> port = 2049;
>
> - while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", longopts,
> NULL)) != EOF) {
> + progname = strdup(basename(argv[0]));
> + if (!progname) {
> + fprintf(stderr, "%s: unable to allocate memory.\n", argv[0]);
> + exit(1);
> + }
> +
> + xlog_syslog(1);
> + xlog_stderr(0);
> +
> + while ((c = getopt_long(argc, argv, "dH:hN:p:P:TU", longopts,
> NULL)) != EOF) {
> switch(c) {
> + case 'd':
> + xlog_config(D_ALL, 1);
> + xlog_syslog(0);
> + xlog_stderr(1);
> + break;
> case 'H':
> if (inet_addr(optarg) != INADDR_NONE) {
> haddr = strdup(optarg);
> @@ -67,8 +82,8 @@ main(int argc, char **argv)
> haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
> } else {
> fprintf(stderr, "%s: Unknown hostname: %s\n",
> - argv[0], optarg);
> - usage(argv [0]);
> + progname, optarg);
> + usage(progname);
> }
> break;
> case 'P': /* XXX for nfs-server compatibility */
> @@ -76,8 +91,8 @@ main(int argc, char **argv)
> port = atoi(optarg);
> if (port <= 0 || port > 65535) {
> fprintf(stderr, "%s: bad port number: %s\n",
> - argv[0], optarg);
> - usage(argv [0]);
> + progname, optarg);
> + usage(progname);
> }
> break;
> case 'N':
> @@ -105,14 +120,17 @@ main(int argc, char **argv)
> default:
> fprintf(stderr, "Invalid argument: '%c'\n", c);
> case 'h':
> - usage(argv[0]);
> + usage(progname);
> }
> }
> +
> + xlog_open(progname);
> +
> /*
> * Do some sanity checking, if the ctlbits are set
> */
> if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
> - fprintf(stderr, "invalid protocol specified\n");
> + xlog(L_ERROR, "invalid protocol specified");
> exit(1);
> }
> found_one = 0;
> @@ -121,12 +139,12 @@ main(int argc, char **argv)
> found_one = 1;
> }
> if (!found_one) {
> - fprintf(stderr, "no version specified\n");
> + xlog(L_ERROR, "no version specified");
> exit(1);
> }
>
> if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
> - fprintf(stderr, "version 4 requires the TCP protocol\n");
> + xlog(L_ERROR, "version 4 requires the TCP protocol");
> exit(1);
> }
> if (haddr == NULL) {
> @@ -135,17 +153,15 @@ main(int argc, char **argv)
> }
>
> if (chdir(NFS_STATEDIR)) {
> - fprintf(stderr, "%s: chdir(%s) failed: %s\n",
> - argv [0], NFS_STATEDIR, strerror(errno));
> + xlog(L_ERROR, "chdir(%s) failed: %m", NFS_STATEDIR);
> exit(1);
> }
>
> if (optind < argc) {
> if ((count = atoi(argv[optind])) < 0) {
> /* insane # of servers */
> - fprintf(stderr,
> - "%s: invalid server count (%d), using 1\n",
> - argv[0], count);
> + xlog(L_ERROR, "invalid server count (%d), using 1",
> + count);
> count = 1;
> }
> }
> @@ -155,21 +171,20 @@ main(int argc, char **argv)
> everything before spawning kernel threads. --Chip */
> fd = open("/dev/null", O_RDWR);
> if (fd == -1)
> - perror("/dev/null");
> + xlog(L_ERROR, "Unable to open /dev/null: %m");
> else {
> (void) dup2(fd, 0);
> (void) dup2(fd, 1);
> (void) dup2(fd, 2);
> }
> + xlog_syslog(1);
> + xlog_stderr(0);
> closeall(3);
>
> - openlog("nfsd", LOG_PID, LOG_DAEMON);
> - if ((error = nfssvc(port, count, versbits, minorvers4, protobits,
> haddr)) < 0) {
> - int e = errno;
> - syslog(LOG_ERR, "nfssvc: %s", strerror(e));
> - closelog();
> - }
> + if ((error = nfssvc(port, count, versbits, minorvers4, protobits,
> haddr)) < 0)
> + xlog(L_ERROR, "nfssvc (%m)");
Nit: Maybe get rid of the "nfssvc" here as well?
>
>
> + free(progname);
You go to the trouble of freeing progname here, but you exit in some
cases in the code above without freeing. I usually copy argv[0] into
a static buffer to avoid all that strdup(3) bother.
> return (error != 0);
> }
>
> @@ -177,7 +192,7 @@ static void
> usage(const char *prog)
> {
> fprintf(stderr, "Usage:\n"
> - "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version
> version ] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
> + "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version
> version ] [-T|--no-tcp] [-U|--no-udp] [-d|--debug] nrservs\n",
> prog);
> exit(2);
> }
> --
> 1.6.2.2
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 03/10] nfs-utils: convert rpc.nfsd to use xlog()
2009-06-03 20:01 ` Chuck Lever
@ 2009-06-03 20:22 ` Jeff Layton
[not found] ` <20090603162214.2aeed744-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 20:22 UTC (permalink / raw)
To: Chuck Lever; +Cc: linux-nfs, steved
On Wed, 3 Jun 2009 16:01:24 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
>
> > ...and add a --debug option to make nfsd log messages to stderr.
> >
> > rpc.nfsd is usually run out of init scripts in which case logging to
> > syslog makes more sense. Make that the default and add a switch that
> > makes log messages go to stderr. Eventually however nfsd has to close
> > stderr, at which point the program switches to logging to syslog
> > unconditionally.
> >
> > For now, stderr gets closed rather early, so --debug isn't terribly
> > helpful. Later patches will make rpc.nfsd delay closing of stderr
> > longer
> > which should make it more useful.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > support/nfs/nfssvc.c | 46 +++++++++++++++++-------------------
> > utils/nfsd/nfsd.c | 63 ++++++++++++++++++++++++++++++
> > +-------------------
> > 2 files changed, 61 insertions(+), 48 deletions(-)
> >
> > diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> > index 33c15a7..3e6bd31 100644
> > --- a/support/nfs/nfssvc.c
> > +++ b/support/nfs/nfssvc.c
> > @@ -16,10 +16,9 @@
> > #include <unistd.h>
> > #include <fcntl.h>
> > #include <errno.h>
> > -#include <syslog.h>
> > -
> >
> > #include "nfslib.h"
> > +#include "xlog.h"
> >
> > #define NFSD_PORTS_FILE "/proc/fs/nfsd/portlist"
> > #define NFSD_VERS_FILE "/proc/fs/nfsd/versions"
> > @@ -57,13 +56,13 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> > char *haddr)
> > if (NFSCTL_UDPISSET(ctlbits)) {
> > udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> > if (udpfd < 0) {
> > - syslog(LOG_ERR, "nfssvc: unable to create UPD socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to create UDP socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (bind(udpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> > - syslog(LOG_ERR, "nfssvc: unable to bind UPD socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to bind UDP socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > }
> > @@ -71,32 +70,32 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> > char *haddr)
> > if (NFSCTL_TCPISSET(ctlbits)) {
> > tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> > if (tcpfd < 0) {
> > - syslog(LOG_ERR, "nfssvc: unable to createt tcp socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to createt tcp socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) <
> > 0) {
> > - syslog(LOG_ERR, "nfssvc: unable to set SO_REUSEADDR: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to set SO_REUSEADDR: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (bind(tcpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> > - syslog(LOG_ERR, "nfssvc: unable to bind TCP socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to bind TCP socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (listen(tcpfd, 64) < 0){
> > - syslog(LOG_ERR, "nfssvc: unable to create listening socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to create listening socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > }
> > if (udpfd >= 0) {
> > snprintf(buf, BUFSIZ,"%d\n", udpfd);
> > if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - syslog(LOG_ERR,
> > - "nfssvc: writing fds to kernel failed: errno %d (%s)",
> > - errno, strerror(errno));
> > + xlog(L_ERROR,
> > + "writing fds to kernel failed: errno %d (%m)",
> > + errno);
> > }
> > close(fd);
> > fd = -1;
> > @@ -106,9 +105,9 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> > char *haddr)
> > fd = open(NFSD_PORTS_FILE, O_WRONLY);
> > snprintf(buf, BUFSIZ,"%d\n", tcpfd);
> > if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - syslog(LOG_ERR,
> > - "nfssvc: writing fds to kernel failed: errno %d (%s)",
> > - errno, strerror(errno));
> > + xlog(L_ERROR,
> > + "writing fds to kernel failed: errno %d (%m)",
> > + errno);
> > }
> > }
> > close(fd);
> > @@ -139,10 +138,9 @@ nfssvc_versbits(unsigned int ctlbits, int
> > minorvers4)
> > minorvers4 > 0 ? '+' : '-',
> > n);
> > snprintf(ptr+off, BUFSIZ - off, "\n");
> > - if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - syslog(LOG_ERR, "nfssvc: Setting version failed: errno %d (%s)",
> > - errno, strerror(errno));
> > - }
> > + if (write(fd, buf, strlen(buf)) != strlen(buf))
> > + xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
> > +
> > close(fd);
> >
> > return;
> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> > index c7bd003..6dfea67 100644
> > --- a/utils/nfsd/nfsd.c
> > +++ b/utils/nfsd/nfsd.c
> > @@ -18,13 +18,13 @@
> > #include <string.h>
> > #include <errno.h>
> > #include <getopt.h>
> > -#include <syslog.h>
> > #include <netdb.h>
> > #include <sys/socket.h>
> > #include <netinet/in.h>
> > #include <arpa/inet.h>
> >
> > #include "nfslib.h"
> > +#include "xlog.h"
> >
> > static void usage(const char *);
> >
> > @@ -37,6 +37,7 @@ static struct option longopts[] =
> > { "no-udp", 0, 0, 'U' },
> > { "port", 1, 0, 'P' },
> > { "port", 1, 0, 'p' },
> > + { "debug", 0, 0, 'd' },
> > { NULL, 0, 0, 0 }
> > };
> > unsigned int protobits = NFSCTL_ALLBITS;
> > @@ -50,7 +51,7 @@ main(int argc, char **argv)
> > int count = 1, c, error, port, fd, found_one;
> > struct servent *ent;
> > struct hostent *hp;
> > - char *p;
> > + char *p, *progname;
> >
> > ent = getservbyname ("nfs", "udp");
> > if (ent != NULL)
> > @@ -58,8 +59,22 @@ main(int argc, char **argv)
> > else
> > port = 2049;
> >
> > - while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", longopts,
> > NULL)) != EOF) {
> > + progname = strdup(basename(argv[0]));
> > + if (!progname) {
> > + fprintf(stderr, "%s: unable to allocate memory.\n", argv[0]);
> > + exit(1);
> > + }
> > +
> > + xlog_syslog(1);
> > + xlog_stderr(0);
> > +
> > + while ((c = getopt_long(argc, argv, "dH:hN:p:P:TU", longopts,
> > NULL)) != EOF) {
> > switch(c) {
> > + case 'd':
> > + xlog_config(D_ALL, 1);
> > + xlog_syslog(0);
> > + xlog_stderr(1);
> > + break;
> > case 'H':
> > if (inet_addr(optarg) != INADDR_NONE) {
> > haddr = strdup(optarg);
> > @@ -67,8 +82,8 @@ main(int argc, char **argv)
> > haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
> > } else {
> > fprintf(stderr, "%s: Unknown hostname: %s\n",
> > - argv[0], optarg);
> > - usage(argv [0]);
> > + progname, optarg);
> > + usage(progname);
> > }
> > break;
> > case 'P': /* XXX for nfs-server compatibility */
> > @@ -76,8 +91,8 @@ main(int argc, char **argv)
> > port = atoi(optarg);
> > if (port <= 0 || port > 65535) {
> > fprintf(stderr, "%s: bad port number: %s\n",
> > - argv[0], optarg);
> > - usage(argv [0]);
> > + progname, optarg);
> > + usage(progname);
> > }
> > break;
> > case 'N':
> > @@ -105,14 +120,17 @@ main(int argc, char **argv)
> > default:
> > fprintf(stderr, "Invalid argument: '%c'\n", c);
> > case 'h':
> > - usage(argv[0]);
> > + usage(progname);
> > }
> > }
> > +
> > + xlog_open(progname);
> > +
> > /*
> > * Do some sanity checking, if the ctlbits are set
> > */
> > if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
> > - fprintf(stderr, "invalid protocol specified\n");
> > + xlog(L_ERROR, "invalid protocol specified");
> > exit(1);
> > }
> > found_one = 0;
> > @@ -121,12 +139,12 @@ main(int argc, char **argv)
> > found_one = 1;
> > }
> > if (!found_one) {
> > - fprintf(stderr, "no version specified\n");
> > + xlog(L_ERROR, "no version specified");
> > exit(1);
> > }
> >
> > if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
> > - fprintf(stderr, "version 4 requires the TCP protocol\n");
> > + xlog(L_ERROR, "version 4 requires the TCP protocol");
> > exit(1);
> > }
> > if (haddr == NULL) {
> > @@ -135,17 +153,15 @@ main(int argc, char **argv)
> > }
> >
> > if (chdir(NFS_STATEDIR)) {
> > - fprintf(stderr, "%s: chdir(%s) failed: %s\n",
> > - argv [0], NFS_STATEDIR, strerror(errno));
> > + xlog(L_ERROR, "chdir(%s) failed: %m", NFS_STATEDIR);
> > exit(1);
> > }
> >
> > if (optind < argc) {
> > if ((count = atoi(argv[optind])) < 0) {
> > /* insane # of servers */
> > - fprintf(stderr,
> > - "%s: invalid server count (%d), using 1\n",
> > - argv[0], count);
> > + xlog(L_ERROR, "invalid server count (%d), using 1",
> > + count);
> > count = 1;
> > }
> > }
> > @@ -155,21 +171,20 @@ main(int argc, char **argv)
> > everything before spawning kernel threads. --Chip */
> > fd = open("/dev/null", O_RDWR);
> > if (fd == -1)
> > - perror("/dev/null");
> > + xlog(L_ERROR, "Unable to open /dev/null: %m");
> > else {
> > (void) dup2(fd, 0);
> > (void) dup2(fd, 1);
> > (void) dup2(fd, 2);
> > }
> > + xlog_syslog(1);
> > + xlog_stderr(0);
> > closeall(3);
> >
> > - openlog("nfsd", LOG_PID, LOG_DAEMON);
> > - if ((error = nfssvc(port, count, versbits, minorvers4, protobits,
> > haddr)) < 0) {
> > - int e = errno;
> > - syslog(LOG_ERR, "nfssvc: %s", strerror(e));
> > - closelog();
> > - }
> > + if ((error = nfssvc(port, count, versbits, minorvers4, protobits,
> > haddr)) < 0)
> > + xlog(L_ERROR, "nfssvc (%m)");
>
> Nit: Maybe get rid of the "nfssvc" here as well?
>
Well, the "nfssvc" here tells us that the nfssvc() call returned an
error. Later patches will clean that up when I break up the nfssvc()
interface. That said though, there's a bug there. %m isn't appropriate
since errno isn't being set. Whoops!
> >
> >
> > + free(progname);
>
> You go to the trouble of freeing progname here, but you exit in some
> cases in the code above without freeing. I usually copy argv[0] into
> a static buffer to avoid all that strdup(3) bother.
>
Six of one, half dozen of another. I try to be good about freeing
memory that's been allocated in case this code gets reorganized in the
future. The existing code is pretty bad about it, but it doesn't
matter much since this program isn't long running.
As far as allocating a static buffer for it, the problem there is that
you don't know how big a buffer you'll need. Using strdup means less
memory is being used.
> > return (error != 0);
> > }
> >
> > @@ -177,7 +192,7 @@ static void
> > usage(const char *prog)
> > {
> > fprintf(stderr, "Usage:\n"
> > - "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version
> > version ] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
> > + "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version
> > version ] [-T|--no-tcp] [-U|--no-udp] [-d|--debug] nrservs\n",
> > prog);
> > exit(2);
> > }
> > --
> > 1.6.2.2
> >
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
[parent not found: <7968D2B9-3E31-4E0D-9D0B-309D757A0014@oracle.com>]
* Re: [PATCH 03/10] nfs-utils: convert rpc.nfsd to use xlog()
[not found] ` <7968D2B9-3E31-4E0D-9D0B-309D757A0014@oracle.com>
@ 2009-06-04 20:25 ` Jeff Layton
[not found] ` <20090604162534.15db803a-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Jeff Layton @ 2009-06-04 20:25 UTC (permalink / raw)
To: Chuck Lever; +Cc: linux-nfs, steved
On Thu, 4 Jun 2009 15:38:32 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
>
> > ...and add a --debug option to make nfsd log messages to stderr.
> >
> > rpc.nfsd is usually run out of init scripts in which case logging to
> > syslog makes more sense. Make that the default and add a switch that
> > makes log messages go to stderr. Eventually however nfsd has to close
> > stderr, at which point the program switches to logging to syslog
> > unconditionally.
> >
> > For now, stderr gets closed rather early, so --debug isn't terribly
> > helpful. Later patches will make rpc.nfsd delay closing of stderr
> > longer
> > which should make it more useful.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > support/nfs/nfssvc.c | 46 +++++++++++++++++-------------------
> > utils/nfsd/nfsd.c | 63 ++++++++++++++++++++++++++++++
> > +-------------------
> > 2 files changed, 61 insertions(+), 48 deletions(-)
> >
> > diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> > index 33c15a7..3e6bd31 100644
> > --- a/support/nfs/nfssvc.c
> > +++ b/support/nfs/nfssvc.c
>
> One wonders why this code is not under utils/nfsd/ . rpc.nfsd seems
> to be its only caller.
>
Yep...not sure either. I could move it, I suppose.
> > @@ -16,10 +16,9 @@
> > #include <unistd.h>
> > #include <fcntl.h>
> > #include <errno.h>
> > -#include <syslog.h>
> > -
> >
> > #include "nfslib.h"
> > +#include "xlog.h"
> >
> > #define NFSD_PORTS_FILE "/proc/fs/nfsd/portlist"
> > #define NFSD_VERS_FILE "/proc/fs/nfsd/versions"
> > @@ -57,13 +56,13 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> > char *haddr)
> > if (NFSCTL_UDPISSET(ctlbits)) {
> > udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> > if (udpfd < 0) {
> > - syslog(LOG_ERR, "nfssvc: unable to create UPD socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to create UDP socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (bind(udpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> > - syslog(LOG_ERR, "nfssvc: unable to bind UPD socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to bind UDP socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > }
> > @@ -71,32 +70,32 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> > char *haddr)
> > if (NFSCTL_TCPISSET(ctlbits)) {
> > tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> > if (tcpfd < 0) {
> > - syslog(LOG_ERR, "nfssvc: unable to createt tcp socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to createt tcp socket: "
>
> You could correct the misspelling here (createt), while you're at it.
>
I think I fix that in a later patch, but I'll fix it here too.
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) <
> > 0) {
> > - syslog(LOG_ERR, "nfssvc: unable to set SO_REUSEADDR: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to set SO_REUSEADDR: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (bind(tcpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> > - syslog(LOG_ERR, "nfssvc: unable to bind TCP socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to bind TCP socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > if (listen(tcpfd, 64) < 0){
> > - syslog(LOG_ERR, "nfssvc: unable to create listening socket: "
> > - "errno %d (%s)\n", errno, strerror(errno));
> > + xlog(L_ERROR, "unable to create listening socket: "
> > + "errno %d (%m)", errno);
> > exit(1);
> > }
> > }
> > if (udpfd >= 0) {
> > snprintf(buf, BUFSIZ,"%d\n", udpfd);
> > if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - syslog(LOG_ERR,
> > - "nfssvc: writing fds to kernel failed: errno %d (%s)",
> > - errno, strerror(errno));
> > + xlog(L_ERROR,
> > + "writing fds to kernel failed: errno %d (%m)",
> > + errno);
> > }
> > close(fd);
> > fd = -1;
> > @@ -106,9 +105,9 @@ nfssvc_setfds(int port, unsigned int ctlbits,
> > char *haddr)
> > fd = open(NFSD_PORTS_FILE, O_WRONLY);
> > snprintf(buf, BUFSIZ,"%d\n", tcpfd);
> > if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - syslog(LOG_ERR,
> > - "nfssvc: writing fds to kernel failed: errno %d (%s)",
> > - errno, strerror(errno));
> > + xlog(L_ERROR,
> > + "writing fds to kernel failed: errno %d (%m)",
> > + errno);
> > }
> > }
> > close(fd);
> > @@ -139,10 +138,9 @@ nfssvc_versbits(unsigned int ctlbits, int
> > minorvers4)
> > minorvers4 > 0 ? '+' : '-',
> > n);
> > snprintf(ptr+off, BUFSIZ - off, "\n");
> > - if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - syslog(LOG_ERR, "nfssvc: Setting version failed: errno %d (%s)",
> > - errno, strerror(errno));
> > - }
> > + if (write(fd, buf, strlen(buf)) != strlen(buf))
> > + xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
> > +
> > close(fd);
> >
> > return;
> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> > index c7bd003..6dfea67 100644
> > --- a/utils/nfsd/nfsd.c
> > +++ b/utils/nfsd/nfsd.c
> > @@ -18,13 +18,13 @@
> > #include <string.h>
> > #include <errno.h>
> > #include <getopt.h>
> > -#include <syslog.h>
> > #include <netdb.h>
> > #include <sys/socket.h>
> > #include <netinet/in.h>
> > #include <arpa/inet.h>
> >
> > #include "nfslib.h"
> > +#include "xlog.h"
> >
> > static void usage(const char *);
> >
> > @@ -37,6 +37,7 @@ static struct option longopts[] =
> > { "no-udp", 0, 0, 'U' },
> > { "port", 1, 0, 'P' },
> > { "port", 1, 0, 'p' },
> > + { "debug", 0, 0, 'd' },
> > { NULL, 0, 0, 0 }
> > };
> > unsigned int protobits = NFSCTL_ALLBITS;
> > @@ -50,7 +51,7 @@ main(int argc, char **argv)
> > int count = 1, c, error, port, fd, found_one;
> > struct servent *ent;
> > struct hostent *hp;
> > - char *p;
> > + char *p, *progname;
> >
> > ent = getservbyname ("nfs", "udp");
> > if (ent != NULL)
> > @@ -58,8 +59,22 @@ main(int argc, char **argv)
> > else
> > port = 2049;
> >
> > - while ((c = getopt_long(argc, argv, "H:hN:p:P:TU", longopts,
> > NULL)) != EOF) {
> > + progname = strdup(basename(argv[0]));
> > + if (!progname) {
> > + fprintf(stderr, "%s: unable to allocate memory.\n", argv[0]);
> > + exit(1);
> > + }
> > +
> > + xlog_syslog(1);
> > + xlog_stderr(0);
> > +
> > + while ((c = getopt_long(argc, argv, "dH:hN:p:P:TU", longopts,
> > NULL)) != EOF) {
> > switch(c) {
> > + case 'd':
> > + xlog_config(D_ALL, 1);
> > + xlog_syslog(0);
> > + xlog_stderr(1);
> > + break;
> > case 'H':
> > if (inet_addr(optarg) != INADDR_NONE) {
> > haddr = strdup(optarg);
> > @@ -67,8 +82,8 @@ main(int argc, char **argv)
> > haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
> > } else {
> > fprintf(stderr, "%s: Unknown hostname: %s\n",
> > - argv[0], optarg);
> > - usage(argv [0]);
> > + progname, optarg);
> > + usage(progname);
> > }
> > break;
> > case 'P': /* XXX for nfs-server compatibility */
> > @@ -76,8 +91,8 @@ main(int argc, char **argv)
> > port = atoi(optarg);
> > if (port <= 0 || port > 65535) {
> > fprintf(stderr, "%s: bad port number: %s\n",
> > - argv[0], optarg);
> > - usage(argv [0]);
> > + progname, optarg);
> > + usage(progname);
> > }
> > break;
> > case 'N':
> > @@ -105,14 +120,17 @@ main(int argc, char **argv)
> > default:
> > fprintf(stderr, "Invalid argument: '%c'\n", c);
> > case 'h':
> > - usage(argv[0]);
> > + usage(progname);
> > }
> > }
> > +
> > + xlog_open(progname);
> > +
> > /*
> > * Do some sanity checking, if the ctlbits are set
> > */
> > if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
> > - fprintf(stderr, "invalid protocol specified\n");
> > + xlog(L_ERROR, "invalid protocol specified");
>
> Nit: This code looks like additional command line option parsing, so
> fprintf(stderr) might be entirely adequate. Any reason to promote
> these to xlog() ?
>
I think when I asked you about this before you said that you thought
that nfsd should always log to syslog whenever possible. Once the
command line parsing is done, it's safe to switch to syslog...
I'm starting to wonder though whether that's the ideal thing. Given
that rpc.nfsd isn't really a daemon per-se, maybe it would be better to
make it send output to stderr instead? The parent processes for init
scripts usually take care of logging stderr to syslog. If someone is
running interactively then it probably makes sense to give them
feedback on stderr rather than requiring them to run with --debug...
> > exit(1);
> > }
> > found_one = 0;
> > @@ -121,12 +139,12 @@ main(int argc, char **argv)
> > found_one = 1;
> > }
> > if (!found_one) {
> > - fprintf(stderr, "no version specified\n");
> > + xlog(L_ERROR, "no version specified");
> > exit(1);
> > }
> >
> > if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
> > - fprintf(stderr, "version 4 requires the TCP protocol\n");
> > + xlog(L_ERROR, "version 4 requires the TCP protocol");
> > exit(1);
> > }
> > if (haddr == NULL) {
> > @@ -135,17 +153,15 @@ main(int argc, char **argv)
> > }
> >
> > if (chdir(NFS_STATEDIR)) {
> > - fprintf(stderr, "%s: chdir(%s) failed: %s\n",
> > - argv [0], NFS_STATEDIR, strerror(errno));
> > + xlog(L_ERROR, "chdir(%s) failed: %m", NFS_STATEDIR);
> > exit(1);
> > }
> >
> > if (optind < argc) {
> > if ((count = atoi(argv[optind])) < 0) {
> > /* insane # of servers */
> > - fprintf(stderr,
> > - "%s: invalid server count (%d), using 1\n",
> > - argv[0], count);
> > + xlog(L_ERROR, "invalid server count (%d), using 1",
> > + count);
> > count = 1;
> > }
> > }
> > @@ -155,21 +171,20 @@ main(int argc, char **argv)
> > everything before spawning kernel threads. --Chip */
> > fd = open("/dev/null", O_RDWR);
> > if (fd == -1)
> > - perror("/dev/null");
> > + xlog(L_ERROR, "Unable to open /dev/null: %m");
> > else {
> > (void) dup2(fd, 0);
> > (void) dup2(fd, 1);
> > (void) dup2(fd, 2);
> > }
> > + xlog_syslog(1);
> > + xlog_stderr(0);
> > closeall(3);
> >
> > - openlog("nfsd", LOG_PID, LOG_DAEMON);
> > - if ((error = nfssvc(port, count, versbits, minorvers4, protobits,
> > haddr)) < 0) {
> > - int e = errno;
> > - syslog(LOG_ERR, "nfssvc: %s", strerror(e));
> > - closelog();
> > - }
> > + if ((error = nfssvc(port, count, versbits, minorvers4, protobits,
> > haddr)) < 0)
> > + xlog(L_ERROR, "nfssvc (%m)");
> >
> > + free(progname);
> > return (error != 0);
> > }
> >
> > @@ -177,7 +192,7 @@ static void
> > usage(const char *prog)
> > {
> > fprintf(stderr, "Usage:\n"
> > - "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version
> > version ] [-T|--no-tcp] [-U|--no-udp] nrservs\n",
> > + "%s [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version
> > version ] [-T|--no-tcp] [-U|--no-udp] [-d|--debug] nrservs\n",
> > prog);
> > exit(2);
> > }
> > --
> > 1.6.2.2
> >
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 04/10] nfs-utils: clean up NFSCTL_* macros for handling protocol bits
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
` (2 preceding siblings ...)
2009-06-03 19:52 ` [PATCH 03/10] nfs-utils: convert rpc.nfsd to use xlog() Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-03 19:52 ` [PATCH 05/10] nfs-utils: move check for active knfsd to helper function Jeff Layton
` (5 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
They are a little hard to follow currently. Clean them up and add new
macros that can set these bits in addition to the ones that unset them.
Also add a new macro that reports when any valid protocol bit is set.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
support/include/nfs/nfs.h | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/support/include/nfs/nfs.h b/support/include/nfs/nfs.h
index 00b0028..a64eb0a 100644
--- a/support/include/nfs/nfs.h
+++ b/support/include/nfs/nfs.h
@@ -42,14 +42,21 @@ struct nfs_fh_old {
#define NFSCTL_GETFD 7 /* get an fh by path (used by mountd) */
#define NFSCTL_GETFS 8 /* get an fh by path with max size (used by mountd) */
+#define NFSCTL_UDPBIT (1 << (17 - 1))
+#define NFSCTL_TCPBIT (1 << (18 - 1))
+
#define NFSCTL_VERUNSET(_cltbits, _v) ((_cltbits) &= ~(1 << ((_v) - 1)))
-#define NFSCTL_UDPUNSET(_cltbits) ((_cltbits) &= ~(1 << (17 - 1)))
-#define NFSCTL_TCPUNSET(_cltbits) ((_cltbits) &= ~(1 << (18 - 1)))
+#define NFSCTL_UDPUNSET(_cltbits) ((_cltbits) &= ~NFSCTL_UDPBIT)
+#define NFSCTL_TCPUNSET(_cltbits) ((_cltbits) &= ~NFSCTL_TCPBIT)
#define NFSCTL_VERISSET(_cltbits, _v) ((_cltbits) & (1 << ((_v) - 1)))
-#define NFSCTL_UDPISSET(_cltbits) ((_cltbits) & (1 << (17 - 1)))
-#define NFSCTL_TCPISSET(_cltbits) ((_cltbits) & (1 << (18 - 1)))
+#define NFSCTL_UDPISSET(_cltbits) ((_cltbits) & NFSCTL_UDPBIT)
+#define NFSCTL_TCPISSET(_cltbits) ((_cltbits) & NFSCTL_TCPBIT)
+
+#define NFSCTL_UDPSET(_cltbits) ((_cltbits) |= NFSCTL_UDPBIT)
+#define NFSCTL_TCPSET(_cltbits) ((_cltbits) |= NFSCTL_TCPBIT)
+#define NFSCTL_ANYPROTO(_cltbits) ((_cltbits) & (NFSCTL_UDPBIT | NFSCTL_TCPBIT))
#define NFSCTL_ALLBITS (~0)
/* SVC */
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 05/10] nfs-utils: move check for active knfsd to helper function
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
` (3 preceding siblings ...)
2009-06-03 19:52 ` [PATCH 04/10] nfs-utils: clean up NFSCTL_* macros for handling protocol bits Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-04 20:00 ` Chuck Lever
2009-06-03 19:52 ` [PATCH 06/10] nfs-utils: convert nfssvc_setfds to use getaddrinfo Jeff Layton
` (4 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
nfssvc_setfds checks to see if knfsd is already running. Move this
check to a helper function. Eventually the nfsd code will call this
directly.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
support/nfs/nfssvc.c | 44 +++++++++++++++++++++++++++++++-------------
1 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 3e6bd31..8b15c4d 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -24,28 +24,46 @@
#define NFSD_VERS_FILE "/proc/fs/nfsd/versions"
#define NFSD_THREAD_FILE "/proc/fs/nfsd/threads"
-static void
-nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
+/*
+ * Are there already sockets configured? If not, then it is safe to try to
+ * open some and pass them through.
+ *
+ * Note: If the user explicitly asked for 'udp', then we should probably check
+ * if that is open, and should open it if not. However we don't yet. All
+ * sockets have to be opened when the first daemon is started.
+ */
+int
+nfssvc_inuse(void)
{
- int fd, n, on=1;
+ int fd, n;
char buf[BUFSIZ];
- int udpfd = -1, tcpfd = -1;
- struct sockaddr_in sin;
fd = open(NFSD_PORTS_FILE, O_RDONLY);
+
+ /* problem opening file, assume that nothing is configured */
if (fd < 0)
- return;
+ return 0;
+
n = read(fd, buf, BUFSIZ);
close(fd);
+
if (n != 0)
+ return 1;
+
+ return 0;
+}
+
+static void
+nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
+{
+ int fd, n, on=1;
+ char buf[BUFSIZ];
+ int udpfd = -1, tcpfd = -1;
+ struct sockaddr_in sin;
+
+ if (nfssvc_inuse())
return;
- /* there are no ports currently open, so it is safe to
- * try to open some and pass them through.
- * Note: If the user explicitly asked for 'udp', then
- * we should probably check if that is open, and should
- * open it if not. However we don't yet. All sockets
- * have to be opened when the first daemon is started.
- */
+
fd = open(NFSD_PORTS_FILE, O_WRONLY);
if (fd < 0)
return;
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 05/10] nfs-utils: move check for active knfsd to helper function
2009-06-03 19:52 ` [PATCH 05/10] nfs-utils: move check for active knfsd to helper function Jeff Layton
@ 2009-06-04 20:00 ` Chuck Lever
2009-06-04 20:29 ` Jeff Layton
0 siblings, 1 reply; 26+ messages in thread
From: Chuck Lever @ 2009-06-04 20:00 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs, steved
On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
> nfssvc_setfds checks to see if knfsd is already running. Move this
> check to a helper function. Eventually the nfsd code will call this
> directly.
Some of this isn't your fault, but this code could use a little extra
clean up as you split it up, IMO.
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> support/nfs/nfssvc.c | 44 ++++++++++++++++++++++++++++++
> +-------------
> 1 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> index 3e6bd31..8b15c4d 100644
> --- a/support/nfs/nfssvc.c
> +++ b/support/nfs/nfssvc.c
> @@ -24,28 +24,46 @@
> #define NFSD_VERS_FILE "/proc/fs/nfsd/versions"
> #define NFSD_THREAD_FILE "/proc/fs/nfsd/threads"
>
> -static void
> -nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
> +/*
> + * Are there already sockets configured? If not, then it is safe to
> try to
> + * open some and pass them through.
> + *
> + * Note: If the user explicitly asked for 'udp', then we should
> probably check
> + * if that is open, and should open it if not. However we don't
> yet. All
> + * sockets have to be opened when the first daemon is started.
> + */
> +int
> +nfssvc_inuse(void)
> {
> - int fd, n, on=1;
> + int fd, n;
read(2) returns a ssize_t result, not an int.
> char buf[BUFSIZ];
Eesh. BUFSIZ looks like it's two pages on my system, so adding this
helper makes the stack requirements in this path over 16KB.
Might be better if we used something a little more stack friendly
here. The kernel bounds the size of the read(2) result to
SIMPLE_TRANSACTION_LIMIT, which is less than a page, for example.
Making it a static would keep "buf" off the stack, too; or checking
how much buffer we really need (like only a few bytes, for this
particular check) might be better.
> - int udpfd = -1, tcpfd = -1;
> - struct sockaddr_in sin;
>
> fd = open(NFSD_PORTS_FILE, O_RDONLY);
> +
> + /* problem opening file, assume that nothing is configured */
> if (fd < 0)
> - return;
> + return 0;
> +
> n = read(fd, buf, BUFSIZ);
Nit: Using "sizeof(buf)" might be slightly more maintainable than
"BUFSIZ".
> close(fd);
> +
> if (n != 0)
If read(2) returns -1, we return 1 here. How about "return (n > 0);" ?
> + return 1;
> +
> + return 0;
> +}
> +
> +static void
> +nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
> +{
> + int fd, n, on=1;
Another compiler warning issue: "n" is probably now unused in
nfssvc_setfds().
>
> + char buf[BUFSIZ];
> + int udpfd = -1, tcpfd = -1;
> + struct sockaddr_in sin;
> +
> + if (nfssvc_inuse())
> return;
> - /* there are no ports currently open, so it is safe to
> - * try to open some and pass them through.
> - * Note: If the user explicitly asked for 'udp', then
> - * we should probably check if that is open, and should
> - * open it if not. However we don't yet. All sockets
> - * have to be opened when the first daemon is started.
> - */
> +
> fd = open(NFSD_PORTS_FILE, O_WRONLY);
> if (fd < 0)
> return;
> --
> 1.6.2.2
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 05/10] nfs-utils: move check for active knfsd to helper function
2009-06-04 20:00 ` Chuck Lever
@ 2009-06-04 20:29 ` Jeff Layton
0 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-04 20:29 UTC (permalink / raw)
To: Chuck Lever; +Cc: linux-nfs, steved
On Thu, 4 Jun 2009 16:00:43 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
>
> > nfssvc_setfds checks to see if knfsd is already running. Move this
> > check to a helper function. Eventually the nfsd code will call this
> > directly.
>
> Some of this isn't your fault, but this code could use a little extra
> clean up as you split it up, IMO.
>
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > support/nfs/nfssvc.c | 44 ++++++++++++++++++++++++++++++
> > +-------------
> > 1 files changed, 31 insertions(+), 13 deletions(-)
> >
> > diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> > index 3e6bd31..8b15c4d 100644
> > --- a/support/nfs/nfssvc.c
> > +++ b/support/nfs/nfssvc.c
> > @@ -24,28 +24,46 @@
> > #define NFSD_VERS_FILE "/proc/fs/nfsd/versions"
> > #define NFSD_THREAD_FILE "/proc/fs/nfsd/threads"
> >
> > -static void
> > -nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
> > +/*
> > + * Are there already sockets configured? If not, then it is safe to
> > try to
> > + * open some and pass them through.
> > + *
> > + * Note: If the user explicitly asked for 'udp', then we should
> > probably check
> > + * if that is open, and should open it if not. However we don't
> > yet. All
> > + * sockets have to be opened when the first daemon is started.
> > + */
> > +int
> > +nfssvc_inuse(void)
> > {
> > - int fd, n, on=1;
> > + int fd, n;
>
> read(2) returns a ssize_t result, not an int.
>
> > char buf[BUFSIZ];
>
> Eesh. BUFSIZ looks like it's two pages on my system, so adding this
> helper makes the stack requirements in this path over 16KB.
>
> Might be better if we used something a little more stack friendly
> here. The kernel bounds the size of the read(2) result to
> SIMPLE_TRANSACTION_LIMIT, which is less than a page, for example.
> Making it a static would keep "buf" off the stack, too; or checking
> how much buffer we really need (like only a few bytes, for this
> particular check) might be better.
>
Hmm good catch -- I'll admit that I never bothered to look at what BUFSIZ
actually is. SIMPLE_TRANSACTION_LIMIT sounds more reasonable, but we
can probably get away with even less than that for this.
> > - int udpfd = -1, tcpfd = -1;
> > - struct sockaddr_in sin;
> >
> > fd = open(NFSD_PORTS_FILE, O_RDONLY);
> > +
> > + /* problem opening file, assume that nothing is configured */
> > if (fd < 0)
> > - return;
> > + return 0;
> > +
> > n = read(fd, buf, BUFSIZ);
>
> Nit: Using "sizeof(buf)" might be slightly more maintainable than
> "BUFSIZ".
>
> > close(fd);
> > +
> > if (n != 0)
>
> If read(2) returns -1, we return 1 here. How about "return (n > 0);" ?
>
Good catch -- will fix.
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > +static void
> > +nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
> > +{
> > + int fd, n, on=1;
>
> Another compiler warning issue: "n" is probably now unused in
> nfssvc_setfds().
>
I think that gets removed later (part of the problem with trying to
break up huge patches into smaller ones is that you miss stuff like
this). I'll see if it can be removed here.
> >
> > + char buf[BUFSIZ];
> > + int udpfd = -1, tcpfd = -1;
> > + struct sockaddr_in sin;
> > +
> > + if (nfssvc_inuse())
> > return;
> > - /* there are no ports currently open, so it is safe to
> > - * try to open some and pass them through.
> > - * Note: If the user explicitly asked for 'udp', then
> > - * we should probably check if that is open, and should
> > - * open it if not. However we don't yet. All sockets
> > - * have to be opened when the first daemon is started.
> > - */
> > +
> > fd = open(NFSD_PORTS_FILE, O_WRONLY);
> > if (fd < 0)
> > return;
> > --
> > 1.6.2.2
> >
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
Thanks!
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 06/10] nfs-utils: convert nfssvc_setfds to use getaddrinfo
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
` (4 preceding siblings ...)
2009-06-03 19:52 ` [PATCH 05/10] nfs-utils: move check for active knfsd to helper function Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-04 20:17 ` Chuck Lever
2009-06-03 19:52 ` [PATCH 07/10] nfs-utils: break up the nfssvc interface Jeff Layton
` (3 subsequent siblings)
9 siblings, 1 reply; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
Convert nfssvc_setfds to use getaddrinfo. Change the args that it takes
and fix up nfssvc function to pass in the proper args. The things that
nfssvc has to do to call the new nfssvc_setfds is a little cumbersome
for now, but that will eventually be cleaned up in a later patch.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
support/nfs/nfssvc.c | 191 ++++++++++++++++++++++++++++++++++----------------
utils/nfsd/nfsd.c | 18 +++--
2 files changed, 140 insertions(+), 69 deletions(-)
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 8b15c4d..168414c 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -10,7 +10,9 @@
#include <config.h>
#endif
+#include <sys/types.h>
#include <sys/socket.h>
+#include <netdb.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <unistd.h>
@@ -53,85 +55,148 @@ nfssvc_inuse(void)
return 0;
}
-static void
-nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
+static int
+nfssvc_setfds(const struct addrinfo *hints, const char *node, const char *port)
{
- int fd, n, on=1;
+ int fd, on = 1, fac = L_ERROR;
+ int sockfd = -1, rc = 0;
char buf[BUFSIZ];
- int udpfd = -1, tcpfd = -1;
- struct sockaddr_in sin;
-
- if (nfssvc_inuse())
- return;
+ struct addrinfo *addrhead = NULL, *addr;
+ char *proto, *family;
+ /*
+ * if file can't be opened, then assume that it's not available and
+ * that the caller should just fall back to the old nfsctl interface
+ */
fd = open(NFSD_PORTS_FILE, O_WRONLY);
if (fd < 0)
- return;
- sin.sin_family = AF_INET;
- sin.sin_port = htons(port);
- sin.sin_addr.s_addr = inet_addr(haddr);
-
- if (NFSCTL_UDPISSET(ctlbits)) {
- udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
- if (udpfd < 0) {
- xlog(L_ERROR, "unable to create UDP socket: "
- "errno %d (%m)", errno);
- exit(1);
- }
- if (bind(udpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
- xlog(L_ERROR, "unable to bind UDP socket: "
- "errno %d (%m)", errno);
- exit(1);
- }
+ return 0;
+
+ switch(hints->ai_family) {
+ case AF_INET:
+ family = "inet";
+ break;
+ default:
+ xlog(L_ERROR, "Unknown address family specified: %d\n",
+ hints->ai_family);
+ rc = EAFNOSUPPORT;
+ goto error;
}
- if (NFSCTL_TCPISSET(ctlbits)) {
- tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
- if (tcpfd < 0) {
- xlog(L_ERROR, "unable to createt tcp socket: "
- "errno %d (%m)", errno);
- exit(1);
+ rc = getaddrinfo(node, port, hints, &addrhead);
+ if (rc == EAI_NONAME && !strcmp(port, "nfs")) {
+ snprintf(buf, BUFSIZ, "%d", NFS_PORT);
+ rc = getaddrinfo(node, buf, hints, &addrhead);
+ }
+
+ if (rc != 0) {
+ xlog(L_ERROR, "unable to resolve %s:%s to %s address: "
+ "%s", node ? node : "ANYADDR", port, family,
+ rc == EAI_SYSTEM ? strerror(errno) :
+ gai_strerror(rc));
+ goto error;
+ }
+
+ addr = addrhead;
+ while(addr) {
+ /* skip non-TCP / non-UDP sockets */
+ switch(addr->ai_protocol) {
+ case IPPROTO_UDP:
+ proto = "UDP";
+ break;
+ case IPPROTO_TCP:
+ proto = "TCP";
+ break;
+ default:
+ addr = addr->ai_next;
+ continue;
}
- if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)) < 0) {
- xlog(L_ERROR, "unable to set SO_REUSEADDR: "
- "errno %d (%m)", errno);
- exit(1);
+
+ xlog(D_GENERAL, "Creating %s %s socket.", family, proto);
+
+ /* open socket and prepare to hand it off to kernel */
+ sockfd = socket(addr->ai_family, addr->ai_socktype,
+ addr->ai_protocol);
+ if (sockfd < 0) {
+ xlog(L_ERROR, "unable to create %s %s socket: "
+ "errno %d (%m)", family, proto, errno);
+ rc = errno;
+ goto error;
}
- if (bind(tcpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
- xlog(L_ERROR, "unable to bind TCP socket: "
- "errno %d (%m)", errno);
- exit(1);
+ if (addr->ai_protocol == IPPROTO_TCP &&
+ setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) {
+ xlog(L_ERROR, "unable to set SO_REUSEADDR on %s "
+ "socket: errno %d (%m)", family, errno);
+ rc = errno;
+ goto error;
+ }
+ if (bind(sockfd, addr->ai_addr, addr->ai_addrlen)) {
+ xlog(L_ERROR, "unable to bind %s %s socket: "
+ "errno %d (%m)", family, proto, errno);
+ rc = errno;
+ goto error;
}
- if (listen(tcpfd, 64) < 0){
+ if (addr->ai_protocol == IPPROTO_TCP && listen(sockfd, 64)) {
xlog(L_ERROR, "unable to create listening socket: "
"errno %d (%m)", errno);
- exit(1);
+ rc = errno;
+ goto error;
}
- }
- if (udpfd >= 0) {
- snprintf(buf, BUFSIZ,"%d\n", udpfd);
- if (write(fd, buf, strlen(buf)) != strlen(buf)) {
- xlog(L_ERROR,
- "writing fds to kernel failed: errno %d (%m)",
- errno);
- }
- close(fd);
- fd = -1;
- }
- if (tcpfd >= 0) {
+
if (fd < 0)
fd = open(NFSD_PORTS_FILE, O_WRONLY);
- snprintf(buf, BUFSIZ,"%d\n", tcpfd);
+
+ if (fd < 0) {
+ xlog(L_ERROR, "couldn't open ports file: errno "
+ "%d (%m)", errno);
+ goto error;
+ }
+ snprintf(buf, BUFSIZ, "%d\n", sockfd);
if (write(fd, buf, strlen(buf)) != strlen(buf)) {
- xlog(L_ERROR,
- "writing fds to kernel failed: errno %d (%m)",
- errno);
+ /*
+ * this error may be common on older kernels that don't
+ * support IPv6, so turn into a debug message.
+ */
+ if (errno == EAFNOSUPPORT)
+ fac = D_ALL;
+ xlog(fac, "writing fd to kernel failed: errno %d (%m)",
+ errno);
+ rc = errno;
+ goto error;
}
+ close(fd);
+ close(sockfd);
+ sockfd = fd = -1;
+ addr = addr->ai_next;
}
- close(fd);
+error:
+ if (fd >= 0)
+ close(fd);
+ if (sockfd >= 0)
+ close(sockfd);
+ if (addrhead)
+ freeaddrinfo(addrhead);
+ return rc;
+}
- return;
+static int
+nfssvc_set_sockets(const int family, const unsigned int protobits,
+ const char *host, const char *port)
+{
+ struct addrinfo hints = { .ai_flags = AI_PASSIVE | AI_ADDRCONFIG };
+
+ hints.ai_family = family;
+
+ if (!NFSCTL_ANYPROTO(protobits))
+ return EPROTOTYPE;
+ else if (!NFSCTL_UDPISSET(protobits))
+ hints.ai_protocol = IPPROTO_TCP;
+ else if (!NFSCTL_TCPISSET(protobits))
+ hints.ai_protocol = IPPROTO_UDP;
+
+ return nfssvc_setfds(&hints, host, port);
}
+
static void
nfssvc_versbits(unsigned int ctlbits, int minorvers4)
{
@@ -169,13 +234,17 @@ nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
{
struct nfsctl_arg arg;
int fd;
+ char portstr[BUFSIZ];
/* Note: must set versions before fds so that
* the ports get registered with portmap against correct
* versions
*/
- nfssvc_versbits(versbits, minorvers4);
- nfssvc_setfds(port, protobits, haddr);
+ if (!nfssvc_inuse()) {
+ nfssvc_versbits(versbits, minorvers4);
+ snprintf(portstr, BUFSIZ, "%d", port);
+ nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
+ }
fd = open(NFSD_THREAD_FILE, O_WRONLY);
if (fd < 0)
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 6dfea67..e9d0bf9 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -76,14 +76,16 @@ main(int argc, char **argv)
xlog_stderr(1);
break;
case 'H':
- if (inet_addr(optarg) != INADDR_NONE) {
- haddr = strdup(optarg);
- } else if ((hp = gethostbyname(optarg)) != NULL) {
- haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
- } else {
- fprintf(stderr, "%s: Unknown hostname: %s\n",
- progname, optarg);
- usage(progname);
+ /*
+ * for now, this only handles one -H option. Use the
+ * last one specified.
+ */
+ free(haddr);
+ haddr = strdup(optarg);
+ if (!haddr) {
+ fprintf(stderr, "%s: unable to allocate "
+ "memory.\n", progname);
+ exit(1);
}
break;
case 'P': /* XXX for nfs-server compatibility */
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 06/10] nfs-utils: convert nfssvc_setfds to use getaddrinfo
2009-06-03 19:52 ` [PATCH 06/10] nfs-utils: convert nfssvc_setfds to use getaddrinfo Jeff Layton
@ 2009-06-04 20:17 ` Chuck Lever
2009-06-04 20:36 ` Jeff Layton
0 siblings, 1 reply; 26+ messages in thread
From: Chuck Lever @ 2009-06-04 20:17 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs, steved
On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
> Convert nfssvc_setfds to use getaddrinfo. Change the args that it
> takes
> and fix up nfssvc function to pass in the proper args. The things that
> nfssvc has to do to call the new nfssvc_setfds is a little cumbersome
> for now, but that will eventually be cleaned up in a later patch.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> support/nfs/nfssvc.c | 191 +++++++++++++++++++++++++++++++++
> +----------------
> utils/nfsd/nfsd.c | 18 +++--
> 2 files changed, 140 insertions(+), 69 deletions(-)
>
> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> index 8b15c4d..168414c 100644
> --- a/support/nfs/nfssvc.c
> +++ b/support/nfs/nfssvc.c
> @@ -10,7 +10,9 @@
> #include <config.h>
> #endif
>
> +#include <sys/types.h>
> #include <sys/socket.h>
> +#include <netdb.h>
> #include <netinet/in.h>
> #include <arpa/inet.h>
> #include <unistd.h>
> @@ -53,85 +55,148 @@ nfssvc_inuse(void)
> return 0;
> }
>
> -static void
> -nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
> +static int
> +nfssvc_setfds(const struct addrinfo *hints, const char *node, const
> char *port)
> {
> - int fd, n, on=1;
> + int fd, on = 1, fac = L_ERROR;
> + int sockfd = -1, rc = 0;
> char buf[BUFSIZ];
Same comment as earlier; BUFSIZ is probably unnecessarily large.
> - int udpfd = -1, tcpfd = -1;
> - struct sockaddr_in sin;
> -
> - if (nfssvc_inuse())
> - return;
> + struct addrinfo *addrhead = NULL, *addr;
> + char *proto, *family;
>
> + /*
> + * if file can't be opened, then assume that it's not available and
> + * that the caller should just fall back to the old nfsctl interface
> + */
> fd = open(NFSD_PORTS_FILE, O_WRONLY);
> if (fd < 0)
> - return;
> - sin.sin_family = AF_INET;
> - sin.sin_port = htons(port);
> - sin.sin_addr.s_addr = inet_addr(haddr);
> -
> - if (NFSCTL_UDPISSET(ctlbits)) {
> - udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> - if (udpfd < 0) {
> - xlog(L_ERROR, "unable to create UDP socket: "
> - "errno %d (%m)", errno);
> - exit(1);
> - }
> - if (bind(udpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> - xlog(L_ERROR, "unable to bind UDP socket: "
> - "errno %d (%m)", errno);
> - exit(1);
> - }
> + return 0;
> +
> + switch(hints->ai_family) {
> + case AF_INET:
> + family = "inet";
> + break;
> + default:
> + xlog(L_ERROR, "Unknown address family specified: %d\n",
> + hints->ai_family);
> + rc = EAFNOSUPPORT;
> + goto error;
> }
>
> - if (NFSCTL_TCPISSET(ctlbits)) {
> - tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> - if (tcpfd < 0) {
> - xlog(L_ERROR, "unable to createt tcp socket: "
> - "errno %d (%m)", errno);
> - exit(1);
> + rc = getaddrinfo(node, port, hints, &addrhead);
> + if (rc == EAI_NONAME && !strcmp(port, "nfs")) {
> + snprintf(buf, BUFSIZ, "%d", NFS_PORT);
> + rc = getaddrinfo(node, buf, hints, &addrhead);
> + }
So, I wonder about this semantic. Should rpc.nfsd fail outright if
the specified port isn't usable? If it succeeds, but silently creates
a listener on a port that wasn't specified, that might be a little
surprising to some administrators.
Also, passing sizeof(buf) instead of BUFSIZ might be more maintainable.
> +
> + if (rc != 0) {
> + xlog(L_ERROR, "unable to resolve %s:%s to %s address: "
> + "%s", node ? node : "ANYADDR", port, family,
> + rc == EAI_SYSTEM ? strerror(errno) :
> + gai_strerror(rc));
> + goto error;
> + }
> +
> + addr = addrhead;
> + while(addr) {
> + /* skip non-TCP / non-UDP sockets */
> + switch(addr->ai_protocol) {
> + case IPPROTO_UDP:
> + proto = "UDP";
> + break;
> + case IPPROTO_TCP:
> + proto = "TCP";
> + break;
> + default:
> + addr = addr->ai_next;
> + continue;
> }
> - if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))
> < 0) {
> - xlog(L_ERROR, "unable to set SO_REUSEADDR: "
> - "errno %d (%m)", errno);
> - exit(1);
> +
> + xlog(D_GENERAL, "Creating %s %s socket.", family, proto);
> +
> + /* open socket and prepare to hand it off to kernel */
> + sockfd = socket(addr->ai_family, addr->ai_socktype,
> + addr->ai_protocol);
> + if (sockfd < 0) {
> + xlog(L_ERROR, "unable to create %s %s socket: "
> + "errno %d (%m)", family, proto, errno);
> + rc = errno;
> + goto error;
> }
> - if (bind(tcpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> - xlog(L_ERROR, "unable to bind TCP socket: "
> - "errno %d (%m)", errno);
> - exit(1);
> + if (addr->ai_protocol == IPPROTO_TCP &&
> + setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &on,
> sizeof(on))) {
> + xlog(L_ERROR, "unable to set SO_REUSEADDR on %s "
> + "socket: errno %d (%m)", family, errno);
> + rc = errno;
> + goto error;
> + }
> + if (bind(sockfd, addr->ai_addr, addr->ai_addrlen)) {
> + xlog(L_ERROR, "unable to bind %s %s socket: "
> + "errno %d (%m)", family, proto, errno);
> + rc = errno;
> + goto error;
> }
> - if (listen(tcpfd, 64) < 0){
> + if (addr->ai_protocol == IPPROTO_TCP && listen(sockfd, 64)) {
> xlog(L_ERROR, "unable to create listening socket: "
> "errno %d (%m)", errno);
> - exit(1);
> + rc = errno;
> + goto error;
> }
> - }
> - if (udpfd >= 0) {
> - snprintf(buf, BUFSIZ,"%d\n", udpfd);
> - if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> - xlog(L_ERROR,
> - "writing fds to kernel failed: errno %d (%m)",
> - errno);
> - }
> - close(fd);
> - fd = -1;
> - }
> - if (tcpfd >= 0) {
> +
> if (fd < 0)
> fd = open(NFSD_PORTS_FILE, O_WRONLY);
> - snprintf(buf, BUFSIZ,"%d\n", tcpfd);
> +
> + if (fd < 0) {
> + xlog(L_ERROR, "couldn't open ports file: errno "
> + "%d (%m)", errno);
> + goto error;
> + }
> + snprintf(buf, BUFSIZ, "%d\n", sockfd);
> if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> - xlog(L_ERROR,
> - "writing fds to kernel failed: errno %d (%m)",
> - errno);
> + /*
> + * this error may be common on older kernels that don't
> + * support IPv6, so turn into a debug message.
> + */
> + if (errno == EAFNOSUPPORT)
> + fac = D_ALL;
> + xlog(fac, "writing fd to kernel failed: errno %d (%m)",
> + errno);
> + rc = errno;
> + goto error;
> }
> + close(fd);
> + close(sockfd);
> + sockfd = fd = -1;
> + addr = addr->ai_next;
> }
> - close(fd);
> +error:
> + if (fd >= 0)
> + close(fd);
> + if (sockfd >= 0)
> + close(sockfd);
> + if (addrhead)
> + freeaddrinfo(addrhead);
> + return rc;
> +}
>
> - return;
> +static int
> +nfssvc_set_sockets(const int family, const unsigned int protobits,
> + const char *host, const char *port)
> +{
> + struct addrinfo hints = { .ai_flags = AI_PASSIVE | AI_ADDRCONFIG };
> +
> + hints.ai_family = family;
> +
> + if (!NFSCTL_ANYPROTO(protobits))
> + return EPROTOTYPE;
> + else if (!NFSCTL_UDPISSET(protobits))
> + hints.ai_protocol = IPPROTO_TCP;
> + else if (!NFSCTL_TCPISSET(protobits))
> + hints.ai_protocol = IPPROTO_UDP;
> +
> + return nfssvc_setfds(&hints, host, port);
> }
> +
> static void
> nfssvc_versbits(unsigned int ctlbits, int minorvers4)
> {
> @@ -169,13 +234,17 @@ nfssvc(int port, int nrservs, unsigned int
> versbits, int minorvers4,
> {
> struct nfsctl_arg arg;
> int fd;
> + char portstr[BUFSIZ];
Ditto.
> /* Note: must set versions before fds so that
> * the ports get registered with portmap against correct
> * versions
> */
> - nfssvc_versbits(versbits, minorvers4);
> - nfssvc_setfds(port, protobits, haddr);
> + if (!nfssvc_inuse()) {
> + nfssvc_versbits(versbits, minorvers4);
> + snprintf(portstr, BUFSIZ, "%d", port);
> + nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
> + }
>
> fd = open(NFSD_THREAD_FILE, O_WRONLY);
> if (fd < 0)
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 6dfea67..e9d0bf9 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -76,14 +76,16 @@ main(int argc, char **argv)
> xlog_stderr(1);
> break;
> case 'H':
> - if (inet_addr(optarg) != INADDR_NONE) {
> - haddr = strdup(optarg);
> - } else if ((hp = gethostbyname(optarg)) != NULL) {
> - haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
> - } else {
> - fprintf(stderr, "%s: Unknown hostname: %s\n",
> - progname, optarg);
> - usage(progname);
> + /*
> + * for now, this only handles one -H option. Use the
> + * last one specified.
> + */
> + free(haddr);
> + haddr = strdup(optarg);
> + if (!haddr) {
> + fprintf(stderr, "%s: unable to allocate "
> + "memory.\n", progname);
> + exit(1);
Aside: I've been trying to stick with EXIT_SUCCESS and EXIT_FAILURE
when calling exit(3).
> }
> break;
> case 'P': /* XXX for nfs-server compatibility */
> --
> 1.6.2.2
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 06/10] nfs-utils: convert nfssvc_setfds to use getaddrinfo
2009-06-04 20:17 ` Chuck Lever
@ 2009-06-04 20:36 ` Jeff Layton
0 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-04 20:36 UTC (permalink / raw)
To: Chuck Lever; +Cc: linux-nfs, steved
On Thu, 4 Jun 2009 16:17:25 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
>
> > Convert nfssvc_setfds to use getaddrinfo. Change the args that it
> > takes
> > and fix up nfssvc function to pass in the proper args. The things that
> > nfssvc has to do to call the new nfssvc_setfds is a little cumbersome
> > for now, but that will eventually be cleaned up in a later patch.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > support/nfs/nfssvc.c | 191 +++++++++++++++++++++++++++++++++
> > +----------------
> > utils/nfsd/nfsd.c | 18 +++--
> > 2 files changed, 140 insertions(+), 69 deletions(-)
> >
> > diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> > index 8b15c4d..168414c 100644
> > --- a/support/nfs/nfssvc.c
> > +++ b/support/nfs/nfssvc.c
> > @@ -10,7 +10,9 @@
> > #include <config.h>
> > #endif
> >
> > +#include <sys/types.h>
> > #include <sys/socket.h>
> > +#include <netdb.h>
> > #include <netinet/in.h>
> > #include <arpa/inet.h>
> > #include <unistd.h>
> > @@ -53,85 +55,148 @@ nfssvc_inuse(void)
> > return 0;
> > }
> >
> > -static void
> > -nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
> > +static int
> > +nfssvc_setfds(const struct addrinfo *hints, const char *node, const
> > char *port)
> > {
> > - int fd, n, on=1;
> > + int fd, on = 1, fac = L_ERROR;
> > + int sockfd = -1, rc = 0;
> > char buf[BUFSIZ];
>
> Same comment as earlier; BUFSIZ is probably unnecessarily large.
>
Yup. Will give that a hard look.
> > - int udpfd = -1, tcpfd = -1;
> > - struct sockaddr_in sin;
> > -
> > - if (nfssvc_inuse())
> > - return;
> > + struct addrinfo *addrhead = NULL, *addr;
> > + char *proto, *family;
> >
> > + /*
> > + * if file can't be opened, then assume that it's not available and
> > + * that the caller should just fall back to the old nfsctl interface
> > + */
> > fd = open(NFSD_PORTS_FILE, O_WRONLY);
> > if (fd < 0)
> > - return;
> > - sin.sin_family = AF_INET;
> > - sin.sin_port = htons(port);
> > - sin.sin_addr.s_addr = inet_addr(haddr);
> > -
> > - if (NFSCTL_UDPISSET(ctlbits)) {
> > - udpfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
> > - if (udpfd < 0) {
> > - xlog(L_ERROR, "unable to create UDP socket: "
> > - "errno %d (%m)", errno);
> > - exit(1);
> > - }
> > - if (bind(udpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> > - xlog(L_ERROR, "unable to bind UDP socket: "
> > - "errno %d (%m)", errno);
> > - exit(1);
> > - }
> > + return 0;
> > +
> > + switch(hints->ai_family) {
> > + case AF_INET:
> > + family = "inet";
> > + break;
> > + default:
> > + xlog(L_ERROR, "Unknown address family specified: %d\n",
> > + hints->ai_family);
> > + rc = EAFNOSUPPORT;
> > + goto error;
> > }
> >
> > - if (NFSCTL_TCPISSET(ctlbits)) {
> > - tcpfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> > - if (tcpfd < 0) {
> > - xlog(L_ERROR, "unable to createt tcp socket: "
> > - "errno %d (%m)", errno);
> > - exit(1);
> > + rc = getaddrinfo(node, port, hints, &addrhead);
> > + if (rc == EAI_NONAME && !strcmp(port, "nfs")) {
> > + snprintf(buf, BUFSIZ, "%d", NFS_PORT);
> > + rc = getaddrinfo(node, buf, hints, &addrhead);
> > + }
>
> So, I wonder about this semantic. Should rpc.nfsd fail outright if
> the specified port isn't usable? If it succeeds, but silently creates
> a listener on a port that wasn't specified, that might be a little
> surprising to some administrators.
>
Well, I think that's the semantic we have here. It should only retry
with NFS_PORT if a '-p' option wasn't specified (that's what
the !strcmp is all about).
> Also, passing sizeof(buf) instead of BUFSIZ might be more maintainable.
>
Yep, I'll fix that here and in the earlier patch.
> > +
> > + if (rc != 0) {
> > + xlog(L_ERROR, "unable to resolve %s:%s to %s address: "
> > + "%s", node ? node : "ANYADDR", port, family,
> > + rc == EAI_SYSTEM ? strerror(errno) :
> > + gai_strerror(rc));
> > + goto error;
> > + }
> > +
> > + addr = addrhead;
> > + while(addr) {
> > + /* skip non-TCP / non-UDP sockets */
> > + switch(addr->ai_protocol) {
> > + case IPPROTO_UDP:
> > + proto = "UDP";
> > + break;
> > + case IPPROTO_TCP:
> > + proto = "TCP";
> > + break;
> > + default:
> > + addr = addr->ai_next;
> > + continue;
> > }
> > - if (setsockopt(tcpfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))
> > < 0) {
> > - xlog(L_ERROR, "unable to set SO_REUSEADDR: "
> > - "errno %d (%m)", errno);
> > - exit(1);
> > +
> > + xlog(D_GENERAL, "Creating %s %s socket.", family, proto);
> > +
> > + /* open socket and prepare to hand it off to kernel */
> > + sockfd = socket(addr->ai_family, addr->ai_socktype,
> > + addr->ai_protocol);
> > + if (sockfd < 0) {
> > + xlog(L_ERROR, "unable to create %s %s socket: "
> > + "errno %d (%m)", family, proto, errno);
> > + rc = errno;
> > + goto error;
> > }
> > - if (bind(tcpfd, (struct sockaddr *)&sin, sizeof(sin)) < 0){
> > - xlog(L_ERROR, "unable to bind TCP socket: "
> > - "errno %d (%m)", errno);
> > - exit(1);
> > + if (addr->ai_protocol == IPPROTO_TCP &&
> > + setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &on,
> > sizeof(on))) {
> > + xlog(L_ERROR, "unable to set SO_REUSEADDR on %s "
> > + "socket: errno %d (%m)", family, errno);
> > + rc = errno;
> > + goto error;
> > + }
> > + if (bind(sockfd, addr->ai_addr, addr->ai_addrlen)) {
> > + xlog(L_ERROR, "unable to bind %s %s socket: "
> > + "errno %d (%m)", family, proto, errno);
> > + rc = errno;
> > + goto error;
> > }
> > - if (listen(tcpfd, 64) < 0){
> > + if (addr->ai_protocol == IPPROTO_TCP && listen(sockfd, 64)) {
> > xlog(L_ERROR, "unable to create listening socket: "
> > "errno %d (%m)", errno);
> > - exit(1);
> > + rc = errno;
> > + goto error;
> > }
> > - }
> > - if (udpfd >= 0) {
> > - snprintf(buf, BUFSIZ,"%d\n", udpfd);
> > - if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - xlog(L_ERROR,
> > - "writing fds to kernel failed: errno %d (%m)",
> > - errno);
> > - }
> > - close(fd);
> > - fd = -1;
> > - }
> > - if (tcpfd >= 0) {
> > +
> > if (fd < 0)
> > fd = open(NFSD_PORTS_FILE, O_WRONLY);
> > - snprintf(buf, BUFSIZ,"%d\n", tcpfd);
> > +
> > + if (fd < 0) {
> > + xlog(L_ERROR, "couldn't open ports file: errno "
> > + "%d (%m)", errno);
> > + goto error;
> > + }
> > + snprintf(buf, BUFSIZ, "%d\n", sockfd);
> > if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > - xlog(L_ERROR,
> > - "writing fds to kernel failed: errno %d (%m)",
> > - errno);
> > + /*
> > + * this error may be common on older kernels that don't
> > + * support IPv6, so turn into a debug message.
> > + */
> > + if (errno == EAFNOSUPPORT)
> > + fac = D_ALL;
> > + xlog(fac, "writing fd to kernel failed: errno %d (%m)",
> > + errno);
> > + rc = errno;
> > + goto error;
> > }
> > + close(fd);
> > + close(sockfd);
> > + sockfd = fd = -1;
> > + addr = addr->ai_next;
> > }
> > - close(fd);
> > +error:
> > + if (fd >= 0)
> > + close(fd);
> > + if (sockfd >= 0)
> > + close(sockfd);
> > + if (addrhead)
> > + freeaddrinfo(addrhead);
> > + return rc;
> > +}
> >
> > - return;
> > +static int
> > +nfssvc_set_sockets(const int family, const unsigned int protobits,
> > + const char *host, const char *port)
> > +{
> > + struct addrinfo hints = { .ai_flags = AI_PASSIVE | AI_ADDRCONFIG };
> > +
> > + hints.ai_family = family;
> > +
> > + if (!NFSCTL_ANYPROTO(protobits))
> > + return EPROTOTYPE;
> > + else if (!NFSCTL_UDPISSET(protobits))
> > + hints.ai_protocol = IPPROTO_TCP;
> > + else if (!NFSCTL_TCPISSET(protobits))
> > + hints.ai_protocol = IPPROTO_UDP;
> > +
> > + return nfssvc_setfds(&hints, host, port);
> > }
> > +
> > static void
> > nfssvc_versbits(unsigned int ctlbits, int minorvers4)
> > {
> > @@ -169,13 +234,17 @@ nfssvc(int port, int nrservs, unsigned int
> > versbits, int minorvers4,
> > {
> > struct nfsctl_arg arg;
> > int fd;
> > + char portstr[BUFSIZ];
>
> Ditto.
>
> > /* Note: must set versions before fds so that
> > * the ports get registered with portmap against correct
> > * versions
> > */
> > - nfssvc_versbits(versbits, minorvers4);
> > - nfssvc_setfds(port, protobits, haddr);
> > + if (!nfssvc_inuse()) {
> > + nfssvc_versbits(versbits, minorvers4);
> > + snprintf(portstr, BUFSIZ, "%d", port);
> > + nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
> > + }
> >
> > fd = open(NFSD_THREAD_FILE, O_WRONLY);
> > if (fd < 0)
> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> > index 6dfea67..e9d0bf9 100644
> > --- a/utils/nfsd/nfsd.c
> > +++ b/utils/nfsd/nfsd.c
> > @@ -76,14 +76,16 @@ main(int argc, char **argv)
> > xlog_stderr(1);
> > break;
> > case 'H':
> > - if (inet_addr(optarg) != INADDR_NONE) {
> > - haddr = strdup(optarg);
> > - } else if ((hp = gethostbyname(optarg)) != NULL) {
> > - haddr = inet_ntoa((*(struct in_addr*)(hp->h_addr_list[0])));
> > - } else {
> > - fprintf(stderr, "%s: Unknown hostname: %s\n",
> > - progname, optarg);
> > - usage(progname);
> > + /*
> > + * for now, this only handles one -H option. Use the
> > + * last one specified.
> > + */
> > + free(haddr);
> > + haddr = strdup(optarg);
> > + if (!haddr) {
> > + fprintf(stderr, "%s: unable to allocate "
> > + "memory.\n", progname);
> > + exit(1);
>
> Aside: I've been trying to stick with EXIT_SUCCESS and EXIT_FAILURE
> when calling exit(3).
>
Ok. I'll have a look at that too. There's value in being consistent
here I think.
> > }
> > break;
> > case 'P': /* XXX for nfs-server compatibility */
> > --
> > 1.6.2.2
> >
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
Thanks,
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 07/10] nfs-utils: break up the nfssvc interface
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
` (5 preceding siblings ...)
2009-06-03 19:52 ` [PATCH 06/10] nfs-utils: convert nfssvc_setfds to use getaddrinfo Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-03 19:52 ` [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds Jeff Layton
` (2 subsequent siblings)
9 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
Currently, the only public interface to the routines in nfssvc.c is
nfssvc(). This means that we do an awful lot of work after closing
stderr that could be done while it's still available.
Add prototypes to the header so that more functions in nfssvc.c can be
called individually, and change the nfsd program to call those routines
individually.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
support/include/nfslib.h | 7 +++-
support/nfs/nfssvc.c | 30 ++++++++---------
utils/nfsd/nfsd.c | 77 ++++++++++++++++++++++++++++++++++------------
3 files changed, 77 insertions(+), 37 deletions(-)
diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index ae98650..14b8ba9 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -130,7 +130,12 @@ int wildmat(char *text, char *pattern);
* nfsd library functions.
*/
int nfsctl(int, struct nfsctl_arg *, union nfsctl_res *);
-int nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4, unsigned int portbits, char *haddr);
+int nfssvc_inuse(void);
+int nfssvc_set_sockets(const int family,
+ const unsigned int protobits,
+ const char *host, const char *port);
+void nfssvc_setvers(unsigned int ctlbits, int minorvers4);
+int nfssvc_threads(unsigned short port, int nrservs);
int nfsaddclient(struct nfsctl_client *clp);
int nfsdelclient(struct nfsctl_client *clp);
int nfsexport(struct nfsctl_export *exp);
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 168414c..89fb5be 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -179,7 +179,7 @@ error:
return rc;
}
-static int
+int
nfssvc_set_sockets(const int family, const unsigned int protobits,
const char *host, const char *port)
{
@@ -197,8 +197,8 @@ nfssvc_set_sockets(const int family, const unsigned int protobits,
return nfssvc_setfds(&hints, host, port);
}
-static void
-nfssvc_versbits(unsigned int ctlbits, int minorvers4)
+void
+nfssvc_setvers(unsigned int ctlbits, int minorvers4)
{
int fd, n, off;
char buf[BUFSIZ], *ptr;
@@ -228,23 +228,13 @@ nfssvc_versbits(unsigned int ctlbits, int minorvers4)
return;
}
+
int
-nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
- unsigned protobits, char *haddr)
+nfssvc_threads(unsigned short port, const int nrservs)
{
struct nfsctl_arg arg;
+ struct servent *ent;
int fd;
- char portstr[BUFSIZ];
-
- /* Note: must set versions before fds so that
- * the ports get registered with portmap against correct
- * versions
- */
- if (!nfssvc_inuse()) {
- nfssvc_versbits(versbits, minorvers4);
- snprintf(portstr, BUFSIZ, "%d", port);
- nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
- }
fd = open(NFSD_THREAD_FILE, O_WRONLY);
if (fd < 0)
@@ -265,6 +255,14 @@ nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
return 0;
}
+ if (!port) {
+ ent = getservbyname("nfs", "udp");
+ if (ent != NULL)
+ port = ntohs(ent->s_port);
+ else
+ port = NFS_PORT;
+ }
+
arg.ca_version = NFSCTL_VERSION;
arg.ca_svc.svc_nthreads = nrservs;
arg.ca_svc.svc_port = port;
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index e9d0bf9..012847b 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -43,21 +43,14 @@ static struct option longopts[] =
unsigned int protobits = NFSCTL_ALLBITS;
unsigned int versbits = NFSCTL_ALLBITS;
int minorvers4 = NFSD_MAXMINORVERS4; /* nfsv4 minor version */
-char *haddr = NULL;
int
main(int argc, char **argv)
{
- int count = 1, c, error, port, fd, found_one;
- struct servent *ent;
- struct hostent *hp;
- char *p, *progname;
-
- ent = getservbyname ("nfs", "udp");
- if (ent != NULL)
- port = ntohs (ent->s_port);
- else
- port = 2049;
+ int count = 1, c, error, portnum = 0, fd, found_one;
+ char *p, *progname, *port;
+ char *haddr = NULL;
+ int socket_up = 0;
progname = strdup(basename(argv[0]));
if (!progname) {
@@ -65,6 +58,12 @@ main(int argc, char **argv)
exit(1);
}
+ port = strdup("nfs");
+ if (!port) {
+ fprintf(stderr, "%s: unable to allocate memory.\n", progname);
+ exit(1);
+ }
+
xlog_syslog(1);
xlog_stderr(0);
@@ -90,12 +89,20 @@ main(int argc, char **argv)
break;
case 'P': /* XXX for nfs-server compatibility */
case 'p':
- port = atoi(optarg);
- if (port <= 0 || port > 65535) {
+ /* only the last -p option has any effect */
+ portnum = atoi(optarg);
+ if (portnum <= 0 || portnum > 65535) {
fprintf(stderr, "%s: bad port number: %s\n",
progname, optarg);
usage(progname);
}
+ free(port);
+ port = strdup(optarg);
+ if (!port) {
+ fprintf(stderr, "%s: unable to allocate "
+ "memory.\n", progname);
+ exit(1);
+ }
break;
case 'N':
switch((c = strtol(optarg, &p, 0))) {
@@ -167,10 +174,38 @@ main(int argc, char **argv)
count = 1;
}
}
- /* KLUDGE ALERT:
- Some kernels let nfsd kernel threads inherit open files
- from the program that spawns them (i.e. us). So close
- everything before spawning kernel threads. --Chip */
+
+ /* can only change number of threads if nfsd is already up */
+ if (nfssvc_inuse()) {
+ socket_up = 1;
+ goto set_threads;
+ }
+
+ /*
+ * must set versions before the fd's so that the right versions get
+ * registered with rpcbind. Note that on older kernels w/o the right
+ * interfaces, these are a no-op.
+ */
+ nfssvc_setvers(versbits, minorvers4);
+
+ error = nfssvc_set_sockets(AF_INET, protobits, haddr, port);
+ if (!error)
+ socket_up = 1;
+
+set_threads:
+ /* don't start any threads if unable to hand off any sockets */
+ if (!socket_up) {
+ xlog(L_ERROR, "unable to set any sockets for nfsd");
+ goto out;
+ }
+ error = 0;
+
+ /*
+ * KLUDGE ALERT:
+ * Some kernels let nfsd kernel threads inherit open files
+ * from the program that spawns them (i.e. us). So close
+ * everything before spawning kernel threads. --Chip
+ */
fd = open("/dev/null", O_RDWR);
if (fd == -1)
xlog(L_ERROR, "Unable to open /dev/null: %m");
@@ -183,9 +218,11 @@ main(int argc, char **argv)
xlog_stderr(0);
closeall(3);
- if ((error = nfssvc(port, count, versbits, minorvers4, protobits, haddr)) < 0)
- xlog(L_ERROR, "nfssvc (%m)");
-
+ if ((error = nfssvc_threads(portnum, count)) < 0)
+ xlog(L_ERROR, "error starting threads: %s", strerror(error));
+out:
+ free(port);
+ free(haddr);
free(progname);
return (error != 0);
}
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
` (6 preceding siblings ...)
2009-06-03 19:52 ` [PATCH 07/10] nfs-utils: break up the nfssvc interface Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-03 20:08 ` Chuck Lever
2009-06-04 21:00 ` Chuck Lever
2009-06-03 19:52 ` [PATCH 09/10] nfs-utils: add IPv6 support to nfsd Jeff Layton
2009-06-03 19:52 ` [PATCH 10/10] nfs-utils: update the nfsd manpage Jeff Layton
9 siblings, 2 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
Allow nfssvc_setfds to properly deal with AF_INET6.
IPv6 sockets for knfsd can't be allowed to accept IPv4 packets. Set the
correct option to prevent that from occurring on IPv6 sockets.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
support/nfs/nfssvc.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
index 89fb5be..19f1a4c 100644
--- a/support/nfs/nfssvc.c
+++ b/support/nfs/nfssvc.c
@@ -76,6 +76,11 @@ nfssvc_setfds(const struct addrinfo *hints, const char *node, const char *port)
case AF_INET:
family = "inet";
break;
+#ifdef IPV6_SUPPORTED
+ case AF_INET6:
+ family = "inet6";
+ break;
+#endif /* IPV6_SUPPORTED */
default:
xlog(L_ERROR, "Unknown address family specified: %d\n",
hints->ai_family);
@@ -123,6 +128,15 @@ nfssvc_setfds(const struct addrinfo *hints, const char *node, const char *port)
rc = errno;
goto error;
}
+#ifdef IPV6_SUPPORTED
+ if (addr->ai_family == AF_INET6 &&
+ setsockopt(sockfd, IPPROTO_IPV6, IPV6_V6ONLY, &on, sizeof(on))) {
+ xlog(L_ERROR, "unable to set IPV6_V6ONLY: "
+ "errno %d (%s)\n", errno, strerror(errno));
+ rc = -errno;
+ goto error;
+ }
+#endif /* IPV6_SUPPORTED */
if (addr->ai_protocol == IPPROTO_TCP &&
setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) {
xlog(L_ERROR, "unable to set SO_REUSEADDR on %s "
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds
2009-06-03 19:52 ` [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds Jeff Layton
@ 2009-06-03 20:08 ` Chuck Lever
2009-06-03 20:16 ` Steve Dickson
2009-06-04 21:00 ` Chuck Lever
1 sibling, 1 reply; 26+ messages in thread
From: Chuck Lever @ 2009-06-03 20:08 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs, steved
On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
> Allow nfssvc_setfds to properly deal with AF_INET6.
>
> IPv6 sockets for knfsd can't be allowed to accept IPv4 packets. Set
> the
> correct option to prevent that from occurring on IPv6 sockets.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> support/nfs/nfssvc.c | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> index 89fb5be..19f1a4c 100644
> --- a/support/nfs/nfssvc.c
> +++ b/support/nfs/nfssvc.c
> @@ -76,6 +76,11 @@ nfssvc_setfds(const struct addrinfo *hints, const
> char *node, const char *port)
> case AF_INET:
> family = "inet";
> break;
> +#ifdef IPV6_SUPPORTED
> + case AF_INET6:
> + family = "inet6";
> + break;
> +#endif /* IPV6_SUPPORTED */
> default:
> xlog(L_ERROR, "Unknown address family specified: %d\n",
> hints->ai_family);
> @@ -123,6 +128,15 @@ nfssvc_setfds(const struct addrinfo *hints,
> const char *node, const char *port)
> rc = errno;
> goto error;
> }
> +#ifdef IPV6_SUPPORTED
> + if (addr->ai_family == AF_INET6 &&
> + setsockopt(sockfd, IPPROTO_IPV6, IPV6_V6ONLY, &on,
> sizeof(on))) {
> + xlog(L_ERROR, "unable to set IPV6_V6ONLY: "
> + "errno %d (%s)\n", errno, strerror(errno));
I'm still looking at the other patches, but I notice you could use %m
here too.
> + rc = -errno;
> + goto error;
> + }
> +#endif /* IPV6_SUPPORTED */
> if (addr->ai_protocol == IPPROTO_TCP &&
> setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) {
> xlog(L_ERROR, "unable to set SO_REUSEADDR on %s "
> --
> 1.6.2.2
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds
2009-06-03 20:08 ` Chuck Lever
@ 2009-06-03 20:16 ` Steve Dickson
[not found] ` <4A26DA3B.3090404-AfCzQyP5zfLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 26+ messages in thread
From: Steve Dickson @ 2009-06-03 20:16 UTC (permalink / raw)
To: Chuck Lever; +Cc: Jeff Layton, linux-nfs
Chuck Lever wrote:
>
> On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
>
>> Allow nfssvc_setfds to properly deal with AF_INET6.
>>
>> IPv6 sockets for knfsd can't be allowed to accept IPv4 packets. Set the
>> correct option to prevent that from occurring on IPv6 sockets.
>>
>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
>> ---
>> support/nfs/nfssvc.c | 14 ++++++++++++++
>> 1 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
>> index 89fb5be..19f1a4c 100644
>> --- a/support/nfs/nfssvc.c
>> +++ b/support/nfs/nfssvc.c
>> @@ -76,6 +76,11 @@ nfssvc_setfds(const struct addrinfo *hints, const
>> char *node, const char *port)
>> case AF_INET:
>> family = "inet";
>> break;
>> +#ifdef IPV6_SUPPORTED
>> + case AF_INET6:
>> + family = "inet6";
>> + break;
>> +#endif /* IPV6_SUPPORTED */
>> default:
>> xlog(L_ERROR, "Unknown address family specified: %d\n",
>> hints->ai_family);
>> @@ -123,6 +128,15 @@ nfssvc_setfds(const struct addrinfo *hints, const
>> char *node, const char *port)
>> rc = errno;
>> goto error;
>> }
>> +#ifdef IPV6_SUPPORTED
>> + if (addr->ai_family == AF_INET6 &&
>> + setsockopt(sockfd, IPPROTO_IPV6, IPV6_V6ONLY, &on,
>> sizeof(on))) {
>> + xlog(L_ERROR, "unable to set IPV6_V6ONLY: "
>> + "errno %d (%s)\n", errno, strerror(errno));
>
> I'm still looking at the other patches, but I notice you could use %m
> here too.
I kinda like having both... the errno *and* the error string...
steved.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds
2009-06-03 19:52 ` [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds Jeff Layton
2009-06-03 20:08 ` Chuck Lever
@ 2009-06-04 21:00 ` Chuck Lever
1 sibling, 0 replies; 26+ messages in thread
From: Chuck Lever @ 2009-06-04 21:00 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-nfs, steved
On Jun 3, 2009, at 3:52 PM, Jeff Layton wrote:
> Allow nfssvc_setfds to properly deal with AF_INET6.
>
> IPv6 sockets for knfsd can't be allowed to accept IPv4 packets. Set
> the
> correct option to prevent that from occurring on IPv6 sockets.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> support/nfs/nfssvc.c | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/support/nfs/nfssvc.c b/support/nfs/nfssvc.c
> index 89fb5be..19f1a4c 100644
> --- a/support/nfs/nfssvc.c
> +++ b/support/nfs/nfssvc.c
> @@ -76,6 +76,11 @@ nfssvc_setfds(const struct addrinfo *hints, const
> char *node, const char *port)
> case AF_INET:
> family = "inet";
> break;
> +#ifdef IPV6_SUPPORTED
> + case AF_INET6:
> + family = "inet6";
> + break;
> +#endif /* IPV6_SUPPORTED */
> default:
> xlog(L_ERROR, "Unknown address family specified: %d\n",
> hints->ai_family);
> @@ -123,6 +128,15 @@ nfssvc_setfds(const struct addrinfo *hints,
> const char *node, const char *port)
> rc = errno;
> goto error;
> }
> +#ifdef IPV6_SUPPORTED
> + if (addr->ai_family == AF_INET6 &&
> + setsockopt(sockfd, IPPROTO_IPV6, IPV6_V6ONLY, &on,
> sizeof(on))) {
> + xlog(L_ERROR, "unable to set IPV6_V6ONLY: "
> + "errno %d (%s)\n", errno, strerror(errno));
> + rc = -errno;
Maybe "rc = errno;" ?
> + goto error;
> + }
> +#endif /* IPV6_SUPPORTED */
> if (addr->ai_protocol == IPPROTO_TCP &&
> setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) {
> xlog(L_ERROR, "unable to set SO_REUSEADDR on %s "
> --
> 1.6.2.2
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 09/10] nfs-utils: add IPv6 support to nfsd
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
` (7 preceding siblings ...)
2009-06-03 19:52 ` [PATCH 08/10] nfs-utils: add IPv6 support to nfssvc_setfds Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
2009-06-03 19:52 ` [PATCH 10/10] nfs-utils: update the nfsd manpage Jeff Layton
9 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
Add support for handing off IPv6 sockets to the kernel for nfsd. One of
the main goals here is to not change the behavior of options and not to
add any new ones, so this patch attempts to do that.
We also don't want to break anything in the event that someone has an
rpc.nfsd program built with IPv6 capability, but the knfsd doesn't
support IPv6. Ditto for the cases where IPv6 is either not compiled in
or is compiled in and blacklisted.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsd/nfsd.c | 114 +++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 89 insertions(+), 25 deletions(-)
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 012847b..8d753f2 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -40,17 +40,63 @@ static struct option longopts[] =
{ "debug", 0, 0, 'd' },
{ NULL, 0, 0, 0 }
};
-unsigned int protobits = NFSCTL_ALLBITS;
-unsigned int versbits = NFSCTL_ALLBITS;
-int minorvers4 = NFSD_MAXMINORVERS4; /* nfsv4 minor version */
+
+/* given a family and ctlbits, disable any that aren't listed in netconfig */
+#ifdef HAVE_LIBTIRPC
+static void
+nfsd_enable_protos(unsigned int *proto4, unsigned int *proto6)
+{
+ struct netconfig *nconf;
+ unsigned int *famproto;
+ void *handle;
+
+ xlog(D_GENERAL, "Checking netconfig for visible protocols.");
+
+ handle = setnetconfig();
+ while((nconf = getnetconfig(handle))) {
+ if (!(nconf->nc_flag & NC_VISIBLE))
+ continue;
+
+ if (!strcmp(nconf->nc_protofmly, NC_INET))
+ famproto = proto4;
+ else if (!strcmp(nconf->nc_protofmly, NC_INET6))
+ famproto = proto6;
+ else
+ continue;
+
+ if (!strcmp(nconf->nc_proto, NC_TCP))
+ NFSCTL_TCPSET(*famproto);
+ else if (!strcmp(nconf->nc_proto, NC_UDP))
+ NFSCTL_UDPSET(*famproto);
+
+ xlog(D_GENERAL, "Enabling %s %s.", nconf->nc_protofmly,
+ nconf->nc_proto);
+ }
+ endnetconfig(handle);
+ return;
+}
+#else /* HAVE_LIBTIRPC */
+static void
+nfsd_enable_protos(unsigned int *proto4, unsigned int *proto6)
+{
+ /* Enable all IPv4 protocols if no TIRPC support */
+ *proto4 = NFSCTL_ALLBITS;
+ *proto6 = 0;
+}
+#endif /* HAVE_LIBTIRPC */
int
main(int argc, char **argv)
{
- int count = 1, c, error, portnum = 0, fd, found_one;
+ int count = 1, c, error = 0, portnum = 0, fd, found_one;
char *p, *progname, *port;
char *haddr = NULL;
int socket_up = 0;
+ int minorvers4 = NFSD_MAXMINORVERS4; /* nfsv4 minor version */
+ unsigned int versbits = NFSCTL_ALLBITS;
+ unsigned int protobits = NFSCTL_ALLBITS;
+ unsigned int proto4 = 0;
+ unsigned int proto6 = 0;
progname = strdup(basename(argv[0]));
if (!progname) {
@@ -133,15 +179,38 @@ main(int argc, char **argv)
}
}
+ if (optind < argc) {
+ if ((count = atoi(argv[optind])) < 0) {
+ /* insane # of servers */
+ fprintf(stderr,
+ "%s: invalid server count (%d), using 1\n",
+ argv[0], count);
+ count = 1;
+ } else if (count == 0) {
+ /*
+ * don't bother setting anything else if the threads
+ * are coming down anyway.
+ */
+ socket_up = 1;
+ goto set_threads;
+ }
+ }
+
xlog_open(progname);
- /*
- * Do some sanity checking, if the ctlbits are set
- */
- if (!NFSCTL_UDPISSET(protobits) && !NFSCTL_TCPISSET(protobits)) {
- xlog(L_ERROR, "invalid protocol specified");
- exit(1);
+ nfsd_enable_protos(&proto4, &proto6);
+
+ if (!NFSCTL_TCPISSET(protobits)) {
+ NFSCTL_TCPUNSET(proto4);
+ NFSCTL_TCPUNSET(proto6);
+ }
+
+ if (!NFSCTL_UDPISSET(protobits)) {
+ NFSCTL_UDPUNSET(proto4);
+ NFSCTL_UDPUNSET(proto6);
}
+
+ /* make sure that at least one version is enabled */
found_one = 0;
for (c = NFSD_MINVERS; c <= NFSD_MAXVERS; c++) {
if (NFSCTL_VERISSET(versbits, c))
@@ -152,29 +221,18 @@ main(int argc, char **argv)
exit(1);
}
- if (NFSCTL_VERISSET(versbits, 4) && !NFSCTL_TCPISSET(protobits)) {
+ if (NFSCTL_VERISSET(versbits, 4) &&
+ !NFSCTL_TCPISSET(proto4) &&
+ !NFSCTL_TCPISSET(proto6)) {
xlog(L_ERROR, "version 4 requires the TCP protocol");
exit(1);
}
- if (haddr == NULL) {
- struct in_addr in = {INADDR_ANY};
- haddr = strdup(inet_ntoa(in));
- }
if (chdir(NFS_STATEDIR)) {
xlog(L_ERROR, "chdir(%s) failed: %m", NFS_STATEDIR);
exit(1);
}
- if (optind < argc) {
- if ((count = atoi(argv[optind])) < 0) {
- /* insane # of servers */
- xlog(L_ERROR, "invalid server count (%d), using 1",
- count);
- count = 1;
- }
- }
-
/* can only change number of threads if nfsd is already up */
if (nfssvc_inuse()) {
socket_up = 1;
@@ -187,10 +245,16 @@ main(int argc, char **argv)
* interfaces, these are a no-op.
*/
nfssvc_setvers(versbits, minorvers4);
+
+ error = nfssvc_set_sockets(AF_INET, proto4, haddr, port);
+ if (!error)
+ socket_up = 1;
- error = nfssvc_set_sockets(AF_INET, protobits, haddr, port);
+#ifdef IPV6_SUPPORTED
+ error = nfssvc_set_sockets(AF_INET6, proto6, haddr, port);
if (!error)
socket_up = 1;
+#endif /* IPV6_SUPPORTED */
set_threads:
/* don't start any threads if unable to hand off any sockets */
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 10/10] nfs-utils: update the nfsd manpage
2009-06-03 19:52 [PATCH 00/10] nfs-utils: add IPv6 support to rpc.nfsd (try #5) Jeff Layton
` (8 preceding siblings ...)
2009-06-03 19:52 ` [PATCH 09/10] nfs-utils: add IPv6 support to nfsd Jeff Layton
@ 2009-06-03 19:52 ` Jeff Layton
9 siblings, 0 replies; 26+ messages in thread
From: Jeff Layton @ 2009-06-03 19:52 UTC (permalink / raw)
To: linux-nfs; +Cc: chuck.lever, steved
Add some clarification about the purpose of the program, info about the
--debug option, and a note about how it behaves when TI-RPC support is
built in.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsd/nfsd.man | 19 ++++++++++++++++---
1 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index 4eb3e21..94b041e 100644
--- a/utils/nfsd/nfsd.man
+++ b/utils/nfsd/nfsd.man
@@ -13,8 +13,9 @@ The
program implements the user level part of the NFS service. The
main functionality is handled by the
.B nfsd
-kernel module; the user space program merely starts the specified
-number of kernel threads.
+kernel module. The user space program merely specifies what sort of sockets
+the kernel service should listen on, what NFS versions it should support, and
+how many kernel threads it should use.
.P
The
.B rpc.mountd
@@ -22,6 +23,11 @@ server provides an ancillary service needed to satisfy mount requests
by NFS clients.
.SH OPTIONS
.TP
+.B \-d " or " \-\-debug
+enable debug log messages and log messages to stderr instead of syslog.
+By default, nfsd logs all messages to syslog, except for errors generated
+during option processing which go to stderr.
+.TP
.B \-H " or " \-\-host hostname
specify a particular hostname (or address) that NFS requests will
be accepted on. By default,
@@ -75,12 +81,19 @@ In particular
.B rpc.nfsd 0
will stop all threads and thus close any open connections.
+.SH NOTES
+If the program is built with TI-RPC support, it will enable any protocol and
+address family combinations that are marked visible in the
+.B netconfig
+database.
+
.SH SEE ALSO
.BR rpc.mountd (8),
.BR exports (5),
.BR exportfs (8),
.BR rpc.rquotad (8),
-.BR nfsstat (8).
+.BR nfsstat (8),
+.BR netconfig(5).
.SH AUTHOR
Olaf Kirch, Bill Hawes, H. J. Lu, G. Allan Morris III,
and a host of others.
--
1.6.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread