On Jun 2, 2009, at 2:55 PM, Jeff Layton wrote:
On Tue, 2 Jun 2009 11:35:18 -0400
Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
Forgot a couple of comments from the earlier email:
case 'H':
- if (inet_addr(optarg) != INADDR_NONE) {
+ /*
+ * for now, this only handles one -H option. Use the
+ * first one specified.
+ */
Interesting comment. Did the old version allow you to specify more
than one? Can you run rpc.nfsd more than once, specifying a
different
"-H" on each?
The only option that matters once nfsd is up is the number of threads
-- any others are ignored. This applies to the -H option as well, so
it'll only matter for the first run of rpc.nfsd. You could specify -H
multiple times on a single nfsd run, but only the last one would
matter. With this patch, I changed it to be the first one since that
seemed more intuitive to me.
Everything else I'm aware of (like mount.nfs) uses "rightmost wins."
So the last "-H" would be the one that takes effect. One way might be
more intuitive than another, but it seems like everything else takes
the last command-line option, not the first.
We could eventually allow nfsd to take multiple -H arguments, but
that's really outside the scope of this set.
Ignoring all but one makes sense. The comment suggested that rpc.nfsd
might have taken more than one at one point.
+
+ xlog_open("nfsd");
How about "rpc.nfsd" or even argv[0] ?
rpc.nfsd is probably easier. basename(argv[0]) is probably the best
thing though. I'll fix it up to use that.
+ /*
+ * 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
+ */
xlog_syslog(1);
xlog_stderr(0);
fd = open("/dev/null", O_RDWR);
if (fd == -1)
- perror("/dev/null");
+ xlog(L_ERROR, "unable to open /dev/null: %s", strerror(errno));
("%m") does the same thing as ("%s", strerror(errno))
Nice...ok I'll change it (and other spots if there are any).
else {
(void) dup2(fd, 0);
(void) dup2(fd, 1);
Should you check the return code from these?
The existing code doesn't. If we want to change that it should prob be
a separate patch. I wonder though whether we need to do all of this
stuff with /dev/null. Stupid question: is there some reason we can't
just close stdin/stdout/stderr?
It's hard to say what's right. openlog(3) creates an open file
descriptor, for instance, so that would be inherited by the kernel in
this case if closelog(3) wasn't done. The question is whether we
still care about kernels that old.
I think closing them all, and re-opening if we need to report an
error, is OK. I saw some evidence recently that closeall() wasn't
working at all, so you might want to check that.
--
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
