Hi Rod, On 5 April 2019 20:28:06 BST, "Rodney W. Grimes" <freebsd-...@gndrsh.dnsmgr.net> wrote: >> Hello RC people, >> >> Konstantin has kindly reviewed the patch to fix load_kld behaviour, >but would prefer that someone more familiar with RC give me approval to >commit. I haven't stripped any of the replies, so the conversation >should be fairly easy to follow, though I'm happy to link to archives >if anyone missed it. >> >> Please would someone review and approve? >> >> https://www.bayofrum.net/~crees/patches/rc-kld_list-extension-2.diff > >Can we please either have this up in phabricator, or have permission >to pull your diff into a phabricator review if you do not want to >do it yourself. > >We do not need the comments and exchange of emails, >just the diffs. > >http://reviews.freebsd.org >
https://reviews.freebsd.org/D18670 Appears I'd forgotten it was there- it would be great to have an 'RC' group to review in phabricator. Cheers, Chris > >> Thanks! >> >> Chris >> >> On 25 December 2018 07:41:45 GMT, Konstantin Belousov ><kostik...@gmail.com> wrote: >> >On Mon, Dec 24, 2018 at 06:56:40PM +0000, Chris Rees wrote: >> >> On 24/12/2018 16:50, Konstantin Belousov wrote: >> >> > On Mon, Dec 24, 2018 at 03:34:57PM +0000, Chris Rees wrote: >> >> >> On 24/12/2018 13:37, Konstantin Belousov wrote: >> >> >>> On Mon, Dec 24, 2018 at 01:07:54PM +0000, Chris Rees wrote: >> >> >>>> On 24/12/2018 11:23, Chris Rees wrote: >> >> >>>>> On 24 Dec 2018 11:17, Konstantin Belousov ><kostik...@gmail.com> >> >wrote: >> >> >>>>> >> >> >>>>> On Mon, Dec 24, 2018 at 10:47:48AM +0000, Chris Rees >wrote: >> >> >>>>> > Author: crees (doc,ports committer) >> >> >>>>> > Date: Mon Dec 24 10:47:48 2018 >> >> >>>>> > New Revision: 342389 >> >> >>>>> > URL: https://svnweb.freebsd.org/changeset/base/342389 >> >> >>>>> > >> >> >>>>> > Log: >> >> >>>>> >?? Clarify kld_list format >> >> >>>>> >?? >> >> >>>>> >?? PR: docs/234248 >> >> >>>>> >?? Submitted by: David Fiander >> >> >>>>> >?? Submitted by: Miroslav Lachman >> >> >>>>> > >> >> >>>>> > Modified: >> >> >>>>> >?? head/share/man/man5/rc.conf.5 >> >> >>>>> > >> >> >>>>> > Modified: head/share/man/man5/rc.conf.5 >> >> >>>>> > >> >> >>>>> >> >>============================================================================== >> >> >>>>> > --- head/share/man/man5/rc.conf.5 Mon Dec 24 06:14:32 >> >2018 >> >> >>>>> (r342388) >> >> >>>>> > +++ head/share/man/man5/rc.conf.5 Mon Dec 24 10:47:48 >> >2018 >> >> >>>>> (r342389) >> >> >>>>> > @@ -248,12 +248,14 @@ Default >> >> >>>>> >? .Pa /etc/ddb.conf . >> >> >>>>> >? .It Va kld_list >> >> >>>>> >? .Pq Vt str >> >> >>>>> > -A list of kernel modules to load right after the >local >> >> >>>>> > -disks are mounted. >> >> >>>>> > +A whitespace-separated list of kernel modules to load >> >right after >> >> >>>>> > +the local disks are mounted, without any >> >> >>>>> > +.Pa .ko >> >> >>>>> > +extension or path. >> >> >>>>> I think both extension and path are accepted if >supplied. >> >> >>>>> It is the behaviour described in kldload(8). >> >> >>>>> >> >> >>>>> >> >> >>>>> That's true, but the kld rc script adds .ko, so providing >the >> >> >>>>> extension will probably break, and it checks for existing >> >modules >> >> >>>>> using the provided name as a regex, so that will also fail. >> >> >>>>> >> >> >>>>> I don't think that'd be hard to fix though, so I'll fix that >> >and put a >> >> >>>>> patch up for review later. >> >> >>>> Having looked again, rc.subr uses kldstat -v, so the path is >> >indeed not >> >> >>>> a problem, but the extension is-- removing any extension from >> >_kld will >> >> >>>> ensure that it will always match correctly.? At the moment it >is >> >> >>>> fragile, because it will load correctly the first time but >hit >> >an error >> >> >>>> if the user has put the extension in and the module is >already >> >loaded. >> >> >>>> >> >> >>>> @RC people, does this look acceptable (I'll need approval >> >please)? >> >> >>>> >> >> >>>> >> >https://www.bayofrum.net/~crees/patches/rc-kld_list-extension.diff >> >> >>> I do not quite see a point in the check for the module >presence. >> >> >>> Kernel already rejects already loaded modules (by module >name). >> >> >> True; this code predates the -n option to kldload.? Using that >> >makes the >> >> >> whole checking unnecessary. >> >> >> >> >> >> How about this one? >> >> >> >> >> >> >> >https://www.bayofrum.net/~crees/patches/rc-kld_list-extension-2.diff >> >> > It looks reasonable to me. I am not sure if we want to keep the >> >options >> >> > for load_kld for benefit of the third-party scripts, or not. >E.g. >> >we can >> >> > silently ignore them. >> >> >> >> Yeah, my patch ignores them silently.? It has the added bonus of >not >> >> needing to sweep the ports tree, with all the version issues that >> >> entails as the behaviour has slightly changed if the options are >> >> necessary at that point. >> >> >> >> > How was this tested ? >> >> [crees@pegasus]~/workspace/src/head% sudo sh >> >> # . libexec/rc/rc.subr >> >> # kldstat |grep cuse >> >> # load_kld cuse4bsd >> >> # kldstat |grep cuse >> >> 15??? 1 0xffffffff818c3000 40a0???? cuse.ko >> >> # load_kld cuse4bsd >> >> # load_kld doesntexist >> >> kldload: can't load doesntexist: No such file or directory >> >> sh: WARNING: Unable to load kernel module doesntexist >> >> # kldunload cuse >> >> # load_kld -m nothing -e noop /boot/modules/cuse4bsd.ko >> >> # kldstat |grep cuse >> >> 15??? 1 0xffffffff818c3000 4c80???? cuse4bsd.ko >> >> # load_kld -m nothing -e noop /boot/modules/cuse4bsd.ko >> >> # load_kld -m nothing -e noop /boot/modules/cuse4bsd.ko >> >> # load_kld -m nothing -e noop /boot/modules/cuse4bsd.ko >> >> # kldstat |grep cuse >> >> 15??? 1 0xffffffff818c3000 4c80???? cuse4bsd.ko >> >I suppose escapes are something that your mail agent inserted and >not >> >the >> >actual system output. >> > >> >The script looks fine to me, but I am not the right person to stamp >the >> >approval on the rc changes. >> > >> >> # >> >> >> >> It's rather a curiosity for me that cuse4bsd only loads as itself >if >> >> called by path, but it doesn't happen with any other modules-- >this >> >was >> >> just to prove that full paths and extensions work correctly as >> >> intended.? My machine also boots fine. >> >> >> >> Can you think of any other behaviour I'd need to check? >> >No, for me it looks good enough. >> > >> >-- >> >This message has been scanned for viruses and >> >dangerous content by MailScanner, and is >> >believed to be clean. >> >> -- >> Sent from my Android device with K-9 Mail. Please excuse my brevity. >> -- >> This message has been scanned for viruses and >> dangerous content by MailScanner, and is >> believed to be clean. >> >> _______________________________________________ >> freebsd-rc@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/freebsd-rc >> To unsubscribe, send any mail to "freebsd-rc-unsubscr...@freebsd.org" >> >> -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean. _______________________________________________ freebsd-rc@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-rc To unsubscribe, send any mail to "freebsd-rc-unsubscr...@freebsd.org"
