Bug 256240 - devmatch rc.* documentation mismatch and typo fix [patch]
Summary: devmatch rc.* documentation mismatch and typo fix [patch]
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Warner Losh
URL:
Keywords: easy, patch
: 258749 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-05-29 10:49 UTC by Matthias Andree
Modified: 2022-01-16 21:37 UTC (History)
2 users (show)

See Also:
imp: maintainer-feedback-
imp: mfc-stable13+
imp: mfc-stable12+


Attachments
devmatch documentation fixes for main, 13, 12 (1.70 KB, patch)
2021-05-29 10:49 UTC, Matthias Andree
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Andree freebsd_committer freebsd_triage 2021-05-29 10:49:42 UTC
Created attachment 225353 [details]
devmatch documentation fixes for main, 13, 12

I found two glitches in devmatch's rc.* framework, 
the minor one is a typo in the rcscript proper, and 
the major one is a typo in the default/rc.conf comment where documentation does not match implementation. The rc.d/devmatch script itself requires the devmatch_blacklist module names *WITH* .ko and we should adjust documentation to match implementation because people who've debugged this already in their setups would otherwise be, astonished, and documentation for devmatch_blacklist="foo.ko" is afoot.

Bonus question,
Was this code tested with multiple blacklisted modules in devmatch_blacklist?
Comment 1 Warner Losh freebsd_committer freebsd_triage 2021-05-29 19:19:09 UTC
Hmmm, it's supposed to be "if_ed" in the blacklist, not if_ed.ko. if_ed is what's output by devmatch so it needs to match if_ed, not if_ed.ko
Comment 2 Matthias Andree freebsd_committer freebsd_triage 2021-05-29 20:34:55 UTC
Warner I understand what it should be, but there are multiple references as to why the devmatch_blacklist needs to list the module with .ko else it is not skipped on 13.0-RELEASE-p1.

devmatch_blacklist="virtio_random" does NOT work (and still loads the module),
devmatch_blacklist="virtio_random.ko" does work for others and me.

We can't change the implementation now for 13/12.

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=254513
Comment 3 Warner Losh freebsd_committer freebsd_triage 2021-07-08 18:05:01 UTC
Accepted that this is a valid bug. I think the diff may even be good too. I'll try to apply it in my next round.
Comment 4 Warner Losh freebsd_committer freebsd_triage 2021-07-08 18:06:08 UTC
(In reply to Warner Losh from comment #3)
I should have this resolved by the middle of july.
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-07-08 21:27:40 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b29ebb9c65b350e78aedfc790bfcaf9717059b70

commit b29ebb9c65b350e78aedfc790bfcaf9717059b70
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2021-07-08 19:44:21 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2021-07-08 21:22:22 +0000

    devmatch: Be tolerant of .ko being present.

    We document that we did not need .ko on the module names in
    devmatch_blocklist, but we really needed them. Keep the documentation
    the same, but strip the .ko when we need to use the names so you can
    specify either.

    PR:                     256240
    MFC After:              2 weeks
    Sponsored by:           Netflix

 libexec/rc/rc.d/devmatch | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
Comment 6 Warner Losh freebsd_committer freebsd_triage 2021-07-09 06:04:18 UTC
(In reply to Matthias Andree from comment #2)
Please give my committed code a look / spin and let me know if you find it acceptable. Rather than change the docs, I made both work which turned out to be super easy.

I'll MFC it once I'm sure it's good and I didn't mess something up :)
Comment 7 Matthias Andree freebsd_committer freebsd_triage 2021-07-09 16:16:52 UTC
I had to manually merge on 13.0-RELEASE-p3 because of the additional blocklist name that FreeBSD 14 supports; your code appears to work. Module name with .ko and without .ko suffix is _not_ loaded as expacted, and with that devmatch_blacklist commented out in rc.conf, the module gets loaded (and virtio_random hoses my 1-code rented virtual root server).

=> LGTM.
Comment 8 Matthias Andree freebsd_committer freebsd_triage 2021-07-09 16:19:08 UTC
Comment on attachment 225353 [details]
devmatch documentation fixes for main, 13, 12

withdrawing request for maintainer-approval and marking obsolete
Comment 9 Warner Losh freebsd_committer freebsd_triage 2021-07-09 16:20:46 UTC
(In reply to Matthias Andree from comment #7)
Great! I have it queued for MFC to 12 and 13 locally. Closing to reduce clutter in my bug list.
Comment 10 Matthias Andree freebsd_committer freebsd_triage 2021-07-09 16:51:23 UTC
Thanks a bunch!
Comment 11 commit-hook freebsd_committer freebsd_triage 2021-07-16 17:47:43 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3b125a8b3174e4efa2e98d02dfbba4e3ae9e52d5

commit 3b125a8b3174e4efa2e98d02dfbba4e3ae9e52d5
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2021-07-08 19:44:21 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2021-07-16 17:45:11 +0000

    devmatch: Be tolerant of .ko being present.

    We document that we did not need .ko on the module names in
    devmatch_blocklist, but we really needed them. Keep the documentation
    the same, but strip the .ko when we need to use the names so you can
    specify either.

    PR:                     256240
    MFC After:              2 weeks
    Sponsored by:           Netflix

 libexec/rc/rc.d/devmatch | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
Comment 12 commit-hook freebsd_committer freebsd_triage 2021-07-16 18:31:03 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=a4ce800b585ba01dc6e5787521654318906a1efd

commit a4ce800b585ba01dc6e5787521654318906a1efd
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2021-07-08 19:44:21 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2021-07-16 18:28:43 +0000

    devmatch: Be tolerant of .ko being present.

    We document that we did not need .ko on the module names in
    devmatch_blocklist, but we really needed them. Keep the documentation
    the same, but strip the .ko when we need to use the names so you can
    specify either.

    PR:                     256240
    MFC After:              2 weeks
    Sponsored by:           Netflix

    (cherry picked from commit b29ebb9c65b350e78aedfc790bfcaf9717059b70)

 libexec/rc/rc.d/devmatch | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
Comment 13 Andriy Gapon freebsd_committer freebsd_triage 2022-01-16 21:37:47 UTC
*** Bug 258749 has been marked as a duplicate of this bug. ***