Re: [PATCH] Support dropin files for Linux kernel parameters

[Oskari Pirhonen <xxc3ncoredxx@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 6219 bytes --]

On Fri, Mar 01, 2024 at 15:43:50 +0000, Simon Rowe wrote:
> Kernel parameters actually cover a range of purposes, including
> userspace like systemd. They also need setting for a variety of
> reasons:
> 
>   * as distro defaults
>   * to provide configuration for a package
>   * for an admin to set desired behaviour
> 
> Having these all combined in a single line (like GRUB_CMDLINE_LINUX)
> is unwieldy, it is hard to make changes without impacting another
> usecase.
> 
> Add optional support for dropin files in the directories:
> 
>   * /usr/lib/kernel.d/
>   * /etc/kernel.d/
> 
> where the contents of each file with the '.conf' suffix is evaluated
> (excluding comments) and appended to any other kernel parameters
> defined via GRUB_CMDLINE_LINUX etc. Files in /etc/kernel.d/ completely
> replace those of the same name in /usr/lib/kernel.d/. This allows a
> distro or installer to set parameters but then for an admin to
> override them.
> 

Would it be better to have the kernel command line args in GRUB config
override the ones in any drop-ins? At least for me it would be
surprising behavior if my distro set something in /usr/lib/kernel.d/
and changing that value in my GRUB config didn't have an effect.

> Signed-off-by: Simon Rowe <simon.rowe@nutanix.com>
> ---
>  util/grub-mkconfig_lib.in   | 18 ++++++++++++++++++
>  util/grub.d/10_linux.in     |  8 +++++---
>  util/grub.d/20_linux_xen.in |  8 +++++---
>  3 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 08953287c..7aaa747f1 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -348,3 +348,21 @@ grub_add_tab () {
>    sed -e "s/^/$grub_tab/"
>  }
>  
> +kernel_params_from_files () {
> +  # Read Linux kernel parameters from dropin files.
> +
> +  file_bases=""
> +
> +  for f in /etc/kernel.d/*.conf /usr/lib/kernel.d/*.conf; do
> +    [ -r $f ] || continue
> +    file_bases="$file_bases $(basename $f)"
> +  done
> +
> +  for b in $(echo $file_bases | tr ' ' '\n' | sort -u); do
> +    if [ -r /etc/kernel.d/$b ]; then
> +      grep -v '^#' /etc/kernel.d/$b | tr '\n' ' '
> +    elif [ -r /usr/lib/kernel.d/$b ]; then
> +      grep -v '^#' /usr/lib/kernel.d/$b | tr '\n' ' '
> +    fi
> +  done
> +}
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index cc393be7e..fba3775f9 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -275,6 +275,8 @@ for linux in ${reverse_sorted_list}; do
>      fi
>    fi
>  
> +  extra_kernel_params=$(kernel_params_from_files)
> +
>    # The GRUB_DISABLE_SUBMENU option used to be different than others since it was
>    # mentioned in the documentation that has to be set to 'y' instead of 'true' to
>    # enable it. This caused a lot of confusion to users that set the option to 'y',
> @@ -285,7 +287,7 @@ for linux in ${reverse_sorted_list}; do
>  
>    if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xtrue ]; then
>      linux_entry "${OS}" "${version}" simple \
> -    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
> +    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT} ${extra_kernel_params}"
>  

In other words, should this instead be:

    "${extra_kernel_params} ${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"

Similarly for the other instances below.

- Oskari

>      submenu_indentation="$grub_tab"
>      
> @@ -298,10 +300,10 @@ for linux in ${reverse_sorted_list}; do
>    fi
>  
>    linux_entry "${OS}" "${version}" advanced \
> -              "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
> +              "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT} ${extra_kernel_params}"
>    if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
>      linux_entry "${OS}" "${version}" recovery \
> -                "${GRUB_CMDLINE_LINUX_RECOVERY} ${GRUB_CMDLINE_LINUX}"
> +                "${GRUB_CMDLINE_LINUX_RECOVERY} ${GRUB_CMDLINE_LINUX} ${extra_kernel_params}"
>    fi
>  done
>  
> diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
> index 94dd8be13..089f6de43 100644
> --- a/util/grub.d/20_linux_xen.in
> +++ b/util/grub.d/20_linux_xen.in
> @@ -336,6 +336,8 @@ for current_xen in ${reverse_sorted_xen_list}; do
>  	    fi
>  	fi
>  
> +	extra_kernel_params=$(kernel_params_from_files)
> +
>  	# The GRUB_DISABLE_SUBMENU option used to be different than others since it was
>  	# mentioned in the documentation that has to be set to 'y' instead of 'true' to
>  	# enable it. This caused a lot of confusion to users that set the option to 'y',
> @@ -346,7 +348,7 @@ for current_xen in ${reverse_sorted_xen_list}; do
>  
>  	if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xtrue ]; then
>  	    linux_entry "${OS}" "${version}" "${xen_version}" simple \
> -		"${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}"
> +		"${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT} ${extra_kernel_params}"
>  
>  	    submenu_indentation="$grub_tab$grub_tab"
>      
> @@ -360,10 +362,10 @@ for current_xen in ${reverse_sorted_xen_list}; do
>  	fi
>  
>  	linux_entry "${OS}" "${version}" "${xen_version}" advanced \
> -	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}"
> +	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT} ${extra_kernel_params}"
>  	if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
>  	    linux_entry "${OS}" "${version}" "${xen_version}" recovery \
> -		"${GRUB_CMDLINE_LINUX_RECOVERY} ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN}"
> +		"${GRUB_CMDLINE_LINUX_RECOVERY} ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN} ${extra_kernel_params}"
>  	fi
>      done
>      if [ x"$is_top_level" != xtrue ]; then
> -- 
> 2.22.3
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel