All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Support dropin files for Linux kernel parameters
@ 2024-03-01 15:43 Simon Rowe
  2024-03-02  3:21 ` Oskari Pirhonen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Simon Rowe @ 2024-03-01 15:43 UTC (permalink / raw)
  To: grub-devel; +Cc: Simon Rowe

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.

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}"
 
     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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support dropin files for Linux kernel parameters
  2024-03-01 15:43 [PATCH] Support dropin files for Linux kernel parameters Simon Rowe
@ 2024-03-02  3:21 ` Oskari Pirhonen
  2024-03-04  9:04   ` Simon Rowe
  2024-03-14  9:00 ` Simon Rowe
  2024-05-02 10:18 ` Julian Andres Klode
  2 siblings, 1 reply; 6+ messages in thread
From: Oskari Pirhonen @ 2024-03-02  3:21 UTC (permalink / raw)
  To: Simon Rowe; +Cc: grub-devel


[-- 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support dropin files for Linux kernel parameters
  2024-03-02  3:21 ` Oskari Pirhonen
@ 2024-03-04  9:04   ` Simon Rowe
  2024-03-05  7:15     ` Oskari Pirhonen
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Rowe @ 2024-03-04  9:04 UTC (permalink / raw)
  To: Oskari Pirhonen; +Cc: grub-devel@gnu.org


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

On 02/03/2024, 03:21, "Oskari Pirhonen" <xxc3ncoredxx@gmail.com> wrote:

> 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.

That then makes it impossible for an admin to override any parameter present in GRUB_CMDLINE_LINUX as that takes priority over files in either /etc/kernel.d/ or /usr/lib/kernel.d/

In my hubris I imagine that distros would move all static or install-time config to /usr/lib/kernel.d/ and GRUB_CMDLINE_LINUX would be left just for backward compatibility.

If a distro carries this change then the docs telling users how to set their own parameters would need updating to mention /etc/kernel.d/ instead of GRUB_CMDLINE_LINUX.

I can switch the order if that's the preferred behaviour, but I feel it diminishes the usefulness of the feature.

Regards
Simon

[-- Attachment #1.2: Type: text/html, Size: 3193 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support dropin files for Linux kernel parameters
  2024-03-04  9:04   ` Simon Rowe
@ 2024-03-05  7:15     ` Oskari Pirhonen
  0 siblings, 0 replies; 6+ messages in thread
From: Oskari Pirhonen @ 2024-03-05  7:15 UTC (permalink / raw)
  To: Simon Rowe; +Cc: grub-devel@gnu.org


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

On Mon, Mar 04, 2024 at 09:04:04 +0000, Simon Rowe wrote:
> On 02/03/2024, 03:21, "Oskari Pirhonen" <xxc3ncoredxx@gmail.com> wrote:
> 
> > 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.
> 
> That then makes it impossible for an admin to override any parameter
> present in GRUB_CMDLINE_LINUX as that takes priority over files in
> either /etc/kernel.d/ or /usr/lib/kernel.d/
> 
> In my hubris I imagine that distros would move all static or
> install-time config to /usr/lib/kernel.d/ and GRUB_CMDLINE_LINUX would
> be left just for backward compatibility.
> 

I imagine such a distro would ship their GRUB config with nothing set in
GRUB_CMDLINE_LINUX and only use /usr/lib/kernel.d/ for that. Then the
admin doesn't have to worry about overriding things in GRUB directly and
can use /etc/kernel.d/ instead.

If the distro makes use of GRUB_CMDLINE_LINUX_DEFAULT and
GRUB_CMDLINE_LINUX_RECOVERY to create different command lines, the admin
may still have to edit those by hand if they want custom options
specific to normal and "recovery" entries. After all, this behavior
isn't replicated by just slurping the files from
{/usr/lib,/etc}/kernel.d/ and tacking them on to the end.

> If a distro carries this change then the docs telling users how to set
> their own parameters would need updating to mention /etc/kernel.d/
> instead of GRUB_CMDLINE_LINUX.
> 
> I can switch the order if that's the preferred behaviour, but I feel
> it diminishes the usefulness of the feature.
> 

The main points that I currently disagree with are:

1. GRUB_CMDLINE_LINUX [should] be left just for backward compatibility
2. [switching the order] diminishes the usefulness of the feature

I believe they can both coexist if a distro documents their
preferred/supported way of setting the kernel command line (if they have
one). This can even be added as a comment in the installed
/etc/default/grub so that anyone going to edit the file will run across
it and (hopefully) adapt accordingly.

Diverging from the distro's policy is always at the discretion of the
system administrator, and I believe that the friendliest option for GRUB
is to maintain existing behavior as much as possible in these cases.

But feel free to wait for additional comments before sending a v2 with
the order changed.

- Oskari


PS: I was unable to find references to either /usr/lib/kernel.d/ or
/etc/kernel.d/ being used after a brief search online, so I presume that
they are to be provided by the distro's kernel package(s).

[-- 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support dropin files for Linux kernel parameters
  2024-03-01 15:43 [PATCH] Support dropin files for Linux kernel parameters Simon Rowe
  2024-03-02  3:21 ` Oskari Pirhonen
@ 2024-03-14  9:00 ` Simon Rowe
  2024-05-02 10:18 ` Julian Andres Klode
  2 siblings, 0 replies; 6+ messages in thread
From: Simon Rowe @ 2024-03-14  9:00 UTC (permalink / raw)
  To: grub-devel@gnu.org; +Cc: Daniel Kiper


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

Ping?

On 01/03/2024, 15:43, "Simon Rowe" <simon.rowe@nutanix.com> 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.

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}"

     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

[-- Attachment #1.2: Type: text/html, Size: 10111 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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Support dropin files for Linux kernel parameters
  2024-03-01 15:43 [PATCH] Support dropin files for Linux kernel parameters Simon Rowe
  2024-03-02  3:21 ` Oskari Pirhonen
  2024-03-14  9:00 ` Simon Rowe
@ 2024-05-02 10:18 ` Julian Andres Klode
  2 siblings, 0 replies; 6+ messages in thread
From: Julian Andres Klode @ 2024-05-02 10:18 UTC (permalink / raw)
  To: Simon Rowe; +Cc: The development of GNU GRUB

On Fri, Mar 01, 2024 at 03:43:50PM +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.

I'm -1 on this particular design choice given that it conflicts with
existing consensus in the wider Linux world around storing the kernel
command line in

/etc/kernel/cmdline, /usr/lib/kernel/cmdline, /proc/cmdline

(the first of these files is to be used)

And it is misaligned with already having a /etc/kernel directory
in the first place, a .d should be extending a file. Hence the
expectation here is that such drop-ins would go into a cmdline.d
directory.

Of course, you first are going to have to implement reading
/etc/kernel/cmdline and coordinate with other users like
systemd's kernel-install (or canonical/nullboot also uses it)
to add support for cmdline.d to our kernel installation scripts.

Generally speaking with drop-ins here I would expect this to
work by merging the dropins from /etc and /usr and then doing:

- /etc/kernel/cmdline if available
- /usr/lib/kernel/cmdline if available
- merged drop-ins
- /proc/cmdline

i.e. /usr/lib/kernel/cmdline.d would be completely replaced
by creating an /etc/kernel/cmdline.

-- 
debian developer - deb.li/jak | jak-linux.org - free software dev
ubuntu core developer                              i speak de, en

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-05-02 10:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-01 15:43 [PATCH] Support dropin files for Linux kernel parameters Simon Rowe
2024-03-02  3:21 ` Oskari Pirhonen
2024-03-04  9:04   ` Simon Rowe
2024-03-05  7:15     ` Oskari Pirhonen
2024-03-14  9:00 ` Simon Rowe
2024-05-02 10:18 ` Julian Andres Klode

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.