From owner-freebsd-rc@FreeBSD.ORG Sat May 30 19:17:29 2009 Return-Path: Delivered-To: rc@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4C5821065672 for ; Sat, 30 May 2009 19:17:29 +0000 (UTC) (envelope-from dougb@FreeBSD.org) Received: from mail2.fluidhosting.com (mx21.fluidhosting.com [204.14.89.4]) by mx1.freebsd.org (Postfix) with ESMTP id 86DA28FC1A for ; Sat, 30 May 2009 19:17:27 +0000 (UTC) (envelope-from dougb@FreeBSD.org) Received: (qmail 3967 invoked by uid 399); 30 May 2009 19:17:19 -0000 Received: from localhost (HELO foreign.dougb.net) (dougb@dougbarton.us@127.0.0.1) by localhost with ESMTPAM; 30 May 2009 19:17:19 -0000 X-Originating-IP: 127.0.0.1 X-Sender: dougb@dougbarton.us Message-ID: <4A21863D.8040904@FreeBSD.org> Date: Sat, 30 May 2009 12:17:17 -0700 From: Doug Barton Organization: http://www.FreeBSD.org/ User-Agent: Thunderbird 2.0.0.21 (X11/20090423) MIME-Version: 1.0 To: Rick Macklem References: <200905291652.MAA15699@snowhite.cis.uoguelph.ca> In-Reply-To: <200905291652.MAA15699@snowhite.cis.uoguelph.ca> X-Enigmail-Version: 0.95.7 OpenPGP: id=D5B2F0FB Content-Type: multipart/mixed; boundary="------------020701030504040605080407" Cc: kib@FreeBSD.org, rc@FreeBSD.org, rwatson@FreeBSD.org Subject: Re: rc scripts for the new experimental nfs subsystem X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 May 2009 19:17:30 -0000 This is a multi-part message in MIME format. --------------020701030504040605080407 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Rick Macklem wrote: >> cursory inspection. Please review >> http://www.freebsd.org/doc/en_US.ISO8859-1/articles/rc-scripting/ as well. > > Thanks. Very helpful. GTH. > Ok, I've shortened nfsv4_serversupport_enable to nfsv4_server_enable and > changed the names of the others to follow the FreeBSD tradition (now that > I know about it:-). Well, that's what review is for, right? :) > I also made the new scripts follow the traditions, I > think? (I also simplified the if in nfsd, since I thought that's what > you meant by "shorten" the first time I read the email. I think it's more > readable this way, anyhow.) > > The "REQUIRE: NETWORKING" is debatable for nfsuserd, since it doesn't > use networking itself. However, it calls functions like getpwent(), which > usually use LDAP, NIS,... these days. Also, it's used by NFS, which is > useless without networking, so I thought it made sense? It sounds like what you really want to do is have nfsd REQUIRE nfsuserd. I'll only comment on a few things, the rest looks good. > diff -u -N -r rc.d.sav/nfscbd rc.d/nfscbd > --- rc.d.sav/nfscbd 1969-12-31 19:00:00.000000000 -0500 > +++ rc.d/nfscbd 2009-05-29 11:55:38.000000000 -0400 > @@ -0,0 +1,19 @@ > +#!/bin/sh > +# > +# $FreeBSD$ > +# > + > +# PROVIDE: nfscbd > +# REQUIRE: NETWORKING > +# KEYWORD: nojail shutdown > + > +. /etc/rc.subr > + > +name="nfscbd" > +rcvar=`set_rcvar` > +command="/usr/sbin/${name}" > + > +load_rc_config $name > +sig_stop="USR1" For consistency sake sig_stop should really be moved up between command and load_rc_config, but this is just cosmetic. > diff -u -N -r rc.d.sav/nfsd rc.d/nfsd I'm not sure why you added so many tests for the new variable. I've attached a streamlined version of your patch which I think will accomplish the same things. > diff -u -N -r rc.d.sav/nfsuserd rc.d/nfsuserd > --- rc.d.sav/nfsuserd 1969-12-31 19:00:00.000000000 -0500 > +++ rc.d/nfsuserd 2009-05-29 11:55:38.000000000 -0400 > @@ -0,0 +1,19 @@ > +#!/bin/sh > +# > +# $FreeBSD$ > +# > + > +# PROVIDE: nfsuserd > +# REQUIRE: NETWORKING > +# KEYWORD: nojail shutdown > + > +. /etc/rc.subr > + > +name="nfsuserd" > +rcvar=`set_rcvar` > +command="/usr/sbin/${name}" > + > +load_rc_config $name > +sig_stop="USR1" Same here with sig_stop. hope this helps, Doug -- This .signature sanitized for your protection --------------020701030504040605080407 Content-Type: text/plain; name="nfsd.diff" Content-Transfer-Encoding: base64 Content-Disposition: inline; filename="nfsd.diff" SW5kZXg6IG5mc2QKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gbmZzZAkocmV2aXNpb24gMTkyOTkxKQor KysgbmZzZAkod29ya2luZyBjb3B5KQpAQCAtMTQsMTQgKzE0LDMzIEBACiBjb21tYW5kPSIv dXNyL3NiaW4vJHtuYW1lfSIKIAogbG9hZF9yY19jb25maWcgJG5hbWUKLWNvbW1hbmRfYXJn cz0iJHtuZnNfc2VydmVyX2ZsYWdzfSIKIHN0YXJ0X3ByZWNtZD0ibmZzZF9wcmVjbWQiCiBz aWdfc3RvcD0iVVNSMSIKIAogbmZzZF9wcmVjbWQoKQogewotCWlmICEgc3lzY3RsIHZmcy5u ZnNydiA+L2Rldi9udWxsIDI+JjE7IHRoZW4KLQkJZm9yY2VfZGVwZW5kIG5mc3NlcnZlciB8 fCByZXR1cm4gMQorCWlmIGNoZWNreWVzbm8gbmZzdjRfc2VydmVyX2VuYWJsZTsgdGhlbgor CQkjIElmIG5mc3Y0X3NlcnZlcl9lbmFibGUgaXMgeWVzLCBmb3JjZSB1c2UKKwkJIyBvZiB0 aGUgZXhwZXJpbWVudGFsIHNlcnZlcgorCQkjCisJCXJjX2ZsYWdzPSItZSAke25mc19zZXJ2 ZXJfZmxhZ3N9IgorCisJCWlmICEgY2hlY2t5ZXNubyBuZnN1c2VyZF9lbmFibGUgICYmIFwK KwkJICAgICEgL2V0Yy9yYy5kL25mc3VzZXJkIGZvcmNlc3RhdHVzIDE+L2Rldi9udWxsIDI+ JjEKKwkJdGhlbgorCQkJZm9yY2VfZGVwZW5kIG5mc3VzZXJkIHx8IHJldHVybiAxCisJCWZp CisJZWxzZQorCQlyY19mbGFncz0iJHtuZnNfc2VydmVyX2ZsYWdzfSIKKworCQlpZiAhIHN5 c2N0bCB2ZnMubmZzcnYgPi9kZXYvbnVsbCAyPiYxOyB0aGVuCisJCQlmb3JjZV9kZXBlbmQg bmZzc2VydmVyIHx8IHJldHVybiAxCisJCWZpCisKKwkJaWYgY2hlY2t5ZXNubyBuZnNfcmVz ZXJ2ZWRfcG9ydF9vbmx5OyB0aGVuCisJCQllY2hvICdORlMgb24gcmVzZXJ2ZWQgcG9ydCBv bmx5PVlFUycKKwkJCXN5c2N0bCB2ZnMubmZzcnYubmZzX3ByaXZwb3J0PTEgPiAvZGV2L251 bGwKKwkJZmkKIAlmaQogCiAJaWYgISBjaGVja3llc25vIHJwY2JpbmRfZW5hYmxlICAmJiBc CkBAIC0zNSwxMSArNTQsNiBAQAogCXRoZW4KIAkJZm9yY2VfZGVwZW5kIG1vdW50ZCB8fCBy ZXR1cm4gMQogCWZpCi0KLQlpZiBjaGVja3llc25vIG5mc19yZXNlcnZlZF9wb3J0X29ubHk7 IHRoZW4KLQkJZWNobyAnTkZTIG9uIHJlc2VydmVkIHBvcnQgb25seT1ZRVMnCi0JCXN5c2N0 bCB2ZnMubmZzcnYubmZzX3ByaXZwb3J0PTEgPiAvZGV2L251bGwKLQlmaQogCXJldHVybiAw CiB9CiAK --------------020701030504040605080407--