Bringing sanity to the RPC/NFS related scripts

Jilles Tjoelker jilles at stack.nl
Fri Feb 10 11:03:59 UTC 2012


On Wed, Feb 08, 2012 at 03:19:58PM -0800, Doug Barton wrote:
> Thanks for the review, comments below, with snipping.

> On 02/08/2012 15:08, Jilles Tjoelker wrote:
> > On Tue, Feb 07, 2012 at 11:28:55PM -0800, Doug Barton wrote:

> >> and b) the test for "is it running?" is conditional on it not
> >> being _enable'd, which is counterproductive for a couple of reasons. (I
> >> can elaborate if necessary, but hopefully it's obvious?) So I'd like to
> >> propose removing the _enable check from all the relevant scripts that
> >> have this force_depend capability.  For users who already have _enable
> >> for these services it will cause one extra call to forcestatus for them,
> >> but given that rc.d currently has no other way to ensure that required
> >> dependencies are running, I think it's worth it.

> > This was discussed in August 2011 but no patch was committed. See
> > http://lists.freebsd.org/pipermail/freebsd-rc/2011-August/002412.html

> > The existing code makes a lot of sense for the case [ -n "${rc_fast}" ]
> > (avoiding unnecessary slow checks for processes at boot) but may be less
> > convenient for starting such services manually. The patch appears to fix
> > the manual start case without slowing down boot, unlike bluntly removing
> > checkyesno which will slow down boot.

> I understand the motivation not to slow down the boot, however the
> problems we're seeing with "random" statd failures are at boot time. So
> perhaps the right answer is to include the fast_depend idea but with an
> override to always do the check.

Hmm, so statd sometimes does not feel like starting the first time, but
is willing the second time? That would be a bug that should not be
worked around with that extra check.

> Also, I've only taken a cursory glance at the patch (I'll have more time
> to review it later) but it seems to me that rather than introducing a
> new function it would be better to have force_depend test for $rc_fast
> (and it could then also test for the override knob I'd like to add). Any
> objections to that?

No objection.

> >> Index: mountd
> >> ===================================================================
> >> --- mountd	(revision 231185)
> >> +++ mountd	(working copy)
> >> [snip]
> >> @@ -49,7 +47,6 @@
> >>  
> >>  	rm -f /var/db/mountdtab
> >>  	( umask 022 ; > /var/db/mountdtab )
> >> -	return 0
> >>  }
> >>  
> >>  load_rc_config $name

> > Note that this changes the return value of mountd_precmd if
> > /var/db/mountdtab cannot be created. Is this deliberate?

> Yes, since mountd relies on that. Do you think it's overkill? Perhaps it
> would be better to add an '|| err ...' to explain the failure?

That's probably safer, since a subsequent modification may not take into
account that the redirection is deliberately last.

If this change is conscious, that's fine.

-- 
Jilles Tjoelker


More information about the freebsd-rc mailing list