Re: [PATCH] Add knobs to allow non-initrd boot config

[Nick Vinson]

Paul,

Let me start of by saying that I am a contributor and do not have any
authority to approve or deny patches to GRUB.  That said I do have a few
comments below that I would like for you to consider.

On 2/15/19 3:07 AM, Paul Menzel wrote:
> Date: Thu, 10 Nov 2016 13:44:25 -0500
> 
> Add GRUB_FORCE_PARTUUID and GRUB_DISABLE_INITRD configuration knobs to allow
> users to generate GRUB menu entries booting directly to the kernel, without
> using an initramfs.
> 
> Signed-off-by: Mathieu Trudel-Lapierre <address@hidden>
> [1. Upstream patch from Ubuntu repository
>     https://code.launchpad.net/ubuntu/+source/grub2
>  2. Fix typo s/Then/When/]
> Signed-off-by: Paul Menzel <address@hidden>
> ---
>  docs/grub.texi          | 13 +++++++++++++
>  util/grub-mkconfig.in   |  4 +++-
>  util/grub.d/10_linux.in | 11 +++++++++--
>  3 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index ecaba9d5c..6520d9f87 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -1522,6 +1522,19 @@ This option may be set to a list of GRUB module names 
> separated by spaces.
>  Each module will be loaded as early as possible, at the start of
>  @file{grub.cfg}.
>  
> address@hidden GRUB_FORCE_PARTUUID
> +This option forces the root disk entry to be the specified PARTUUID instead
> +of whatever would be used instead. This is useful when you control the
> +partitioning of the disk but cannot guarantee what the actual hardware will
> +be, for example in virtual machine images.
> +Setting this option to @samp{12345678-01} will produce:
> +root=PARTUUID=12345678-01
> 
> address@hidden GRUB_DISABLE_INITRD
> +When set to @samp{true}, this option prevents an initrd to be used at boot
> +time, regardless of whether one is detected or not. @command{grub-mkconfig}
> +will therefore not generate any initrd lines.
> +
>  @end table
>  
>  The following options are still accepted for compatibility with existing
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index 33332360e..265cf959b 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -238,7 +238,9 @@ export GRUB_DEFAULT \
>    GRUB_ENABLE_CRYPTODISK \
>    GRUB_BADRAM \
>    GRUB_OS_PROBER_SKIP_LIST \
> -  GRUB_DISABLE_SUBMENU
> +  GRUB_DISABLE_SUBMENU \
> +  GRUB_FORCE_PARTUUID \
> +  GRUB_DISABLE_INITRD
>  
>  if test "x${grub_cfg}" != "x"; then
>    rm -f "${grub_cfg}.new"
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index 4532266be..d4498f106 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -139,11 +139,18 @@ linux_entry ()
>      printf '%s\n' "${prepare_boot_cache}" | sed "s/^/$submenu_indentation/"
>    fi
>    message="$(gettext_printf "Loading Linux %s ..." ${version})"
> -  sed "s/^/$submenu_indentation/" << EOF
> +  if [ x"$GRUB_FORCE_PARTUUID" = x ]; then
> +    sed "s/^/$submenu_indentation/" << EOF

Since grub has the option GRUB_DISABLE_LINUX_PARTUUID which is expected
to control whether `root=PARTUUID=...` is used, perhaps it would be
better to add something like:

GRUB_DEVICE_PARTUUID=${GRUB_FORCE_PARTUUID:-${GRUB_DEVICE_PARTUUID}}

near the beginning of 10_linux.in?

>       echo    '$(echo "$message" | grub_quote)'
>       linux   ${rel_dirname}/${basename} 
> root=${linux_root_device_thisversion} ro ${args}
>  EOF
> -  if test -n "${initrd}" ; then
> +  else
> +    sed "s/^/$submenu_indentation/" << EOF
> +     echo    '$(echo "$message" | grub_quote)'
> +     linux   ${rel_dirname}/${basename} 
> root=$PARTUUID=${GRUB_FORCE_PARTUUID} ro ${args}
> +EOF
> +  fi
> +  if test -n "${initrd}" && [ x"$GRUB_DISABLE_INITRD" != xtrue ]; then

Perhaps instead of an all-or-nothing approach something similar to how
the GRUB_EARLY_INITRD_LINUX_STOCK and GRUB_EARLY_INITRD_LINUX_CUSTOM
variables are used could be adopted?

My thinking here is a user could set something like
GRUB_INITRD_LINUX_STOCK="" and GRUB_EARLY_INITRD_LINUX_STOCK="".  That
would allow for the same effect while also giving distros and users
greater flexibility in how early initrd and regular initrd images are named.

This also allows early initrds to still be used without normal initrds
to load CPU microcode updates at boot without having to recompile the
kernel on update.

However, for GRUB_EARLY_INITRD_LINUX_STOCK, this might require a small
code change as

`if [ "x${GRUB_EARLY_INITRD_LINUX_STOCK}" = "x" ]; then`

would probably need to be something like

`if ["x${GRUB_EARLY_INITRD_LINUX_STOCK+is_set} = "x"]; then`.

Thanks,
Nicholas Vinson

>      # TRANSLATORS: ramdisk isn't identifier. Should be translated.
>      message="$(gettext_printf "Loading initial ramdisk ...")"
>      initrd_path=
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

signature.asc
Description: OpenPGP digital signature