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

[Konstantin Belousov]

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.

How was this tested ?