Date: Sat, 30 May 2009 12:17:17 -0700 From: Doug Barton <dougb@FreeBSD.org> To: Rick Macklem <rick@snowhite.cis.uoguelph.ca> Cc: kib@FreeBSD.org, rc@FreeBSD.org, rwatson@FreeBSD.org Subject: Re: rc scripts for the new experimental nfs subsystem Message-ID: <4A21863D.8040904@FreeBSD.org> In-Reply-To: <200905291652.MAA15699@snowhite.cis.uoguelph.ca> References: <200905291652.MAA15699@snowhite.cis.uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A21863D.8040904>