I'm copying delphij since this is in part related to the patch he posted
back in August. I had a chance to review that in detail (thanks Jilles
for the pointer) and I agree that his approach is the right one, it just
doesn't go far enough. :)
The attached patch encompasses his work (with modifications as described
below), my previous patch, the idea I had for the always_force_depends
knob, and the change requested by Jilles.
I also sorted the first few vars in the yp* scripts ... they were in
various weird orders, with the problem being that rcvar was often set
after load_rc_config.
The idea of putting the logic for testing whether or not the service is
enabled and the forcestatus bit into a function is definitely the right
way to go. delphij's original patch cut out a lot of code from the rc.d
scripts, and this patch cuts out even more. Hopefully making
force_depend both more useful and easier to use will encourage more use
of it.
I did take one slightly different choice with the syntax for overloading
force_depend .... rather than having to feed it the full rcvar (like
foo_bar_enable) I changed it to leave off the _enable bit in the
argument since it's redundant to include it every time.
Doug
On 02/10/2012 03:03, Jilles Tjoelker wrote:
> 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.
>
--
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/
Index: share/man/man5/rc.conf.5
===================================================================
--- share/man/man5/rc.conf.5 (revision 231503)
+++ share/man/man5/rc.conf.5 (working copy)
@@ -24,7 +24,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd February 8, 2012
+.Dd February 11, 2012
.Dt RC.CONF 5
.Os
.Sh NAME
@@ -149,6 +149,19 @@
adequate provisions to recover from a failed boot
(such as physical contact with the machine,
or reliable remote console access).
+.It Va always_force_depends
+.Pq Vt bool
+Various
+.Pa rc.d
+scripts use the force_depend function to check whether required
+services are already running, and to start them if necessary.
+By default during boot time this check is bypassed if the
+required service is enabled in
+.Pa /etc/rc.conf[.local] .
+Setting this option will bypass that check at boot time and
+always test whether or not the service is actually running.
+Enabling this option is likely to increase your boot time if
+services are enabled that utilize the force_depend check.
.It Va swapfile
.Pq Vt str
If set to
Index: etc/defaults/rc.conf
===================================================================
--- etc/defaults/rc.conf (revision 231503)
+++ etc/defaults/rc.conf (working copy)
@@ -29,6 +29,8 @@
# stages of the boot process. Make sure you know
# the ramifications if you change this.
# See rc.conf(5) for more details.
+always_force_depends="NO" # Set to check that indicated dependencies are
+ # running during boot (can increase boot time).
swapfile="NO" # Set to name of swapfile if aux swapfile desired.
apm_enable="NO" # Set to YES to enable APM BIOS functions (or
NO).
Index: etc/rc.d/ypset
===================================================================
--- etc/rc.d/ypset (revision 231503)
+++ etc/rc.d/ypset (working copy)
@@ -11,25 +11,20 @@
name="ypset"
rcvar="nis_ypset_enable"
+
+load_rc_config $name
+
command="/usr/sbin/${name}"
-start_precmd="ypset_precmd"
-load_rc_config $name
command_args="${nis_ypset_flags}"
+start_precmd="ypset_precmd"
+
ypset_precmd()
{
local _domain
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
- if ! checkyesno nis_client_enable && \
- ! /etc/rc.d/ypbind forcestatus 1>/dev/null 2>&1
- then
- force_depend ypbind || return 1
- fi
+ force_depend rpcbind || return 1
+ force_depend ypbind nis_client || return 1
_domain=`domainname`
if [ -z "$_domain" ]; then
Index: etc/rc.d/mountd
===================================================================
--- etc/rc.d/mountd (revision 231503)
+++ etc/rc.d/mountd (working copy)
@@ -19,11 +19,7 @@
mountd_precmd()
{
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
+ force_depend rpcbind || return 1
# mountd flags will differ depending on rc.conf settings
#
@@ -48,8 +44,8 @@
fi
rm -f /var/db/mountdtab
- ( umask 022 ; > /var/db/mountdtab )
- return 0
+ ( umask 022 ; > /var/db/mountdtab ) ||
+ err 1 'Cannot create /var/db/mountdtab'
}
load_rc_config $name
Index: etc/rc.d/yppasswdd
===================================================================
--- etc/rc.d/yppasswdd (revision 231503)
+++ etc/rc.d/yppasswdd (working copy)
@@ -11,27 +11,22 @@
. /etc/rc.subr
name="yppasswdd"
-command="/usr/sbin/rpc.${name}"
-start_precmd="yppasswdd_precmd"
+rcvar="nis_yppasswdd_enable"
load_rc_config $name
-rcvar="nis_yppasswdd_enable"
+
+command="/usr/sbin/rpc.${name}"
command_args="${nis_yppasswdd_flags}"
+start_precmd="yppasswdd_precmd"
+
yppasswdd_precmd()
{
local _domain
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
- if ! checkyesno nis_server_enable && \
- ! /etc/rc.d/ypserv forcestatus 1>/dev/null 2>&1
- then
- force_depend ypserv || return 1
- fi
+ force_depend rpcbind || return 1
+ force_depend ypserv nis_server || return 1
+
_domain=`domainname`
if [ -z "$_domain" ]; then
warn "NIS domainname(1) is not set."
Index: etc/rc.d/ypupdated
===================================================================
--- etc/rc.d/ypupdated (revision 231503)
+++ etc/rc.d/ypupdated (working copy)
@@ -11,6 +11,9 @@
name="ypupdated"
rcvar="rpc_ypupdated_enable"
+
+load_rc_config $name
+
command="/usr/sbin/rpc.${name}"
start_precmd="rpc_ypupdated_precmd"
@@ -18,16 +21,8 @@
{
local _domain
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
- if ! checkyesno nis_server_enable && \
- ! /etc/rc.d/ypserv forcestatus 1>/dev/null 2>&1
- then
- force_depend ypserv || return 1
- fi
+ force_depend rpcbind || return 1
+ force_depend ypserv nis_server || return 1
_domain=`domainname`
if [ -z "$_domain" ]; then
@@ -36,5 +31,4 @@
fi
}
-load_rc_config $name
run_rc_command "$1"
Index: etc/rc.d/nfsd
===================================================================
--- etc/rc.d/nfsd (revision 231503)
+++ etc/rc.d/nfsd (working copy)
@@ -48,31 +48,15 @@
if checkyesno nfsv4_server_enable; then
sysctl vfs.nfsd.server_max_nfsvers=4 > /dev/null
- 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
+ force_depend nfsuserd || err 1 "Cannot run nfsuserd"
else
echo 'NFSv4 is disabled'
sysctl vfs.nfsd.server_max_nfsvers=3 > /dev/null
fi
fi
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
-
- if ! checkyesno mountd_enable && \
- ! /etc/rc.d/mountd forcestatus 1>/dev/null 2>&1
- then
- force_depend mountd || return 1
- fi
- return 0
+ force_depend rpcbind || return 1
+ force_depend mountd || return 1
}
run_rc_command "$1"
Index: etc/rc.d/ypbind
===================================================================
--- etc/rc.d/ypbind (revision 231503)
+++ etc/rc.d/ypbind (working copy)
@@ -11,22 +11,20 @@
. /etc/rc.subr
name="ypbind"
-command="/usr/sbin/${name}"
-start_precmd="ypbind_precmd"
+rcvar="nis_client_enable"
load_rc_config $name
-rcvar="nis_client_enable"
+
+command="/usr/sbin/${name}"
command_args="${nis_client_flags}"
+start_precmd="ypbind_precmd"
+
ypbind_precmd()
{
local _domain
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
+ force_depend rpcbind || return 1
_domain=`domainname`
if [ -z "$_domain" ]; then
Index: etc/rc.d/ypserv
===================================================================
--- etc/rc.d/ypserv (revision 231503)
+++ etc/rc.d/ypserv (working copy)
@@ -11,21 +11,20 @@
name="ypserv"
rcvar="nis_server_enable"
-command="/usr/sbin/${name}"
-start_precmd="ypserv_prestart"
load_rc_config $name
+
+command="/usr/sbin/${name}"
command_args="${nis_server_flags}"
+start_precmd="ypserv_prestart"
+
ypserv_prestart()
{
local _domain
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
+ force_depend rpcbind || return 1
+
_domain=`domainname`
if [ -z "$_domain" ]; then
warn "NIS domainname(1) is not set."
Index: etc/rc.d/ypxfrd
===================================================================
--- etc/rc.d/ypxfrd (revision 231503)
+++ etc/rc.d/ypxfrd (working copy)
@@ -11,25 +11,20 @@
name="ypxfrd"
rcvar="nis_ypxfrd_enable"
+
+load_rc_config $name
+
command="/usr/sbin/rpc.${name}"
-start_precmd="ypxfrd_precmd"
-load_rc_config $name
command_args="${nis_ypxfrd_flags}"
+start_precmd="ypxfrd_precmd"
+
ypxfrd_precmd()
{
local _domain
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
- if ! checkyesno nis_server_enable && \
- ! /etc/rc.d/ypserv forcestatus 1>/dev/null 2>&1
- then
- force_depend ypserv || return 1
- fi
+ force_depend rpcbind || return 1
+ force_depend ypserv nis_server || return 1
_domain=`domainname`
if [ -z "$_domain" ]; then
Index: etc/rc.d/amd
===================================================================
--- etc/rc.d/amd (revision 231503)
+++ etc/rc.d/amd (working copy)
@@ -19,16 +19,9 @@
amd_precmd()
{
- if ! checkyesno nfs_client_enable; then
- force_depend nfsclient || return 1
- fi
+ force_depend nfsclient nfs_client || return 1
+ force_depend rpcbind || return 1
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
-
case ${amd_map_program} in
[Nn][Oo] | '')
;;
@@ -49,7 +42,6 @@
command_args="> /var/run/amd.pid 2> /dev/null"
;;
esac
- return 0
}
load_rc_config $name
Index: etc/rc.d/keyserv
===================================================================
--- etc/rc.d/keyserv (revision 231503)
+++ etc/rc.d/keyserv (working copy)
@@ -19,13 +19,7 @@
keyserv_prestart()
{
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || return 1
- fi
-
- return 0
+ force_depend rpcbind || return 1
}
load_rc_config $name
Index: etc/rc.d/apmd
===================================================================
--- etc/rc.d/apmd (revision 231503)
+++ etc/rc.d/apmd (working copy)
@@ -19,24 +19,18 @@
{
case `${SYSCTL_N} hw.machine_arch` in
i386)
- # Enable apm if it is not already enabled
- if ! checkyesno apm_enable && \
- ! /etc/rc.d/apm forcestatus 1>/dev/null 2>&1
- then
- force_depend apm || return 1
- fi
+ force_depend apm || return 1
# Warn user about acpi apm compatibility support which
# does not work with apmd.
if [ ! -e /dev/apmctl ]; then
- warn "/dev/apmctl not found; kernel is missing apm(4)"
+ warn "/dev/apmctl not found; kernel is missing apm(4)"
fi
;;
*)
return 1
;;
esac
- return 0
}
load_rc_config $name
Index: etc/rc.d/lockd
===================================================================
--- etc/rc.d/lockd (revision 231503)
+++ etc/rc.d/lockd (working copy)
@@ -15,28 +15,16 @@
rcvar=rpc_lockd_enable
command="/usr/sbin/rpc.${name}"
start_precmd='lockd_precmd'
-stop_precmd='checkyesno nfs_server_enable || checkyesno nfs_client_enable'
-status_precmd=$stop_precmd
# Make sure that we are either an NFS client or server, and that we get
# the correct flags from rc.conf(5).
#
lockd_precmd()
{
- local ret
- ret=0
-
- if ! checkyesno nfs_server_enable && ! checkyesno nfs_client_enable
- then
- ret=1
- fi
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || ret=1
- fi
+ force_depend rpcbind || return 1
+ force_depend statd rpc_statd || return 1
+
rc_flags=${rpc_lockd_flags}
- return ${ret}
}
load_rc_config $name
Index: etc/rc.d/statd
===================================================================
--- etc/rc.d/statd (revision 231503)
+++ etc/rc.d/statd (working copy)
@@ -15,28 +15,15 @@
rcvar=rpc_statd_enable
command="/usr/sbin/rpc.${name}"
start_precmd='statd_precmd'
-stop_precmd='checkyesno nfs_server_enable || checkyesno nfs_client_enable'
-status_precmd=$stop_precmd
# Make sure that we are either an NFS client or server, and that we get
# the correct flags from rc.conf(5).
#
statd_precmd()
{
- local ret
- ret=0
-
- if ! checkyesno nfs_server_enable && ! checkyesno nfs_client_enable
- then
- ret=1
- fi
- if ! checkyesno rpcbind_enable && \
- ! /etc/rc.d/rpcbind forcestatus 1>/dev/null 2>&1
- then
- force_depend rpcbind || ret=1
- fi
+ force_depend rpcbind || return 1
+
rc_flags=${rpc_statd_flags}
- return ${ret}
}
load_rc_config $name
Index: etc/rc.subr
===================================================================
--- etc/rc.subr (revision 231503)
+++ etc/rc.subr (working copy)
@@ -71,22 +71,29 @@
}
#
-# force_depend script
+# force_depend script [rcvar]
# Force a service to start. Intended for use by services
-# to resolve dependency issues. It is assumed the caller
-# has check to make sure this call is necessary
+# to resolve dependency issues.
# $1 - filename of script, in /etc/rc.d, to run
+# $2 - name of the script's rcvar (minus the _enable)
#
force_depend()
{
+ local _depend _dep_rcvar
+
_depend="$1"
+ _dep_rcvar="${2:-$1}_enable"
+ [ -n "$rc_fast" ] && ! checkyesno always_force_depends &&
+ checkyesno $_dep_rcvar && return 0
+
+ /etc/rc.d/${_depend} forcestatus >/dev/null 2>&1 && return 0
+
info "${name} depends on ${_depend}, which will be forced to start."
if ! /etc/rc.d/${_depend} forcestart; then
warn "Unable to force ${_depend}. It may already be running."
return 1
fi
- return 0
}
#
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-rc
To unsubscribe, send any mail to "[email protected]"
