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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

On Mon, 8 Jun 2009 14:16:48 -0400
Chuck Lever <chuck.lever@xxxxxxxxxx> 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@xxxxxxxxxx>
---
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@xxxxxxxxxxxxxxx>

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



--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux