Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 24 Dec 2018 18:56:40 +0000
From:      Chris Rees <chris@rees.space>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Chris Rees <crees@bayofrum.net>, freebsd-rc@freebsd.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r342389 - head/share/man/man5
Message-ID:  <f7e41992-430a-4e06-9398-7d341353c796@rees.space>
In-Reply-To: <20181224165023.GY60291@kib.kiev.ua>
References:  <9f786428-7fea-4fa4-a29e-ed91997a87fd@email.android.com> <dd115035-34c1-b73a-1ea5-f108407bda8d@rees.space> <20181224133721.GW60291@kib.kiev.ua> <eb8b7db2-d8be-3081-8f76-0291d1fbe3d7@rees.space> <20181224165023.GY60291@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
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@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
#

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?

Chris

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?f7e41992-430a-4e06-9398-7d341353c796>