Date: Sun, 24 Apr 2011 08:47:46 -0400 (EDT) From: Rick Macklem <rmacklem@uoguelph.ca> To: Doug Barton <dougb@FreeBSD.org> Cc: rc@freebsd.org Subject: Re: rc scripts change for review Message-ID: <532445037.487687.1303649266310.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <4DB3AA8C.7090003@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] ----- Original Message ----- > On 04/23/2011 15:40, Rick Macklem wrote: > >> > >>> One thing I am not sure about is the REQUIRE: list in mountd. > >>> nfsserver and nfssrv essentially load the respective module. They > >>> are only > >>> used if sysctl variables need to be set and that's actually done > >>> by > >>> nfsd. > >>> (Should they be listed in mountd or nfsd or ???) > >> > >> It's not a good idea to add single scripts that only have a tiny > >> function, especially if they are only called conditionally (I.e., > >> they > >> contain KEYWORD: nostart). It's preferred to handle these things > >> things > >> in the script that needs them, or if it's necessary in more than > >> one > >> script to add a function to the appropriate .subr (rc, or network). > >> > >> So can you say a little more about what you're trying to > >> accomplish? > >> It's not clear to me why instead of doing this: > >> > >> + if ! sysctl vfs.newnfs>/dev/null 2>&1; then > >> + force_depend nfssrv || return 1 > >> + fi > >> > >> you would not just do this: > >> > >> + if ! sysctl vfs.newnfs>/dev/null 2>&1; then > >> + load_kld nfsd > >> + fi > >> > > Well, the intent of the above was to get the module loaded so that > > sysctl could manipulate its sysctl variables. I played with it a bit > > and it turns out that neither of the above code snippets work in the > > sense that they don't affect the outcome. > > > > What is needed to make the sysctls work is "nfssrv" has to be in the > > REQUIRED: list for either mountd or nfsd. Without it, the sysctls > > fail > > with unknown oid. (I'm guessing there is some delay between the > > load_kld > > and when the module gets its sysctl variables registered?) > > > > Now, I'm not sure whether there is any advantage to specifying > > "nfssrv" > > in nfsd or mountd, although both seem to work when I test them. > > "nfsserver" > > is in mountd, so unless you guys have a better suggestion, that's > > where > > I'll leave it. > > From what you're describing the reason it works by using the nfsserver > and nfssrv scripts is simply an accident of timing. That's not a good > thing no matter how you look at it. :) I understand your dilemma in > that the nfsserver "solution" was pre-existing, so you were just > following the example you had. However I have this odd idea that we > ought to fix broken code, not perpetuate it. Even if I weren't > interested in this for pedantic value, the fact that it's only working > now as an accident of timing is a good reason to fix it. > > The load_kld module is safe to run unconditionally (it will simply > return if the module is loaded). So what you could do (for both cases) > is something like this: > > load_kld nfsd > while ! sysctl vfs.newnfs >/dev/null 2>&1; do > sleep .5 > done > > If that works, I think we should add that feature to load_kld itself. I've updated the scripts at http://people.freebsd.org/~rmacklem/rc.conf and attached them, so you might not need to go there. Could you please take another look at them. This time nfsd uses the two load_klds and I tried to use "err", but I'm not sure if I checked for nfsuserd failing correctly. Thanks for your help, rick [-- Attachment #2 --] --- nfsd.orig 2011-04-20 18:49:08.000000000 -0400 +++ nfsd 2011-04-24 07:33:08.000000000 -0400 @@ -19,19 +19,8 @@ sig_stop="USR1" nfsd_precmd() { - 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 checkyesno oldnfs_server_enable; then + rc_flags="-o ${nfs_server_flags}" if ! sysctl vfs.nfsrv >/dev/null 2>&1; then force_depend nfsserver || return 1 @@ -41,6 +30,31 @@ nfsd_precmd() echo 'NFS on reserved port only=YES' sysctl vfs.nfsrv.nfs_privport=1 > /dev/null fi + else + rc_flags="${nfs_server_flags}" + + # Load the modules now, so that the vfs.newnfs sysctl + # oids are available. + load_kld nfscommon + load_kld nfsd + + if checkyesno nfs_reserved_port_only; then + echo 'NFS on reserved port only=YES' + sysctl vfs.newnfs.nfs_privport=1 > /dev/null + fi + + if checkyesno nfsv4_server_enable; then + if ! checkyesno nfsuserd_enable && \ + ! /etc/rc.d/nfsuserd forcestatus 1>/dev/null 2>&1 + then + if ! force_depend nfsuserd; then + err 1 "Cannot run nfsuserd" + fi + fi + else + echo 'NFSv4 is disabled' + sysctl vfs.newnfs.server_max_nfsvers=3 > /dev/null + fi fi if ! checkyesno rpcbind_enable && \ [-- Attachment #3 --] --- mountd.orig 2011-04-20 18:49:02.000000000 -0400 +++ mountd 2011-04-24 07:33:15.000000000 -0400 @@ -37,11 +37,10 @@ mountd_precmd() fi fi - # If nfsv4_server_enable is yes, force use of the experimental - # server + # If oldnfs_server_enable is yes, force use of the old NFS server # - if checkyesno nfsv4_server_enable; then - rc_flags="-e ${rc_flags}" + if checkyesno oldnfs_server_enable; then + rc_flags="-o ${rc_flags}" fi if checkyesno zfs_enable; then
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?532445037.487687.1303649266310.JavaMail.root>