Date: Sun, 2 Oct 2005 00:22:43 +0400 From: Yar Tikhiy <yar@comp.chem.msu.su> To: freebsd-rc@FreeBSD.org Subject: Re: Minor issues in our rcNG Message-ID: <20051001202243.GA36764@comp.chem.msu.su> In-Reply-To: <433CE171.8080301@FreeBSD.org> References: <20050928164803.GA11556@comp.chem.msu.su> <433CE171.8080301@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Sep 29, 2005 at 11:55:45PM -0700, Doug Barton wrote:
>
> Ok, so, this is going to seem like "pick on Yar" night, but it really isn't
> meant to be. :)
Well, I spent some effort on making that night such, too :-)
> We have a list for discussion of issues related to the rc.d system (it's no
> longer referred to as rcNG), so I'm cc'ing that list, and would ask that you
> follow up there instead of -arch.
On the one hand, I'm sorry that I missed the proper list. On the other
hand, I'm kind of glad to have attracted some attention to the -rc list,
which seems to have been almost ignored by the community lately :-)
> Yar Tikhiy wrote:
> |
> | The larger issue I'd like to discuss is as follows. Presently,
> | ${<name>_program} variables are special in that they always override
> | ${command}. Some rc.d scripts (ab)use this to skip setting ${command},
> | e.g., sshd. Some other scripts (ab)use ${<name>_program} despite
> | they are not just starting ${command} once, e.g., pf, and so make
> | rc.subr think they have functionality that isn't realy there, such as
> | poll. Perhaps it's time to separate these two cases in a way?
>
> I think that here, and in the rest of the message you've stated the problem
> you're concerned about well, but I confess that I'm not 100% sure what the
> negative affects of this problem are. Sorry if I'm being dense here, but
> before I get excited about this I want to be sure I understand the problem.
The main reason for my being concerned is as follows. I met rc.d,
rc.subr and friends quite recently and found that it was a subsystem
of a clean design, relatively unpolluted with bugs, undocumented
features, or ill side-effects, unlike many other parts of the system.
I got so happy with it that even began an article on how to write
rc.d scripts (it's mostly complete now.) While writing the article,
I had to look deeper into the area, and of course I noticed some
oddities, which I'd be glad to fix just to keep this nice little
territory free of code junk.
This particular issue seems important to me because there are
essentially two kinds of variables for an rc.d script to use:
those with special meaning to rc.subr(8) and those without such
special meaning. IMO, it would be unwise to confuse these kinds
of variables and ignore possible ill side-effects.
> | I can see two possible approaches. One is to require scripts just
> | starting a daemon always set ${command}, and to apply the attached
> | patch to rc.subr. With the patch, ${command} is overridden only
> | if it was set in the first place. The other approach is to prevent
> | non-daemon scripts from abusing ${<name>_program}. However, in the
> | latter case defining ${<name>_program}, e.g., by mistake, still can
> | affect the script while it should not.
> |
> | The second issue is rather small. The rc command "reload" is
> | supported by code in /etc/rc.subr, but it doesn't appear on the
> | default list of commands unlike "status" or "poll" when there is
> | ${pidfile} or ${command} set, and so it is unusable by default.
> | Scripts have to use ``extra_commands=reload''. In addition, all
> | this is undocumented. Should "reload" be added to the list of
> | available commands along with "status" and "poll"?
I reconsidered the second issue, and the following idea came to my
mind. The commands "status" and "poll" can be performed by rc.subr
for any rc.d subsystem providing $command because rc.subr can see
if the process still is running given as little as the process name.
OTOH, "reload" makes sense only in case the process will act
respectively upon receiving SIGHUP or another signal, e.g., re-read
its configuration files. While this is common, not all daemons do
this. Therefore it may be reasonable to keep "reload" out of the
default commands. Scripts for subsystems that can do something
like reload using the default method will have to specify "reload"
in $extra_commands, as it is done already. The whole thing just
needs documenting.
> I think this sounds reasonable. Do you have patches for this?
Patch attached below.
> | Finally, we have a script named rcconf.sh which doesn't seem
> | to do anything useful now since all the other scripts invoke
> | load_rc_config by their own. Can we drop it then?
>
> I wouldn't lose sleep over this one, I'm planning to deal with it when I
> introduce the changes for rcorder in /usr/local/etc/rc.d.
Was just nitpicking...
--
Yar
--- //depot/vendor/freebsd/src/etc/rc.d/ike 2004/10/07 14:36:29
+++ //depot/user/yar/hack/etc/rc.d/ike 2005/10/01 16:51:12
@@ -12,6 +12,7 @@
name="ike"
rcvar=`set_rcvar`
+command="/usr/local/sbin/isakmpd"
extra_commands="reload"
load_rc_config $name
--- //depot/vendor/freebsd/src/etc/rc.d/named 2005/05/23 12:37:09
+++ //depot/user/yar/hack/etc/rc.d/named 2005/10/01 16:51:12
@@ -13,6 +13,7 @@
name="named"
rcvar=`set_rcvar`
+command="/usr/sbin/named"
start_precmd="named_precmd"
start_postcmd="make_symlinks"
stop_postcmd="named_poststop"
--- //depot/vendor/freebsd/src/etc/rc.d/ppp-user 2004/12/15 13:36:59
+++ //depot/user/yar/hack/etc/rc.d/ppp-user 2005/10/01 08:23:26
@@ -11,6 +11,7 @@
name="ppp"
rcvar=`set_rcvar`
+command="/usr/sbin/ppp"
start_precmd="ppp_precmd"
start_postcmd="ppp_postcmd"
--- //depot/vendor/freebsd/src/etc/rc.d/sshd 2005/01/16 03:37:14
+++ //depot/user/yar/hack/etc/rc.d/sshd 2005/10/01 16:51:12
@@ -11,6 +11,7 @@
name="sshd"
rcvar=`set_rcvar`
+command="/usr/sbin/sshd"
keygen_cmd="sshd_keygen"
start_precmd="sshd_precmd"
pidfile="/var/run/${name}.pid"
--- //depot/vendor/freebsd/src/etc/rc.subr 2005/09/19 21:36:16
+++ //depot/user/yar/hack/etc/rc.subr 2005/09/19 23:56:24
@@ -493,9 +493,8 @@
esac
eval _overide_command=\$${name}_program
- if [ -n "$_overide_command" ]; then
- command=$_overide_command
- fi
+ _overide_command=${_overide_command:-$command}
+ command=${command:+$_overide_command}
_keywords="start stop restart rcvar $extra_commands"
rc_pid=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20051001202243.GA36764>