From owner-freebsd-rc@FreeBSD.ORG Sun Apr 24 12:47:47 2011 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 9743E106566B; Sun, 24 Apr 2011 12:47:47 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.mail.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id 26CB98FC0C; Sun, 24 Apr 2011 12:47:46 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ap0EAB8btE2DaFvO/2dsb2JhbAAvhCCiC7Ioj1qEeX0EjjU X-IronPort-AV: E=Sophos;i="4.64,263,1301889600"; d="diff'?scan'208";a="118463393" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu-pri.mail.uoguelph.ca with ESMTP; 24 Apr 2011 08:47:46 -0400 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 4FCA2B3F30; Sun, 24 Apr 2011 08:47:46 -0400 (EDT) Date: Sun, 24 Apr 2011 08:47:46 -0400 (EDT) From: Rick Macklem To: Doug Barton Message-ID: <532445037.487687.1303649266310.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <4DB3AA8C.7090003@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_487686_156809358.1303649266308" X-Originating-IP: [172.17.91.201] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - IE7 (Win)/6.0.10_GA_2692) Cc: rc@freebsd.org Subject: Re: rc scripts change for review 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: Sun, 24 Apr 2011 12:47:47 -0000 ------=_Part_487686_156809358.1303649266308 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit ----- Original Message ----- > On 04/23/2011 15:40, Rick Macklem wrote: > >> > >>> One thing I am not sure about is the REQUIRE: list in mountd. > >>> nfsserver and nfssrv essentially load the respective module. They > >>> are only > >>> used if sysctl variables need to be set and that's actually done > >>> by > >>> nfsd. > >>> (Should they be listed in mountd or nfsd or ???) > >> > >> It's not a good idea to add single scripts that only have a tiny > >> function, especially if they are only called conditionally (I.e., > >> they > >> contain KEYWORD: nostart). It's preferred to handle these things > >> things > >> in the script that needs them, or if it's necessary in more than > >> one > >> script to add a function to the appropriate .subr (rc, or network). > >> > >> So can you say a little more about what you're trying to > >> accomplish? > >> It's not clear to me why instead of doing this: > >> > >> + if ! sysctl vfs.newnfs>/dev/null 2>&1; then > >> + force_depend nfssrv || return 1 > >> + fi > >> > >> you would not just do this: > >> > >> + if ! sysctl vfs.newnfs>/dev/null 2>&1; then > >> + load_kld nfsd > >> + fi > >> > > Well, the intent of the above was to get the module loaded so that > > sysctl could manipulate its sysctl variables. I played with it a bit > > and it turns out that neither of the above code snippets work in the > > sense that they don't affect the outcome. > > > > What is needed to make the sysctls work is "nfssrv" has to be in the > > REQUIRED: list for either mountd or nfsd. Without it, the sysctls > > fail > > with unknown oid. (I'm guessing there is some delay between the > > load_kld > > and when the module gets its sysctl variables registered?) > > > > Now, I'm not sure whether there is any advantage to specifying > > "nfssrv" > > in nfsd or mountd, although both seem to work when I test them. > > "nfsserver" > > is in mountd, so unless you guys have a better suggestion, that's > > where > > I'll leave it. > > From what you're describing the reason it works by using the nfsserver > and nfssrv scripts is simply an accident of timing. That's not a good > thing no matter how you look at it. :) I understand your dilemma in > that the nfsserver "solution" was pre-existing, so you were just > following the example you had. However I have this odd idea that we > ought to fix broken code, not perpetuate it. Even if I weren't > interested in this for pedantic value, the fact that it's only working > now as an accident of timing is a good reason to fix it. > > The load_kld module is safe to run unconditionally (it will simply > return if the module is loaded). So what you could do (for both cases) > is something like this: > > load_kld nfsd > while ! sysctl vfs.newnfs >/dev/null 2>&1; do > sleep .5 > done > > If that works, I think we should add that feature to load_kld itself. I've updated the scripts at http://people.freebsd.org/~rmacklem/rc.conf and attached them, so you might not need to go there. Could you please take another look at them. This time nfsd uses the two load_klds and I tried to use "err", but I'm not sure if I checked for nfsuserd failing correctly. Thanks for your help, rick ------=_Part_487686_156809358.1303649266308 Content-Type: text/x-patch; name=nfsd.diff Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename=nfsd.diff LS0tIG5mc2Qub3JpZwkyMDExLTA0LTIwIDE4OjQ5OjA4LjAwMDAwMDAwMCAtMDQwMAorKysgbmZz ZAkyMDExLTA0LTI0IDA3OjMzOjA4LjAwMDAwMDAwMCAtMDQwMApAQCAtMTksMTkgKzE5LDggQEAg c2lnX3N0b3A9IlVTUjEiCiAKIG5mc2RfcHJlY21kKCkKIHsKLQlpZiBjaGVja3llc25vIG5mc3Y0 X3NlcnZlcl9lbmFibGU7IHRoZW4KLQkJIyBJZiBuZnN2NF9zZXJ2ZXJfZW5hYmxlIGlzIHllcywg Zm9yY2UgdXNlCi0JCSMgb2YgdGhlIGV4cGVyaW1lbnRhbCBzZXJ2ZXIKLQkJIwotCQlyY19mbGFn cz0iLWUgJHtuZnNfc2VydmVyX2ZsYWdzfSIKLQotCQlpZiAhIGNoZWNreWVzbm8gbmZzdXNlcmRf ZW5hYmxlICAmJiBcCi0JCSAgICAhIC9ldGMvcmMuZC9uZnN1c2VyZCBmb3JjZXN0YXR1cyAxPi9k ZXYvbnVsbCAyPiYxCi0JCXRoZW4KLQkJCWZvcmNlX2RlcGVuZCBuZnN1c2VyZCB8fCByZXR1cm4g MQotCQlmaQotCWVsc2UKLQkJcmNfZmxhZ3M9IiR7bmZzX3NlcnZlcl9mbGFnc30iCisJaWYgY2hl Y2t5ZXNubyBvbGRuZnNfc2VydmVyX2VuYWJsZTsgdGhlbgorCQlyY19mbGFncz0iLW8gJHtuZnNf c2VydmVyX2ZsYWdzfSIKIAogCQlpZiAhIHN5c2N0bCB2ZnMubmZzcnYgPi9kZXYvbnVsbCAyPiYx OyB0aGVuCiAJCQlmb3JjZV9kZXBlbmQgbmZzc2VydmVyIHx8IHJldHVybiAxCkBAIC00MSw2ICsz MCwzMSBAQCBuZnNkX3ByZWNtZCgpCiAJCQllY2hvICdORlMgb24gcmVzZXJ2ZWQgcG9ydCBvbmx5 PVlFUycKIAkJCXN5c2N0bCB2ZnMubmZzcnYubmZzX3ByaXZwb3J0PTEgPiAvZGV2L251bGwKIAkJ ZmkKKwllbHNlCisJCXJjX2ZsYWdzPSIke25mc19zZXJ2ZXJfZmxhZ3N9IgorCisJCSMgTG9hZCB0 aGUgbW9kdWxlcyBub3csIHNvIHRoYXQgdGhlIHZmcy5uZXduZnMgc3lzY3RsCisJCSMgb2lkcyBh cmUgYXZhaWxhYmxlLgorCQlsb2FkX2tsZCBuZnNjb21tb24KKwkJbG9hZF9rbGQgbmZzZAorCisJ CWlmIGNoZWNreWVzbm8gbmZzX3Jlc2VydmVkX3BvcnRfb25seTsgdGhlbgorCQkJZWNobyAnTkZT IG9uIHJlc2VydmVkIHBvcnQgb25seT1ZRVMnCisJCQlzeXNjdGwgdmZzLm5ld25mcy5uZnNfcHJp dnBvcnQ9MSA+IC9kZXYvbnVsbAorCQlmaQorCisJCWlmIGNoZWNreWVzbm8gbmZzdjRfc2VydmVy X2VuYWJsZTsgdGhlbgorCQkJaWYgISBjaGVja3llc25vIG5mc3VzZXJkX2VuYWJsZSAgJiYgXAor CQkJICAgICEgL2V0Yy9yYy5kL25mc3VzZXJkIGZvcmNlc3RhdHVzIDE+L2Rldi9udWxsIDI+JjEK KwkJCXRoZW4KKwkJCQlpZiAhIGZvcmNlX2RlcGVuZCBuZnN1c2VyZDsgdGhlbgorCQkJCQllcnIg MSAiQ2Fubm90IHJ1biBuZnN1c2VyZCIKKwkJCQlmaQorCQkJZmkKKwkJZWxzZQorCQkJZWNobyAn TkZTdjQgaXMgZGlzYWJsZWQnCisJCQlzeXNjdGwgdmZzLm5ld25mcy5zZXJ2ZXJfbWF4X25mc3Zl cnM9MyA+IC9kZXYvbnVsbAorCQlmaQogCWZpCiAKIAlpZiAhIGNoZWNreWVzbm8gcnBjYmluZF9l bmFibGUgICYmIFwK ------=_Part_487686_156809358.1303649266308 Content-Type: text/x-patch; name=mountd.diff Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename=mountd.diff LS0tIG1vdW50ZC5vcmlnCTIwMTEtMDQtMjAgMTg6NDk6MDIuMDAwMDAwMDAwIC0wNDAwCisrKyBt b3VudGQJMjAxMS0wNC0yNCAwNzozMzoxNS4wMDAwMDAwMDAgLTA0MDAKQEAgLTM3LDExICszNywx MCBAQCBtb3VudGRfcHJlY21kKCkKIAkJZmkKIAlmaQogCi0JIyBJZiBuZnN2NF9zZXJ2ZXJfZW5h YmxlIGlzIHllcywgZm9yY2UgdXNlIG9mIHRoZSBleHBlcmltZW50YWwKLQkjIHNlcnZlcgorCSMg SWYgb2xkbmZzX3NlcnZlcl9lbmFibGUgaXMgeWVzLCBmb3JjZSB1c2Ugb2YgdGhlIG9sZCBORlMg c2VydmVyCiAJIwotCWlmIGNoZWNreWVzbm8gbmZzdjRfc2VydmVyX2VuYWJsZTsgdGhlbgotCQly Y19mbGFncz0iLWUgJHtyY19mbGFnc30iCisJaWYgY2hlY2t5ZXNubyBvbGRuZnNfc2VydmVyX2Vu YWJsZTsgdGhlbgorCQlyY19mbGFncz0iLW8gJHtyY19mbGFnc30iCiAJZmkKIAogCWlmIGNoZWNr eWVzbm8gemZzX2VuYWJsZTsgdGhlbgo= ------=_Part_487686_156809358.1303649266308--