Summary: | devmatch rc.* documentation mismatch and typo fix [patch] | ||||||
---|---|---|---|---|---|---|---|
Product: | Base System | Reporter: | Matthias Andree <mandree> | ||||
Component: | bin | Assignee: | Warner Losh <imp> | ||||
Status: | Closed FIXED | ||||||
Severity: | Affects Many People | CC: | imp, mwlucas | ||||
Priority: | --- | Keywords: | easy, patch | ||||
Version: | CURRENT | Flags: | imp:
maintainer-feedback-
imp: mfc-stable13+ imp: mfc-stable12+ |
||||
Hardware: | Any | ||||||
OS: | Any | ||||||
Attachments: |
|
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 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 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. (In reply to Warner Losh from comment #3) I should have this resolved by the middle of july. 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(-) (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 :) 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 on attachment 225353 [details]
devmatch documentation fixes for main, 13, 12
withdrawing request for maintainer-approval and marking obsolete
(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. Thanks a bunch! 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(-) 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(-) *** Bug 258749 has been marked as a duplicate of this bug. *** |
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?