rc scripts change for review

[Rick Macklem]

> On Sat, Apr 23, 2011 at 09:43:56PM -0700, Doug Barton wrote:
> > 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.
> >

Agreed. I was thinking the same thing.

> > 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.
> 
> This is a kludge, hiding some bug. Sysctl oids are registered
> synchronously,
> and kldload(2) syscall loads the module in context of the called
> thread.
> So if MOD_LOAD even handler registered its oids, then usermode and
> sysctl(8)
> should see them without arbitrary wait.

One possible explanation here (and it may be that I haven't set the
modules/sysctl stuff up correctly?) is that the "nfsd" module depends
on the module "nfscommon" (which is loaded when nfsd is loaded, because
of a MODULE_DEPEND() in "nfsd"). The SYSCTL_NODE() is in "nfscommon".
I only have a SYSCTL_DECL() in "nfsd". (ie. maybe kldload doesn't wait
for "nfscommon" to be loaded.)

I just tried:
load_kld nfscommon
load_kld nfsd
- and this works without needing "nfssrv"

I'm not sure if the above implies a bug in either my server code or
the module loading/sysctl registering stuff, but I'm confortable with
doing the 2 load_kld's.

rick