[PATCH 00/11] nfs-utils: add TIRPC/IPv6 support to rpc.nfsd

[PATCH 00/11] nfs-utils: add TIRPC/IPv6 support to rpc.nfsd

[Jeff Layton]

This is the sixth attempt at a patchset to add TIRPC and IPv6 support to
the rpc.nfsd program. The significant differences from the last set are:

1) Change it to log errors to stderr by default, and add a --syslog
switch to force output to syslog

2) moved nfssvc.c into the nfsd directory (and out of libnfs.a).
rpc.nfsd is the only user of those routines so there's no need to
keep it in common code.

3) declared a common static string buffer for the nfssvc routines
to use to cut down stack usage. It's also smaller than BUFSIZ so the
general memory footprint for rpc.nfsd should be smaller.

There are also a number of cleanups and small fixes.

Comments and suggestions appreciated...

Jeff Layton (11):
  nfs-utils: move nfssvc.c to nfsd dir and clean up linking of nfsd
  nfs-utils: clean up option parsing in nfsd.c
  nfs-utils: convert rpc.nfsd to use xlog()
  nfs-utils: clean up NFSCTL_* macros for handling protocol bits
  nfs-utils: declare a static common buffer for nfssvc.c routines
  nfs-utils: move check for active knfsd to helper function
  nfs-utils: convert nfssvc_setfds to use getaddrinfo
  nfs-utils: break up the nfssvc interface
  nfs-utils: add IPv6 support to nfssvc_setfds
  nfs-utils: add IPv6 support to nfsd
  nfs-utils: update the nfsd manpage and copyright notices

 support/include/nfs/nfs.h |   15 ++-
 support/include/nfslib.h  |    1 -
 support/nfs/Makefile.am   |    2 +-
 support/nfs/nfssvc.c      |  187 -----------------------------
 utils/nfsd/Makefile.am    |    6 +-
 utils/nfsd/nfsd.c         |  264 ++++++++++++++++++++++++++++++-----------
 utils/nfsd/nfsd.man       |   27 ++++-
 utils/nfsd/nfssvc.c       |  290 +++++++++++++++++++++++++++++++++++++++++++++
 utils/nfsd/nfssvc.h       |   27 ++++
 9 files changed, 548 insertions(+), 271 deletions(-)
 delete mode 100644 support/nfs/nfssvc.c
 create mode 100644 utils/nfsd/nfssvc.c
 create mode 100644 utils/nfsd/nfssvc.h

[PATCH 01/11] nfs-utils: move nfssvc.c to nfsd dir and clean up linking of nfsd

[Jeff Layton]

rpc.nfsd is the only user of nfssvc.c, so we might as well move it
out of libnfs.a.

Also, don't link in libexport.a and libmisc.a, they aren't needed.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 support/include/nfslib.h             |    1 -
 support/nfs/Makefile.am              |    2 +-
 utils/nfsd/Makefile.am               |    6 ++----
 utils/nfsd/nfsd.c                    |    1 +
 {support/nfs => utils/nfsd}/nfssvc.c |    2 +-
 utils/nfsd/nfssvc.h                  |   24 ++++++++++++++++++++++++
 6 files changed, 29 insertions(+), 7 deletions(-)
 rename {support/nfs => utils/nfsd}/nfssvc.c (99%)
 create mode 100644 utils/nfsd/nfssvc.h

diff --git a/support/include/nfslib.h b/support/include/nfslib.h
index ae98650..537a31e 100644
--- a/support/include/nfslib.h
+++ b/support/include/nfslib.h
@@ -130,7 +130,6 @@ 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			nfsaddclient(struct nfsctl_client *clp);
 int			nfsdelclient(struct nfsctl_client *clp);
 int			nfsexport(struct nfsctl_export *exp);
diff --git a/support/nfs/Makefile.am b/support/nfs/Makefile.am
index 86f52a1..096f56d 100644
--- a/support/nfs/Makefile.am
+++ b/support/nfs/Makefile.am
@@ -2,7 +2,7 @@
 
 noinst_LIBRARIES = libnfs.a
 libnfs_a_SOURCES = exports.c rmtab.c xio.c rpcmisc.c rpcdispatch.c \
-		   xlog.c xcommon.c wildmat.c nfssvc.c nfsclient.c \
+		   xlog.c xcommon.c wildmat.c nfsclient.c \
 		   nfsexport.c getfh.c nfsctl.c rpc_socket.c getport.c \
 		   svc_socket.c cacheio.c closeall.c nfs_mntent.c
 
diff --git a/utils/nfsd/Makefile.am b/utils/nfsd/Makefile.am
index 445e3fd..c4c6fb0 100644
--- a/utils/nfsd/Makefile.am
+++ b/utils/nfsd/Makefile.am
@@ -7,10 +7,8 @@ RPCPREFIX	= rpc.
 KPREFIX		= @kprefix@
 sbin_PROGRAMS	= nfsd
 
-nfsd_SOURCES = nfsd.c
-nfsd_LDADD = ../../support/export/libexport.a \
-	     ../../support/nfs/libnfs.a \
-	     ../../support/misc/libmisc.a
+nfsd_SOURCES = nfsd.c nfssvc.c
+nfsd_LDADD = ../../support/nfs/libnfs.a
 
 MAINTAINERCLEANFILES = Makefile.in
 
diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index e3c0094..8563c5c 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -25,6 +25,7 @@
 #include <arpa/inet.h>
 
 #include "nfslib.h"
+#include "nfssvc.h"
 
 static void	usage(const char *);
 
diff --git a/support/nfs/nfssvc.c b/utils/nfsd/nfssvc.c
similarity index 99%
rename from support/nfs/nfssvc.c
rename to utils/nfsd/nfssvc.c
index 33c15a7..6c5289a 100644
--- a/support/nfs/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -1,5 +1,5 @@
 /*
- * support/nfs/nfssvc.c
+ * utils/nfsd/nfssvc.c
  *
  * Run an NFS daemon.
  *
diff --git a/utils/nfsd/nfssvc.h b/utils/nfsd/nfssvc.h
new file mode 100644
index 0000000..e77ff94
--- /dev/null
+++ b/utils/nfsd/nfssvc.h
@@ -0,0 +1,24 @@
+/*
+ *   utils/nfsd/nfssvc.h -- nfs service control routines for rpc.nfsd
+ *
+ *   Copyright (c) 2009 Red Hat, Inc.
+ *   Author(s): Jeff Layton (jlayton@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+int	nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
+	       unsigned int portbits, char *haddr);
-- 
1.6.0.6

[PATCH 02/11] nfs-utils: clean up option parsing in nfsd.c

[Jeff Layton]

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 8563c5c..183681b 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -98,11 +98,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.0.6

[PATCH 03/11] nfs-utils: convert rpc.nfsd to use xlog()

[Jeff Layton]

...and add --debug and --syslog options.

With the switch to xlog(), it becomes trivial to add debug messages, so
add an option to turn them on when requested.

Also, rpc.nfsd isn't a proper daemon per-se, so it makes more sense to
log errors to stderr where possible. Usually init scripts take care of
redirecting stderr output to syslog anyway.

For those that don't, add a --syslog option that forces all output to go
to syslog instead. Note that even with this option, errors encountered
during option processing will still go to stderr.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsd/nfsd.c   |   67 ++++++++++++++++++++++++++++++++------------------
 utils/nfsd/nfssvc.c |   47 +++++++++++++++++------------------
 2 files changed, 66 insertions(+), 48 deletions(-)

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 183681b..1589a9f 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -18,7 +18,6 @@
 #include <string.h>
 #include <errno.h>
 #include <getopt.h>
-#include <syslog.h>
 #include <netdb.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
@@ -26,6 +25,7 @@
 
 #include "nfslib.h"
 #include "nfssvc.h"
+#include "xlog.h"
 
 static void	usage(const char *);
 
@@ -38,6 +38,8 @@ static struct option longopts[] =
 	{ "no-udp", 0, 0, 'U' },
 	{ "port", 1, 0, 'P' },
 	{ "port", 1, 0, 'p' },
+	{ "debug", 0, 0, 'd' },
+	{ "syslog", 0, 0, 's' },
 	{ NULL, 0, 0, 0 }
 };
 unsigned int protobits = NFSCTL_ALLBITS;
@@ -51,7 +53,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)
@@ -59,8 +61,20 @@ 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(0);
+	xlog_stderr(1);
+
+	while ((c = getopt_long(argc, argv, "dH:hN:p:P:sTU", longopts, NULL)) != EOF) {
 		switch(c) {
+		case 'd':
+			xlog_config(D_ALL, 1);
+			break;
 		case 'H':
 			if (inet_addr(optarg) != INADDR_NONE) {
 				haddr = strdup(optarg);
@@ -68,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 */
@@ -77,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':
@@ -97,6 +111,10 @@ main(int argc, char **argv)
 				exit(1);
 			}
 			break;
+		case 's':
+			xlog_syslog(1);
+			xlog_stderr(0);
+			break;
 		case 'T':
 			NFSCTL_TCPUNSET(protobits);
 			break;
@@ -106,14 +124,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;
@@ -122,12 +143,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) {
@@ -136,17 +157,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;
 		}
 	}
@@ -156,21 +175,21 @@ 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 {
+		/* switch xlog output to syslog since stderr is being closed */
+		xlog_syslog(1);
+		xlog_stderr(0);
 		(void) dup2(fd, 0);
 		(void) dup2(fd, 1);
 		(void) dup2(fd, 2);
 	}
 	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: errno %d (%m)", errno);
 
+	free(progname);
 	return (error != 0);
 }
 
@@ -178,7 +197,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 [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs\n", 
 		prog);
 	exit(2);
 }
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 6c5289a..0a7546a 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/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 create 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);
@@ -138,11 +137,11 @@ nfssvc_versbits(unsigned int ctlbits, int minorvers4)
 		    off += snprintf(ptr+off, BUFSIZ - off, "%c4.%d",
 				    minorvers4 > 0 ? '+' : '-',
 				    n);
+	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
 	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;
-- 
1.6.0.6

[PATCH 04/11] nfs-utils: clean up NFSCTL_* macros for handling protocol bits

[Jeff Layton]

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.0.6

[PATCH 05/11] nfs-utils: declare a static common buffer for nfssvc.c routines

[Jeff Layton]

Several of the routines in nfssvc.c declare a buffer for strings. Use a
shared static buffer instead to keep it off of the stack. Also, the
buffer allocated in some places is *really* large. BUFSIZ is generally
8k. These routines don't need nearly that much.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsd/nfssvc.c |   28 +++++++++++++++++-----------
 1 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 0a7546a..025554d 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -24,18 +24,25 @@
 #define NFSD_VERS_FILE    "/proc/fs/nfsd/versions"
 #define NFSD_THREAD_FILE  "/proc/fs/nfsd/threads"
 
+/*
+ * declaring a common static scratch buffer here keeps us from having to
+ * continually thrash the stack. The value of 128 bytes here is really just a
+ * SWAG and can be increased if necessary. It ought to be enough for the
+ * routines below however.
+ */
+char buf[128];
+
 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;
 
 	fd = open(NFSD_PORTS_FILE, O_RDONLY);
 	if (fd < 0)
 		return;
-	n = read(fd, buf, BUFSIZ);
+	n = read(fd, buf, sizeof(buf));
 	close(fd);
 	if (n != 0)
 		return;
@@ -91,7 +98,7 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
 		}
 	}
 	if (udpfd >= 0) {
-		snprintf(buf, BUFSIZ,"%d\n", udpfd); 
+		snprintf(buf, sizeof(buf), "%d\n", udpfd); 
 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
 			xlog(L_ERROR, 
 			       "writing fds to kernel failed: errno %d (%m)", 
@@ -103,7 +110,7 @@ nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
 	if (tcpfd >= 0) {
 		if (fd < 0)
 			fd = open(NFSD_PORTS_FILE, O_WRONLY);
-		snprintf(buf, BUFSIZ,"%d\n", tcpfd); 
+		snprintf(buf, sizeof(buf), "%d\n", tcpfd); 
 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
 			xlog(L_ERROR, 
 			       "writing fds to kernel failed: errno %d (%m)", 
@@ -118,7 +125,7 @@ static void
 nfssvc_versbits(unsigned int ctlbits, int minorvers4)
 {
 	int fd, n, off;
-	char buf[BUFSIZ], *ptr;
+	char *ptr;
 
 	ptr = buf;
 	off = 0;
@@ -128,17 +135,17 @@ nfssvc_versbits(unsigned int ctlbits, int minorvers4)
 
 	for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
 		if (NFSCTL_VERISSET(ctlbits, n))
-		    off += snprintf(ptr+off, BUFSIZ - off, "+%d ", n);
+		    off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
 		else
-		    off += snprintf(ptr+off, BUFSIZ - off, "-%d ", n);
+		    off += snprintf(ptr+off, sizeof(buf) - off, "-%d ", n);
 	}
 	n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
 	if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
-		    off += snprintf(ptr+off, BUFSIZ - off, "%c4.%d",
+		    off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
 				    minorvers4 > 0 ? '+' : '-',
 				    n);
 	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
-	snprintf(ptr+off, BUFSIZ - off, "\n");
+	snprintf(ptr+off, sizeof(buf) - off, "\n");
 	if (write(fd, buf, strlen(buf)) != strlen(buf))
 		xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
 
@@ -168,9 +175,8 @@ nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
 		 * Just write the number in.
 		 * Cannot handle port number yet, but does anyone care?
 		 */
-		char buf[20];
 		int n;
-		snprintf(buf, 20,"%d\n", nrservs);
+		snprintf(buf, sizeof(buf), "%d\n", nrservs);
 		n = write(fd, buf, strlen(buf));
 		close(fd);
 		if (n != strlen(buf))
-- 
1.6.0.6

[PATCH 06/11] nfs-utils: move check for active knfsd to helper function

[Jeff Layton]

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>
---
 utils/nfsd/nfssvc.c |   44 ++++++++++++++++++++++++++++++--------------
 1 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 025554d..7ecaea9 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -32,27 +32,43 @@
  */
 char buf[128];
 
-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 udpfd = -1, tcpfd = -1;
-	struct sockaddr_in sin;
+	int fd, n;
 
 	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, sizeof(buf));
 	close(fd);
-	if (n != 0)
+
+	xlog(D_GENERAL, "knfsd is currently %s", (n > 0) ? "up" : "down");
+
+	return (n > 0);
+}
+
+static void
+nfssvc_setfds(int port, unsigned int ctlbits, char *haddr)
+{
+	int fd, on=1;
+	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.0.6

[PATCH 07/11] nfs-utils: convert nfssvc_setfds to use getaddrinfo

[Jeff Layton]

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>
---
 utils/nfsd/nfsd.c   |   18 +++--
 utils/nfsd/nfssvc.c |  191 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 140 insertions(+), 69 deletions(-)

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 1589a9f..78dddc2 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -76,14 +76,16 @@ main(int argc, char **argv)
 			xlog_config(D_ALL, 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 */
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 7ecaea9..68899cb 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/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>
@@ -59,84 +61,148 @@ nfssvc_inuse(void)
 	return (n > 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, on=1;
-	int udpfd = -1, tcpfd = -1;
-	struct sockaddr_in sin;
-
-	if (nfssvc_inuse())
-		return;
+	int fd, on = 1, fac = L_ERROR;
+	int sockfd = -1, rc = 0;
+	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 create TCP socket: "
-				"errno %d (%m)", errno);
-			exit(1);
+	rc = getaddrinfo(node, port, hints, &addrhead);
+	if (rc == EAI_NONAME && !strcmp(port, "nfs")) {
+		snprintf(buf, sizeof(buf), "%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, sizeof(buf), "%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, sizeof(buf), "%d\n", tcpfd); 
+
+		if (fd < 0) {
+			xlog(L_ERROR, "couldn't open ports file: errno "
+				      "%d (%m)", errno);
+			goto error;
+		}
+
+		snprintf(buf, sizeof(buf), "%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)
 {
@@ -180,8 +246,11 @@ nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
 	 * 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(buf, sizeof(buf), "%d", port);
+		nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
+	}
 
 	fd = open(NFSD_THREAD_FILE, O_WRONLY);
 	if (fd < 0)
-- 
1.6.0.6

[PATCH 08/11] nfs-utils: break up the nfssvc interface

[Jeff Layton]

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>
---
 utils/nfsd/nfsd.c   |   77 +++++++++++++++++++++++++++++++++++++-------------
 utils/nfsd/nfssvc.c |   34 ++++++++++------------
 utils/nfsd/nfssvc.h |    7 +++-
 3 files changed, 78 insertions(+), 40 deletions(-)

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 78dddc2..c82249b 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -45,21 +45,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) {
@@ -67,6 +60,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(0);
 	xlog_stderr(1);
 
@@ -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))) {
@@ -171,10 +178,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");
@@ -188,9 +223,11 @@ main(int argc, char **argv)
 	}
 	closeall(3);
 
-	if ((error = nfssvc(port, count, versbits, minorvers4, protobits, haddr)) < 0)
-		xlog(L_ERROR, "nfssvc: errno %d (%m)", errno);
-
+	if ((error = nfssvc_threads(portnum, count)) < 0)
+		xlog(L_ERROR, "error starting threads: errno %d (%m)", errno);
+out:
+	free(port);
+	free(haddr);
 	free(progname);
 	return (error != 0);
 }
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 68899cb..106f6e7 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -185,7 +185,7 @@ error:
 	return rc;
 }
 
-static int
+int
 nfssvc_set_sockets(const int family, const unsigned int protobits,
 		   const char *host, const char *port)
 {
@@ -203,8 +203,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 *ptr;
@@ -235,32 +235,22 @@ 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;
+	ssize_t n;
 	int fd;
 
-	/* 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(buf, sizeof(buf), "%d", port);
-		nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
-	}
-
 	fd = open(NFSD_THREAD_FILE, O_WRONLY);
 	if (fd < 0)
 		fd = open("/proc/fs/nfs/threads", O_WRONLY);
 	if (fd >= 0) {
 		/* 2.5+ kernel with nfsd filesystem mounted.
-		 * Just write the number in.
-		 * Cannot handle port number yet, but does anyone care?
+		 * Just write the number of threads.
 		 */
-		int n;
 		snprintf(buf, sizeof(buf), "%d\n", nrservs);
 		n = write(fd, buf, strlen(buf));
 		close(fd);
@@ -270,6 +260,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/nfssvc.h b/utils/nfsd/nfssvc.h
index e77ff94..3ac3ed4 100644
--- a/utils/nfsd/nfssvc.h
+++ b/utils/nfsd/nfssvc.h
@@ -20,5 +20,8 @@
  *
  */
 
-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);
-- 
1.6.0.6

[PATCH 09/11] nfs-utils: add IPv6 support to nfssvc_setfds

[Jeff Layton]

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>
---
 utils/nfsd/nfssvc.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index 106f6e7..ee862b2 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -81,6 +81,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);
@@ -128,6 +133,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 (%m)\n", 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.0.6

[PATCH 10/11] nfs-utils: add IPv6 support to nfsd

[Jeff Layton]

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 c82249b..650c593 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -42,17 +42,63 @@ static struct option longopts[] =
 	{ "syslog", 0, 0, 's' },
 	{ 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) {
@@ -137,15 +183,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))
@@ -156,29 +225,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;
@@ -191,10 +249,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.0.6

[PATCH 11/11] nfs-utils: update the nfsd manpage and copyright notices

[Jeff Layton]

Add some clarification about the purpose of the program, info about the
--debug and --syslog options, and a note about how it behaves when
TI-RPC support is built in.

Also update copyright messages for the files under utils/nfsd.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 utils/nfsd/nfsd.c   |    1 +
 utils/nfsd/nfsd.man |   27 +++++++++++++++++++++++----
 utils/nfsd/nfssvc.c |    1 +
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
index 650c593..53f9bbb 100644
--- a/utils/nfsd/nfsd.c
+++ b/utils/nfsd/nfsd.c
@@ -5,6 +5,7 @@
  * all the work is now done in the kernel module.
  *
  * Copyright (C) 1995, 1996 Olaf Kirch <okir-pn4DOG8n3UYbFoVRYvo4fw@public.gmane.org>
+ * Copyright (c) 2009 Jeff Layton <jlayton@redhat.com>
  */
 
 #ifdef HAVE_CONFIG_H
diff --git a/utils/nfsd/nfsd.man b/utils/nfsd/nfsd.man
index 4eb3e21..2c8f7f9 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,9 @@ server provides an ancillary service needed to satisfy mount requests
 by NFS clients.
 .SH OPTIONS
 .TP
+.B \-d " or " \-\-debug
+enable logging of debugging messages
+.TP
 .B \-H " or " \-\-host  hostname
 specify a particular hostname (or address) that NFS requests will
 be accepted on. By default,
@@ -45,6 +49,14 @@ does not offer certain versions of NFS. The current version of
 .B rpc.nfsd
 can support both NFS version 2,3 and the newer version 4.
 .TP
+.B \-s " or " \-\-syslog
+By default,
+.B rpc.nfsd
+logs error messages (and debug messages, if enabled) to stderr. This option makes 
+.B rpc.nfsd
+log these messages to syslog instead. Note that errors encountered during
+option processing will still be logged to stderr regardless of this option.
+.TP
 .B \-T " or " \-\-no-tcp
 Disable 
 .B rpc.nfsd 
@@ -75,12 +87,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,
+Olaf Kirch, Bill Hawes, H. J. Lu, G. Allan Morris III, Jeff Layton
 and a host of others.
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
index ee862b2..c2e90c4 100644
--- a/utils/nfsd/nfssvc.c
+++ b/utils/nfsd/nfssvc.c
@@ -4,6 +4,7 @@
  * Run an NFS daemon.
  *
  * Copyright (C) 1995, 1996 Olaf Kirch <okir-pn4DOG8n3UYbFoVRYvo4fw@public.gmane.org>
+ * Copyright (c) 2009 Jeff Layton <jlayton@redhat.com>
  */
 
 #ifdef HAVE_CONFIG_H
-- 
1.6.0.6

Re: [PATCH 03/11] nfs-utils: convert rpc.nfsd to use xlog()

[Chuck Lever]

On Jun 8, 2009, at 2:00 PM, Jeff Layton wrote:

> ...and add --debug and --syslog options.
>
> With the switch to xlog(), it becomes trivial to add debug messages,  
> so
> add an option to turn them on when requested.
>
> Also, rpc.nfsd isn't a proper daemon per-se, so it makes more sense to
> log errors to stderr where possible. Usually init scripts take care of
> redirecting stderr output to syslog anyway.
>
> For those that don't, add a --syslog option that forces all output  
> to go
> to syslog instead. Note that even with this option, errors encountered
> during option processing will still go to stderr.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> utils/nfsd/nfsd.c   |   67 +++++++++++++++++++++++++++++++ 
> +------------------
> utils/nfsd/nfssvc.c |   47 +++++++++++++++++------------------
> 2 files changed, 66 insertions(+), 48 deletions(-)
>
> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> index 183681b..1589a9f 100644
> --- a/utils/nfsd/nfsd.c
> +++ b/utils/nfsd/nfsd.c
> @@ -18,7 +18,6 @@
> #include <string.h>
> #include <errno.h>
> #include <getopt.h>
> -#include <syslog.h>
> #include <netdb.h>
> #include <sys/socket.h>
> #include <netinet/in.h>
> @@ -26,6 +25,7 @@
>
> #include "nfslib.h"
> #include "nfssvc.h"
> +#include "xlog.h"
>
> static void	usage(const char *);
>
> @@ -38,6 +38,8 @@ static struct option longopts[] =
> 	{ "no-udp", 0, 0, 'U' },
> 	{ "port", 1, 0, 'P' },
> 	{ "port", 1, 0, 'p' },
> +	{ "debug", 0, 0, 'd' },
> +	{ "syslog", 0, 0, 's' },
> 	{ NULL, 0, 0, 0 }
> };
> unsigned int protobits = NFSCTL_ALLBITS;
> @@ -51,7 +53,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)
> @@ -59,8 +61,20 @@ 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(0);
> +	xlog_stderr(1);

I think this is an OK design overall, but don't you need to do an  
xlog_open() before you start generating these error messages?  I think  
that's why most of the other components just stick with  
fprintf(stderr) while processing command line options.

> +
> +	while ((c = getopt_long(argc, argv, "dH:hN:p:P:sTU", longopts,  
> NULL)) != EOF) {
> 		switch(c) {
> +		case 'd':
> +			xlog_config(D_ALL, 1);
> +			break;
> 		case 'H':
> 			if (inet_addr(optarg) != INADDR_NONE) {
> 				haddr = strdup(optarg);
> @@ -68,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 */
> @@ -77,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':
> @@ -97,6 +111,10 @@ main(int argc, char **argv)
> 				exit(1);
> 			}
> 			break;
> +		case 's':
> +			xlog_syslog(1);
> +			xlog_stderr(0);
> +			break;
> 		case 'T':
> 			NFSCTL_TCPUNSET(protobits);
> 			break;
> @@ -106,14 +124,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;
> @@ -122,12 +143,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) {
> @@ -136,17 +157,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;
> 		}
> 	}
> @@ -156,21 +175,21 @@ 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 {
> +		/* switch xlog output to syslog since stderr is being closed */
> +		xlog_syslog(1);
> +		xlog_stderr(0);
> 		(void) dup2(fd, 0);
> 		(void) dup2(fd, 1);
> 		(void) dup2(fd, 2);
> 	}
> 	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: errno %d (%m)", errno);
>
> +	free(progname);
> 	return (error != 0);
> }
>
> @@ -178,7 +197,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 [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs- 
> version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs 
> \n",
> 		prog);
> 	exit(2);
> }
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 6c5289a..0a7546a 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/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 create 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);
> @@ -138,11 +137,11 @@ nfssvc_versbits(unsigned int ctlbits, int  
> minorvers4)
> 		    off += snprintf(ptr+off, BUFSIZ - off, "%c4.%d",
> 				    minorvers4 > 0 ? '+' : '-',
> 				    n);
> +	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
> 	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;
> -- 
> 1.6.0.6
>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 05/11] nfs-utils: declare a static common buffer for nfssvc.c routines

[Chuck Lever]

On Jun 8, 2009, at 2:00 PM, Jeff Layton wrote:

> Several of the routines in nfssvc.c declare a buffer for strings.  
> Use a
> shared static buffer instead to keep it off of the stack. Also, the
> buffer allocated in some places is *really* large. BUFSIZ is generally
> 8k. These routines don't need nearly that much.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> utils/nfsd/nfssvc.c |   28 +++++++++++++++++-----------
> 1 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> index 0a7546a..025554d 100644
> --- a/utils/nfsd/nfssvc.c
> +++ b/utils/nfsd/nfssvc.c
> @@ -24,18 +24,25 @@
> #define NFSD_VERS_FILE    "/proc/fs/nfsd/versions"
> #define NFSD_THREAD_FILE  "/proc/fs/nfsd/threads"
>
> +/*
> + * declaring a common static scratch buffer here keeps us from  
> having to
> + * continually thrash the stack. The value of 128 bytes here is  
> really just a
> + * SWAG and can be increased if necessary. It ought to be enough  
> for the
> + * routines below however.
> + */
> +char buf[128];
> +
> 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;
>
> 	fd = open(NFSD_PORTS_FILE, O_RDONLY);
> 	if (fd < 0)
> 		return;
> -	n = read(fd, buf, BUFSIZ);
> +	n = read(fd, buf, sizeof(buf));
>
> 	close(fd);
> 	if (n != 0)
> 		return;
> @@ -91,7 +98,7 @@ nfssvc_setfds(int port, unsigned int ctlbits, char  
> *haddr)
> 		}
> 	}
> 	if (udpfd >= 0) {
> -		snprintf(buf, BUFSIZ,"%d\n", udpfd);
> +		snprintf(buf, sizeof(buf), "%d\n", udpfd);

Since sizeof(buf) is a SWAG, maybe we should have some way of finding  
out if the buffer is ever overflowed. Just a thought.

> 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> 			xlog(L_ERROR,
> 			       "writing fds to kernel failed: errno %d (%m)",
> @@ -103,7 +110,7 @@ nfssvc_setfds(int port, unsigned int ctlbits,  
> char *haddr)
> 	if (tcpfd >= 0) {
> 		if (fd < 0)
> 			fd = open(NFSD_PORTS_FILE, O_WRONLY);
> -		snprintf(buf, BUFSIZ,"%d\n", tcpfd);
> +		snprintf(buf, sizeof(buf), "%d\n", tcpfd);
> 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> 			xlog(L_ERROR,
> 			       "writing fds to kernel failed: errno %d (%m)",
> @@ -118,7 +125,7 @@ static void
> nfssvc_versbits(unsigned int ctlbits, int minorvers4)
> {
> 	int fd, n, off;
> -	char buf[BUFSIZ], *ptr;
> +	char *ptr;
>
> 	ptr = buf;
> 	off = 0;
> @@ -128,17 +135,17 @@ nfssvc_versbits(unsigned int ctlbits, int  
> minorvers4)
>
> 	for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
> 		if (NFSCTL_VERISSET(ctlbits, n))
> -		    off += snprintf(ptr+off, BUFSIZ - off, "+%d ", n);
> +		    off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
> 		else
> -		    off += snprintf(ptr+off, BUFSIZ - off, "-%d ", n);
> +		    off += snprintf(ptr+off, sizeof(buf) - off, "-%d ", n);
> 	}
> 	n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
> 	if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
> -		    off += snprintf(ptr+off, BUFSIZ - off, "%c4.%d",
> +		    off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
> 				    minorvers4 > 0 ? '+' : '-',
> 				    n);
> 	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
> -	snprintf(ptr+off, BUFSIZ - off, "\n");
> +	snprintf(ptr+off, sizeof(buf) - off, "\n");
> 	if (write(fd, buf, strlen(buf)) != strlen(buf))
> 		xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
>
> @@ -168,9 +175,8 @@ nfssvc(int port, int nrservs, unsigned int  
> versbits, int minorvers4,
> 		 * Just write the number in.
> 		 * Cannot handle port number yet, but does anyone care?
> 		 */
> -		char buf[20];
> 		int n;
> -		snprintf(buf, 20,"%d\n", nrservs);
> +		snprintf(buf, sizeof(buf), "%d\n", nrservs);
> 		n = write(fd, buf, strlen(buf));
> 		close(fd);
> 		if (n != strlen(buf))
> -- 
> 1.6.0.6
>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 03/11] nfs-utils: convert rpc.nfsd to use xlog()

[Jeff Layton]

On Mon, 8 Jun 2009 14:16:48 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On Jun 8, 2009, at 2:00 PM, Jeff Layton wrote:
> 
> > ...and add --debug and --syslog options.
> >
> > With the switch to xlog(), it becomes trivial to add debug messages,  
> > so
> > add an option to turn them on when requested.
> >
> > Also, rpc.nfsd isn't a proper daemon per-se, so it makes more sense to
> > log errors to stderr where possible. Usually init scripts take care of
> > redirecting stderr output to syslog anyway.
> >
> > For those that don't, add a --syslog option that forces all output  
> > to go
> > to syslog instead. Note that even with this option, errors encountered
> > during option processing will still go to stderr.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > utils/nfsd/nfsd.c   |   67 +++++++++++++++++++++++++++++++ 
> > +------------------
> > utils/nfsd/nfssvc.c |   47 +++++++++++++++++------------------
> > 2 files changed, 66 insertions(+), 48 deletions(-)
> >
> > diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
> > index 183681b..1589a9f 100644
> > --- a/utils/nfsd/nfsd.c
> > +++ b/utils/nfsd/nfsd.c
> > @@ -18,7 +18,6 @@
> > #include <string.h>
> > #include <errno.h>
> > #include <getopt.h>
> > -#include <syslog.h>
> > #include <netdb.h>
> > #include <sys/socket.h>
> > #include <netinet/in.h>
> > @@ -26,6 +25,7 @@
> >
> > #include "nfslib.h"
> > #include "nfssvc.h"
> > +#include "xlog.h"
> >
> > static void	usage(const char *);
> >
> > @@ -38,6 +38,8 @@ static struct option longopts[] =
> > 	{ "no-udp", 0, 0, 'U' },
> > 	{ "port", 1, 0, 'P' },
> > 	{ "port", 1, 0, 'p' },
> > +	{ "debug", 0, 0, 'd' },
> > +	{ "syslog", 0, 0, 's' },
> > 	{ NULL, 0, 0, 0 }
> > };
> > unsigned int protobits = NFSCTL_ALLBITS;
> > @@ -51,7 +53,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)
> > @@ -59,8 +61,20 @@ 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(0);
> > +	xlog_stderr(1);
> 
> I think this is an OK design overall, but don't you need to do an  
> xlog_open() before you start generating these error messages?  I think  
> that's why most of the other components just stick with  
> fprintf(stderr) while processing command line options.
> 

Yes, and this code does that. It doesn't use xlog until xlog_open has
been done. The 2 lines above just set the variables that determine
where xlog output actually goes. That just sets the defaults for these
values so that they can be properly overriden by --syslog.

> > +
> > +	while ((c = getopt_long(argc, argv, "dH:hN:p:P:sTU", longopts,  
> > NULL)) != EOF) {
> > 		switch(c) {
> > +		case 'd':
> > +			xlog_config(D_ALL, 1);
> > +			break;
> > 		case 'H':
> > 			if (inet_addr(optarg) != INADDR_NONE) {
> > 				haddr = strdup(optarg);
> > @@ -68,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 */
> > @@ -77,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':
> > @@ -97,6 +111,10 @@ main(int argc, char **argv)
> > 				exit(1);
> > 			}
> > 			break;
> > +		case 's':
> > +			xlog_syslog(1);
> > +			xlog_stderr(0);
> > +			break;
> > 		case 'T':
> > 			NFSCTL_TCPUNSET(protobits);
> > 			break;
> > @@ -106,14 +124,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;
> > @@ -122,12 +143,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) {
> > @@ -136,17 +157,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;
> > 		}
> > 	}
> > @@ -156,21 +175,21 @@ 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 {
> > +		/* switch xlog output to syslog since stderr is being closed */
> > +		xlog_syslog(1);
> > +		xlog_stderr(0);
> > 		(void) dup2(fd, 0);
> > 		(void) dup2(fd, 1);
> > 		(void) dup2(fd, 2);
> > 	}
> > 	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: errno %d (%m)", errno);
> >
> > +	free(progname);
> > 	return (error != 0);
> > }
> >
> > @@ -178,7 +197,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 [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs- 
> > version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs 
> > \n",
> > 		prog);
> > 	exit(2);
> > }
> > diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> > index 6c5289a..0a7546a 100644
> > --- a/utils/nfsd/nfssvc.c
> > +++ b/utils/nfsd/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 create 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);
> > @@ -138,11 +137,11 @@ nfssvc_versbits(unsigned int ctlbits, int  
> > minorvers4)
> > 		    off += snprintf(ptr+off, BUFSIZ - off, "%c4.%d",
> > 				    minorvers4 > 0 ? '+' : '-',
> > 				    n);
> > +	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
> > 	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;
> > -- 
> > 1.6.0.6
> >
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 


-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH 03/11] nfs-utils: convert rpc.nfsd to use xlog()

[Chuck Lever]

On Jun 8, 2009, at 2:23 PM, Jeff Layton wrote:

> On Mon, 8 Jun 2009 14:16:48 -0400
> Chuck Lever <chuck.lever@oracle.com> wrote:
>
>>
>> On Jun 8, 2009, at 2:00 PM, Jeff Layton wrote:
>>
>>> ...and add --debug and --syslog options.
>>>
>>> With the switch to xlog(), it becomes trivial to add debug messages,
>>> so
>>> add an option to turn them on when requested.
>>>
>>> Also, rpc.nfsd isn't a proper daemon per-se, so it makes more  
>>> sense to
>>> log errors to stderr where possible. Usually init scripts take  
>>> care of
>>> redirecting stderr output to syslog anyway.
>>>
>>> For those that don't, add a --syslog option that forces all output
>>> to go
>>> to syslog instead. Note that even with this option, errors  
>>> encountered
>>> during option processing will still go to stderr.
>>>
>>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
>>> ---
>>> utils/nfsd/nfsd.c   |   67 +++++++++++++++++++++++++++++++
>>> +------------------
>>> utils/nfsd/nfssvc.c |   47 +++++++++++++++++------------------
>>> 2 files changed, 66 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/utils/nfsd/nfsd.c b/utils/nfsd/nfsd.c
>>> index 183681b..1589a9f 100644
>>> --- a/utils/nfsd/nfsd.c
>>> +++ b/utils/nfsd/nfsd.c
>>> @@ -18,7 +18,6 @@
>>> #include <string.h>
>>> #include <errno.h>
>>> #include <getopt.h>
>>> -#include <syslog.h>
>>> #include <netdb.h>
>>> #include <sys/socket.h>
>>> #include <netinet/in.h>
>>> @@ -26,6 +25,7 @@
>>>
>>> #include "nfslib.h"
>>> #include "nfssvc.h"
>>> +#include "xlog.h"
>>>
>>> static void	usage(const char *);
>>>
>>> @@ -38,6 +38,8 @@ static struct option longopts[] =
>>> 	{ "no-udp", 0, 0, 'U' },
>>> 	{ "port", 1, 0, 'P' },
>>> 	{ "port", 1, 0, 'p' },
>>> +	{ "debug", 0, 0, 'd' },
>>> +	{ "syslog", 0, 0, 's' },
>>> 	{ NULL, 0, 0, 0 }
>>> };
>>> unsigned int protobits = NFSCTL_ALLBITS;
>>> @@ -51,7 +53,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)
>>> @@ -59,8 +61,20 @@ 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(0);
>>> +	xlog_stderr(1);
>>
>> I think this is an OK design overall, but don't you need to do an
>> xlog_open() before you start generating these error messages?  I  
>> think
>> that's why most of the other components just stick with
>> fprintf(stderr) while processing command line options.
>>
>
> Yes, and this code does that. It doesn't use xlog until xlog_open has
> been done. The 2 lines above just set the variables that determine
> where xlog output actually goes. That just sets the defaults for these
> values so that they can be properly overriden by --syslog.
>
>>> +
>>> +	while ((c = getopt_long(argc, argv, "dH:hN:p:P:sTU", longopts,
>>> NULL)) != EOF) {
>>> 		switch(c) {
>>> +		case 'd':
>>> +			xlog_config(D_ALL, 1);

OK, I was distracted by this (and the above)...

>>>
>>> +			break;
>>> 		case 'H':
>>> 			if (inet_addr(optarg) != INADDR_NONE) {
>>> 				haddr = strdup(optarg);
>>> @@ -68,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",

And did not notice this.

>>>
>>> -					argv[0], optarg);
>>> -				usage(argv [0]);
>>> +					progname, optarg);
>>> +				usage(progname);
>>> 			}
>>> 			break;
>>> 		case 'P':	/* XXX for nfs-server compatibility */
>>> @@ -77,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':
>>> @@ -97,6 +111,10 @@ main(int argc, char **argv)
>>> 				exit(1);
>>> 			}
>>> 			break;
>>> +		case 's':
>>> +			xlog_syslog(1);
>>> +			xlog_stderr(0);
>>> +			break;
>>> 		case 'T':
>>> 			NFSCTL_TCPUNSET(protobits);
>>> 			break;
>>> @@ -106,14 +124,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;
>>> @@ -122,12 +143,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) {
>>> @@ -136,17 +157,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;
>>> 		}
>>> 	}
>>> @@ -156,21 +175,21 @@ 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 {
>>> +		/* switch xlog output to syslog since stderr is being closed */
>>> +		xlog_syslog(1);
>>> +		xlog_stderr(0);
>>> 		(void) dup2(fd, 0);
>>> 		(void) dup2(fd, 1);
>>> 		(void) dup2(fd, 2);
>>> 	}
>>> 	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: errno %d (%m)", errno);
>>>
>>> +	free(progname);
>>> 	return (error != 0);
>>> }
>>>
>>> @@ -178,7 +197,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 [-d|--debug] [-H hostname] [-p|-P|--port port] [-N|--no-nfs-
>>> version version ] [-s|--syslog] [-T|--no-tcp] [-U|--no-udp] nrservs
>>> \n",
>>> 		prog);
>>> 	exit(2);
>>> }
>>> diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
>>> index 6c5289a..0a7546a 100644
>>> --- a/utils/nfsd/nfssvc.c
>>> +++ b/utils/nfsd/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 create 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);
>>> @@ -138,11 +137,11 @@ nfssvc_versbits(unsigned int ctlbits, int
>>> minorvers4)
>>> 		    off += snprintf(ptr+off, BUFSIZ - off, "%c4.%d",
>>> 				    minorvers4 > 0 ? '+' : '-',
>>> 				    n);
>>> +	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
>>> 	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;
>>> -- 
>>> 1.6.0.6
>>>
>>
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>
>
>
> -- 
> Jeff Layton <jlayton@poochiereds.net>

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 05/11] nfs-utils: declare a static common buffer for nfssvc.c routines

[Jeff Layton]

On Mon, 8 Jun 2009 14:22:28 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 
> On Jun 8, 2009, at 2:00 PM, Jeff Layton wrote:
> 
> > Several of the routines in nfssvc.c declare a buffer for strings.  
> > Use a
> > shared static buffer instead to keep it off of the stack. Also, the
> > buffer allocated in some places is *really* large. BUFSIZ is generally
> > 8k. These routines don't need nearly that much.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > utils/nfsd/nfssvc.c |   28 +++++++++++++++++-----------
> > 1 files changed, 17 insertions(+), 11 deletions(-)
> >
> > diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c
> > index 0a7546a..025554d 100644
> > --- a/utils/nfsd/nfssvc.c
> > +++ b/utils/nfsd/nfssvc.c
> > @@ -24,18 +24,25 @@
> > #define NFSD_VERS_FILE    "/proc/fs/nfsd/versions"
> > #define NFSD_THREAD_FILE  "/proc/fs/nfsd/threads"
> >
> > +/*
> > + * declaring a common static scratch buffer here keeps us from  
> > having to
> > + * continually thrash the stack. The value of 128 bytes here is  
> > really just a
> > + * SWAG and can be increased if necessary. It ought to be enough  
> > for the
> > + * routines below however.
> > + */
> > +char buf[128];
> > +
> > 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;
> >
> > 	fd = open(NFSD_PORTS_FILE, O_RDONLY);
> > 	if (fd < 0)
> > 		return;
> > -	n = read(fd, buf, BUFSIZ);
> > +	n = read(fd, buf, sizeof(buf));
> >
> > 	close(fd);
> > 	if (n != 0)
> > 		return;
> > @@ -91,7 +98,7 @@ nfssvc_setfds(int port, unsigned int ctlbits, char  
> > *haddr)
> > 		}
> > 	}
> > 	if (udpfd >= 0) {
> > -		snprintf(buf, BUFSIZ,"%d\n", udpfd);
> > +		snprintf(buf, sizeof(buf), "%d\n", udpfd);
> 
> Since sizeof(buf) is a SWAG, maybe we should have some way of finding  
> out if the buffer is ever overflowed. Just a thought.
> 

I don't think it's possible with the strings we're snprintf'ing into
there today. In general I'm all for bounds checking, but I don't think
it'll ever actually fire here.

It may be more of a danger for the nfssvc_versbits function, but even
there I don't think it's possible. It might be reasonable follow-on
patch though to guard against that problem for future changes.


> > 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > 			xlog(L_ERROR,
> > 			       "writing fds to kernel failed: errno %d (%m)",
> > @@ -103,7 +110,7 @@ nfssvc_setfds(int port, unsigned int ctlbits,  
> > char *haddr)
> > 	if (tcpfd >= 0) {
> > 		if (fd < 0)
> > 			fd = open(NFSD_PORTS_FILE, O_WRONLY);
> > -		snprintf(buf, BUFSIZ,"%d\n", tcpfd);
> > +		snprintf(buf, sizeof(buf), "%d\n", tcpfd);
> > 		if (write(fd, buf, strlen(buf)) != strlen(buf)) {
> > 			xlog(L_ERROR,
> > 			       "writing fds to kernel failed: errno %d (%m)",
> > @@ -118,7 +125,7 @@ static void
> > nfssvc_versbits(unsigned int ctlbits, int minorvers4)
> > {
> > 	int fd, n, off;
> > -	char buf[BUFSIZ], *ptr;
> > +	char *ptr;
> >
> > 	ptr = buf;
> > 	off = 0;
> > @@ -128,17 +135,17 @@ nfssvc_versbits(unsigned int ctlbits, int  
> > minorvers4)
> >
> > 	for (n = NFSD_MINVERS; n <= NFSD_MAXVERS; n++) {
> > 		if (NFSCTL_VERISSET(ctlbits, n))
> > -		    off += snprintf(ptr+off, BUFSIZ - off, "+%d ", n);
> > +		    off += snprintf(ptr+off, sizeof(buf) - off, "+%d ", n);
> > 		else
> > -		    off += snprintf(ptr+off, BUFSIZ - off, "-%d ", n);
> > +		    off += snprintf(ptr+off, sizeof(buf) - off, "-%d ", n);
> > 	}
> > 	n = minorvers4 >= 0 ? minorvers4 : -minorvers4;
> > 	if (n >= NFSD_MINMINORVERS4 && n <= NFSD_MAXMINORVERS4)
> > -		    off += snprintf(ptr+off, BUFSIZ - off, "%c4.%d",
> > +		    off += snprintf(ptr+off, sizeof(buf) - off, "%c4.%d",
> > 				    minorvers4 > 0 ? '+' : '-',
> > 				    n);
> > 	xlog(D_GENERAL, "Writing version string to kernel: %s", buf);
> > -	snprintf(ptr+off, BUFSIZ - off, "\n");
> > +	snprintf(ptr+off, sizeof(buf) - off, "\n");
> > 	if (write(fd, buf, strlen(buf)) != strlen(buf))
> > 		xlog(L_ERROR, "Setting version failed: errno %d (%m)", errno);
> >
> > @@ -168,9 +175,8 @@ nfssvc(int port, int nrservs, unsigned int  
> > versbits, int minorvers4,
> > 		 * Just write the number in.
> > 		 * Cannot handle port number yet, but does anyone care?
> > 		 */
> > -		char buf[20];
> > 		int n;
> > -		snprintf(buf, 20,"%d\n", nrservs);
> > +		snprintf(buf, sizeof(buf), "%d\n", nrservs);
> > 		n = write(fd, buf, strlen(buf));
> > 		close(fd);
> > 		if (n != strlen(buf))
> > -- 
> > 1.6.0.6
> >
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 


-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 07/11] nfs-utils: convert nfssvc_setfds to use getaddrinfo

[Steve Dickson]

On 06/08/2009 02:00 PM, Jeff Layton wrote:
>  static void
>  nfssvc_versbits(unsigned int ctlbits, int minorvers4)
>  {
> @@ -180,8 +246,11 @@ nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
>  	 * 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(buf, sizeof(buf), "%d", port);
> +		nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
portstr is not defined here.... and as far as I can tell its not
defined in any of the next 3 patches either... 

What am I missing or it is missing?

steved.

Re: [PATCH 07/11] nfs-utils: convert nfssvc_setfds to use getaddrinfo

[Jeff Layton]

On Sat, 01 Aug 2009 07:35:50 -0400
Steve Dickson <SteveD@redhat.com> wrote:

> On 06/08/2009 02:00 PM, Jeff Layton wrote:
> >  static void
> >  nfssvc_versbits(unsigned int ctlbits, int minorvers4)
> >  {
> > @@ -180,8 +246,11 @@ nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
> >  	 * 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(buf, sizeof(buf), "%d", port);
> > +		nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
> portstr is not defined here.... and as far as I can tell its not
> defined in any of the next 3 patches either... 
> 
> What am I missing or it is missing?
> 
> steved.
> 

You're right. That's a spot that I missed when I converted that code to
use the static buffer instead of allocating a memory all over the
place. That "portstr" should be "buf" instead.

The good news is that a later patch removes that entire if statement.
The final outcome of the set shouldn't be affected by this. Would you
like me to respin that patch and the later one that removes that chunk
of code?

-- 
Jeff Layton <jlayton@redhat.com>

Re: [PATCH 07/11] nfs-utils: convert nfssvc_setfds to use getaddrinfo

[Steve Dickson]

On 08/01/2009 08:45 AM, Jeff Layton wrote:
> On Sat, 01 Aug 2009 07:35:50 -0400
> Steve Dickson <SteveD@redhat.com> wrote:
> 
>> On 06/08/2009 02:00 PM, Jeff Layton wrote:
>>>  static void
>>>  nfssvc_versbits(unsigned int ctlbits, int minorvers4)
>>>  {
>>> @@ -180,8 +246,11 @@ nfssvc(int port, int nrservs, unsigned int versbits, int minorvers4,
>>>  	 * 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(buf, sizeof(buf), "%d", port);
>>> +		nfssvc_set_sockets(AF_INET, protobits, haddr, portstr);
>> portstr is not defined here.... and as far as I can tell its not
>> defined in any of the next 3 patches either... 
>>
>> What am I missing or it is missing?
>>
>> steved.
>>
> 
> You're right. That's a spot that I missed when I converted that code to
> use the static buffer instead of allocating a memory all over the
> place. That "portstr" should be "buf" instead.
> 
> The good news is that a later patch removes that entire if statement.
> The final outcome of the set shouldn't be affected by this. Would you
> like me to respin that patch and the later one that removes that chunk
> of code?
> 
No Respin will be necessary... But in the future please make sure
each patch will compile and is somewhat functional... Also, if
possible, please order patches so if new code is added and then 
later removed, that code is never seen at all... 

steved.
   

Re: [PATCH 00/11] nfs-utils: add TIRPC/IPv6 support to rpc.nfsd

[Steve Dickson]

On 06/08/2009 02:00 PM, Jeff Layton wrote:
> This is the sixth attempt at a patchset to add TIRPC and IPv6 support to
> the rpc.nfsd program. The significant differences from the last set are:
> 
> 1) Change it to log errors to stderr by default, and add a --syslog
> switch to force output to syslog
> 
> 2) moved nfssvc.c into the nfsd directory (and out of libnfs.a).
> rpc.nfsd is the only user of those routines so there's no need to
> keep it in common code.
> 
> 3) declared a common static string buffer for the nfssvc routines
> to use to cut down stack usage. It's also smaller than BUFSIZ so the
> general memory footprint for rpc.nfsd should be smaller.
> 
> There are also a number of cleanups and small fixes.
> 
> Comments and suggestions appreciated...
> 
> Jeff Layton (11):
>   nfs-utils: move nfssvc.c to nfsd dir and clean up linking of nfsd
>   nfs-utils: clean up option parsing in nfsd.c
>   nfs-utils: convert rpc.nfsd to use xlog()
>   nfs-utils: clean up NFSCTL_* macros for handling protocol bits
>   nfs-utils: declare a static common buffer for nfssvc.c routines
>   nfs-utils: move check for active knfsd to helper function
>   nfs-utils: convert nfssvc_setfds to use getaddrinfo
>   nfs-utils: break up the nfssvc interface
>   nfs-utils: add IPv6 support to nfssvc_setfds
>   nfs-utils: add IPv6 support to nfsd
>   nfs-utils: update the nfsd manpage and copyright notices

Committed.... 

steved.