Re: svn commit: r234093 - in head/etc: defaults rc.d

2012-04-11 Thread Doug Barton
This script has many problems. First, new scripts in the base should 
never use BEFORE since this makes debugging rcorder issues harder. 
Please instead add this to the REQUIRE line in DAEMON. You can use 
'service -r' to get an idea of where it will be included at boot time.


Second, for new scripts the name of the file, $name, and PROVIDE should 
all the the same, in this case kfd. That implies that the rcvar should 
be kfd_enable unless there is a very good reason for it to be 
different, which in this case there is not one that I can see. (FYI, 
the d in kfd implies server.)


Scripts that start persistent services should always include KEYWORD: 
shutdown so that they get started cleanly. It's not clear to me if this 
script should also include the nojail keyword. It's also generally a 
good idea to add a REQUIRE line unless it truly doesn't matter. I 
guessed at REQUIRE: kerberos ala the kadamind script, if that's wrong 
please let me know.


Next, the arguments in the script, and the script generally, don't
follow the default format. It's very helpful when doing mass 
reviews/updates if the script looks the same as other similar scripts 
unless there is a good reason for it to be different. The reference at 
http://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/rc-scripts.html 
should help.


It's also unclear why you are unset'ing start_cmd, but not supplying 
your own start method. The way that you're using kfd_server is also 
totally wrong, as is hard-coding kfd_flags in the script. If the -i 
argument is always needed, it should be included in command_args. If 
it's not always needed, but is generally recommended, you should include 
it in /etc/defaults/rc.conf so that the user can easily override it. 
I've assumed the latter, if it should always be included please let me 
know.


Finally, you neglected to update rc.conf.5.

The attached patch fixes the problems mentioned above, modulo the 
rc.conf.5 update which I'll leave to you. In order to avoid 
inconvenience to those tracking HEAD who want to use this new feature I 
plan to commit this in the next couple of days if I don't hear from you.


In the future it would probably be a good idea to submit patches to 
freebsd-rc@ for review.


hth,

Doug


On Tue, 10 Apr 2012, Stanislav Sedov wrote:


Author: stas
Date: Tue Apr 10 09:27:41 2012
New Revision: 234093
URL: http://svn.freebsd.org/changeset/base/234093

Log:
 - Add rc.d script for kfd, kerberos forwarded tickets daemon.

Added:
 head/etc/rc.d/kfd   (contents, props changed)
Modified:
 head/etc/defaults/rc.conf
 head/etc/rc.d/Makefile

Modified: head/etc/defaults/rc.conf
==
--- head/etc/defaults/rc.conf   Tue Apr 10 07:38:58 2012(r234092)
+++ head/etc/defaults/rc.conf   Tue Apr 10 09:27:41 2012(r234093)
@@ -297,6 +297,8 @@ kadmind5_server_enable=NO   # Run kadmin
kadmind5_server=/usr/libexec/kadmind# path to kerberos 5 admin daemon
kpasswdd_server_enable=NO   # Run kpasswdd (or NO)
kpasswdd_server=/usr/libexec/kpasswdd   # path to kerberos 5 passwd daemon
+kfd_server_enable=NO   # Run kfd (or NO)
+kfd_server=/usr/libexec/kfd# path to kerberos 5 kfd daemon

gssd_enable=NO  # Run the gssd daemon (or NO).
gssd_flags= # Flags for gssd.

Modified: head/etc/rc.d/Makefile
==
--- head/etc/rc.d/Makefile  Tue Apr 10 07:38:58 2012(r234092)
+++ head/etc/rc.d/Makefile  Tue Apr 10 09:27:41 2012(r234093)
@@ -66,6 +66,7 @@ FILES=DAEMON \
kadmind \
kerberos \
keyserv \
+   kfd \
kld \
kldxref \
kpasswdd \

Added: head/etc/rc.d/kfd
==
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/etc/rc.d/kfd   Tue Apr 10 09:27:41 2012(r234093)
@@ -0,0 +1,19 @@
+#!/bin/sh
+#
+# $FreeBSD$
+#
+
+# PROVIDE: kfd
+# BEFORE: DAEMON
+
+. /etc/rc.subr
+
+name=kfd
+load_rc_config $name
+rcvar=kfd_server_enable
+unset start_cmd
+command=${kfd_server}
+kfd_flags=-i
+command_args=
+
+run_rc_command $1





--

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: defaults/rc.conf
===
--- defaults/rc.conf(revision 234164)
+++ defaults/rc.conf(working copy)
@@ -297,8 +297,9 @@
 kadmind5_server=/usr/libexec/kadmind # path to kerberos 5 admin daemon
 kpasswdd_server_enable=NO# Run kpasswdd (or NO)
 kpasswdd_server=/usr/libexec/kpasswdd# path to kerberos 5 passwd 
daemon
-kfd_server_enable=NO # Run kfd (or NO)
-kfd_server=/usr/libexec/kfd  # path to kerberos 5 

Re: svn commit: r234093 - in head/etc: defaults rc.d

2012-04-11 Thread Doug Barton

On Wed, 11 Apr 2012, Doug Barton wrote:

Scripts that start persistent services should always include KEYWORD: 
shutdown so that they get started cleanly.


Sorry, that's so that they get shut down cleanly.

Doug
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org