Date: Wed, 08 Feb 2012 15:19:58 -0800 From: Doug Barton <dougb@FreeBSD.org> To: Jilles Tjoelker <jilles@stack.nl> Cc: freebsd-rc@FreeBSD.org Subject: Re: Bringing sanity to the RPC/NFS related scripts Message-ID: <4F33031E.7000507@FreeBSD.org> In-Reply-To: <20120208230852.GA83950@stack.nl> References: <4F322437.5030100@FreeBSD.org> <20120208230852.GA83950@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
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. 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? > If you're touching the code anyway, it would make sense to change > 1>/dev/null to >/dev/null. Agreed, that one slipped by when I was folding the lines, thanks for catching it. >> 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? Doug -- It's always a long day; 86400 doesn't fit into a short. Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4F33031E.7000507>