From owner-svn-src-head@freebsd.org Tue Dec 25 07:41:58 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8EAC41345547; Tue, 25 Dec 2018 07:41:58 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id BBCA58FC06; Tue, 25 Dec 2018 07:41:57 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id wBP7fkFt050136 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 25 Dec 2018 09:41:49 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua wBP7fkFt050136 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id wBP7fjvI050134; Tue, 25 Dec 2018 09:41:45 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 25 Dec 2018 09:41:45 +0200 From: Konstantin Belousov To: Chris Rees Cc: Chris Rees , 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: <20181225074145.GA60291@kib.kiev.ua> References: <9f786428-7fea-4fa4-a29e-ed91997a87fd@email.android.com> <20181224133721.GW60291@kib.kiev.ua> <20181224165023.GY60291@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.11.1 (2018-12-01) X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on tom.home X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 25 Dec 2018 07:41:58 -0000 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 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.