Date: Fri, 10 Feb 2012 12:03:56 +0100 From: Jilles Tjoelker <jilles@stack.nl> To: Doug Barton <dougb@FreeBSD.org> Cc: freebsd-rc@FreeBSD.org Subject: Re: Bringing sanity to the RPC/NFS related scripts Message-ID: <20120210110356.GA6723@stack.nl> In-Reply-To: <4F33031E.7000507@FreeBSD.org> References: <4F322437.5030100@FreeBSD.org> <20120208230852.GA83950@stack.nl> <4F33031E.7000507@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120210110356.GA6723>