Robert, Sebastien and Stuart,
Thanks for your constructive and postive feedback. Combined with some
off-list (positive) feedback, it suggests this is at least worth
further consideration.
On Tue, Aug 24, 2021 at 08:05:37PM +0200, Robert Nagy wrote:
| I am happy to see this. All for it. Did you check all the error cases
| to make sure that reorder_kernel will report what needs to be reported?
Well, the thing I checked was that reorder_kernel sets errexit. So if
an error does occur during the config(8) run that gets added with the
diff, it will get reported as usual.
- no /etc/kernel.conf -> works as expected
- proper /etc/kernel.conf -> works as expected
- full partition -> config still is able to change the kernel
- faulty /etc/kernel.conf -> no new kernel, logged to console
On Wed, Aug 25, 2021 at 10:35:19AM +0200, Sebastien Marie wrote:
| - does it integrate well with syspatch ?
|
| yes, syspatch will call /usr/libexec/reorder_kernel (and the
| configuration will be applied)
Yes, I did consider this case and figured it would be fine as syspatch
just runs reorder_kernel (and even checks its exit status).
| - after install or upgrade, does the installed kernel (by installer)
| has the configuration applied ?
|
| it seems not: install.sub has it owns logic and is directly calling
| "make newbsd ; make install" and do not use reorder_kernel script.
Looking just now, install.sub runs in a chroot of the freshly
installed / upgraded system, and issues make newbsd and make
newinstall. Fixing that would be as straightforward as the original
diff. Since there seems to be general interest in my original diff,
I've included such a diff below (not sure if this should be a separate
commit or not).
| I could see reason to avoid it, and reason to requiring it.
|
|
| For install, no problem: the file is controlled (it doesn't
| exist). It doesn't change anything.
Personally, I don't see a reason to avoid it. For install, it could
exist. site.tgz could install an /etc/kernel.conf, for those users
that need to configure their kernels to have them work after the
initial install (this will help automated installs where the operator
already knows in advance that their kernel will need some further
configuration).
| For upgrade, the file could exists. Should the installer using it or
| not ?
|
|
| If using it, it makes the upgrade process dependent of the file: how
| to deal with an invalid file ? should the file ignored (kernel
| installed but not configured) ?
If you have a broken /etc/kernel.conf, it'll never work. A bit like
screwing up your /etc/boot.conf - you get to keep all pieces of your
broken system.
| how to deal with a format change in config(8) (file on disk in old
| format, used config(8) understanding new format) ? but config(8)
| doesn't change often, and could take care of both formats for one
| release, and/or current.html could mention some required changes.
This is a good point, something I didn't consider. But I agree with
your arguments here - seems like something that a) wouldn't happen
frequently and b) can be dealt with when it does.
On Wed, Aug 25, 2021 at 11:32:36AM +0100, Stuart Henderson wrote:
| Somebody had an earlier method for applying kernel config changes
| automatically that IIRC used a bootloader based mechanism passing the
| config to the kernel. I think that might actually be better overall
| as you could bypass it at the boot loader (rather than requiring a
| backup "clean" kernel) though that's more work and I expect difficult
| to do within space constraints on some archs.
I readily agree that that sounds like a better approach to me.
However, that approach is beyond my current skillset, I had an itch to
scratch, and this was my (relatively unintrusive) solution.
The diff below for install.sub has not been tested (yet).
Thanks again,
Paul
Index: install.sub
===================================================================
RCS file: /home/OpenBSD/cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1172
diff -u -p -r1.1172 install.sub
--- install.sub 9 Aug 2021 13:56:17 -0000 1.1172
+++ install.sub 25 Aug 2021 19:42:49 -0000
@@ -2857,7 +2857,10 @@ finish_up() {
tar -C $_kernel_dir -xzf $_kernel_dir.tgz $_kernel
rm -f $_kernel_dir.tgz
chroot /mnt /bin/ksh -e -c "cd ${_kernel_dir#/mnt}/$_kernel; \
- make newbsd; make newinstall"
+ make newbsd; \
+ [ -e /etc/kernel.conf ] && \
+ config -e -c /etc/kernel.conf -f bsd; \
+ make newinstall"
) >/dev/null 2>&1 && echo " done." || echo " failed."
fi
--
>++++++++[<++++++++++>-]<+++++++.>+++[<------>-]<.>+++[<+
+++++++++++>-]<.>++[<------------>-]<+.--------------.[-]
http://www.weirdnet.nl/
