svn commit: r342389 - head/share/man/man5

[Konstantin Belousov]

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 <kostikbel at 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 at 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.