Date: Sat, 30 May 2009 12:17:17 -0700 From: Doug Barton <dougb@FreeBSD.org> To: Rick Macklem <rick@snowhite.cis.uoguelph.ca> Cc: kib@FreeBSD.org, rc@FreeBSD.org, rwatson@FreeBSD.org Subject: Re: rc scripts for the new experimental nfs subsystem Message-ID: <4A21863D.8040904@FreeBSD.org> In-Reply-To: <200905291652.MAA15699@snowhite.cis.uoguelph.ca> References: <200905291652.MAA15699@snowhite.cis.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] Rick Macklem wrote: >> cursory inspection. Please review >> http://www.freebsd.org/doc/en_US.ISO8859-1/articles/rc-scripting/ as well. > > Thanks. Very helpful. GTH. > Ok, I've shortened nfsv4_serversupport_enable to nfsv4_server_enable and > changed the names of the others to follow the FreeBSD tradition (now that > I know about it:-). Well, that's what review is for, right? :) > I also made the new scripts follow the traditions, I > think? (I also simplified the if in nfsd, since I thought that's what > you meant by "shorten" the first time I read the email. I think it's more > readable this way, anyhow.) > > The "REQUIRE: NETWORKING" is debatable for nfsuserd, since it doesn't > use networking itself. However, it calls functions like getpwent(), which > usually use LDAP, NIS,... these days. Also, it's used by NFS, which is > useless without networking, so I thought it made sense? It sounds like what you really want to do is have nfsd REQUIRE nfsuserd. I'll only comment on a few things, the rest looks good. > diff -u -N -r rc.d.sav/nfscbd rc.d/nfscbd > --- rc.d.sav/nfscbd 1969-12-31 19:00:00.000000000 -0500 > +++ rc.d/nfscbd 2009-05-29 11:55:38.000000000 -0400 > @@ -0,0 +1,19 @@ > +#!/bin/sh > +# > +# $FreeBSD$ > +# > + > +# PROVIDE: nfscbd > +# REQUIRE: NETWORKING > +# KEYWORD: nojail shutdown > + > +. /etc/rc.subr > + > +name="nfscbd" > +rcvar=`set_rcvar` > +command="/usr/sbin/${name}" > + > +load_rc_config $name > +sig_stop="USR1" For consistency sake sig_stop should really be moved up between command and load_rc_config, but this is just cosmetic. > diff -u -N -r rc.d.sav/nfsd rc.d/nfsd I'm not sure why you added so many tests for the new variable. I've attached a streamlined version of your patch which I think will accomplish the same things. > diff -u -N -r rc.d.sav/nfsuserd rc.d/nfsuserd > --- rc.d.sav/nfsuserd 1969-12-31 19:00:00.000000000 -0500 > +++ rc.d/nfsuserd 2009-05-29 11:55:38.000000000 -0400 > @@ -0,0 +1,19 @@ > +#!/bin/sh > +# > +# $FreeBSD$ > +# > + > +# PROVIDE: nfsuserd > +# REQUIRE: NETWORKING > +# KEYWORD: nojail shutdown > + > +. /etc/rc.subr > + > +name="nfsuserd" > +rcvar=`set_rcvar` > +command="/usr/sbin/${name}" > + > +load_rc_config $name > +sig_stop="USR1" Same here with sig_stop. hope this helps, Doug -- This .signature sanitized for your protection [-- Attachment #2 --] Index: nfsd =================================================================== --- nfsd (revision 192991) +++ nfsd (working copy) @@ -14,14 +14,33 @@ command="/usr/sbin/${name}" load_rc_config $name -command_args="${nfs_server_flags}" start_precmd="nfsd_precmd" sig_stop="USR1" nfsd_precmd() { - if ! sysctl vfs.nfsrv >/dev/null 2>&1; then - force_depend nfsserver || return 1 + if checkyesno nfsv4_server_enable; then + # If nfsv4_server_enable is yes, force use + # of the experimental server + # + rc_flags="-e ${nfs_server_flags}" + + if ! checkyesno nfsuserd_enable && \ + ! /etc/rc.d/nfsuserd forcestatus 1>/dev/null 2>&1 + then + force_depend nfsuserd || return 1 + fi + else + rc_flags="${nfs_server_flags}" + + if ! sysctl vfs.nfsrv >/dev/null 2>&1; then + force_depend nfsserver || return 1 + fi + + if checkyesno nfs_reserved_port_only; then + echo 'NFS on reserved port only=YES' + sysctl vfs.nfsrv.nfs_privport=1 > /dev/null + fi fi if ! checkyesno rpcbind_enable && \ @@ -35,11 +54,6 @@ then force_depend mountd || return 1 fi - - if checkyesno nfs_reserved_port_only; then - echo 'NFS on reserved port only=YES' - sysctl vfs.nfsrv.nfs_privport=1 > /dev/null - fi return 0 }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A21863D.8040904>