From 94cbddf43922e9da4b94d25552d002e0aa9f7077 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 May 2022 14:23:48 +0200 Subject: [PATCH 1/5] kernel-install: actually export KERNEL_INSTALL_VERBOSE :( --- src/kernel-install/kernel-install.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index b596d0c33ec..0428c88fb3c 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -75,7 +75,7 @@ fi export KERNEL_INSTALL_VERBOSE=0 if [ "$1" = "--verbose" ] || [ "$1" = "-v" ]; then shift - KERNEL_INSTALL_VERBOSE=1 + export KERNEL_INSTALL_VERBOSE=1 fi if [ "${0##*/}" = "installkernel" ]; then From b21ba8ac6bf6a5856cf1a939d3609c4d680d1dcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 May 2022 14:43:03 +0200 Subject: [PATCH 2/5] kernel-install: bail if machine id generation fails The call is unlikely to fail, but systemd-id128 might not be installed. We shouldn't continue with the empty string. --- src/kernel-install/kernel-install.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 0428c88fb3c..48072e346b1 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -114,7 +114,9 @@ fi # compatibility). [ -z "$MACHINE_ID" ] && [ -r /etc/machine-info ] && . /etc/machine-info && MACHINE_ID="$KERNEL_INSTALL_MACHINE_ID" [ -z "$MACHINE_ID" ] && [ -r /etc/machine-id ] && read -r MACHINE_ID Date: Thu, 19 May 2022 14:50:07 +0200 Subject: [PATCH 3/5] kernel-install: debug the configuration detection if --verbose No changes to behaviour, but let's print everything out as we discover it. The docs say that BOOT_ROOT can be specified by the environment. I have it locally in /etc/kernel/install.conf, and then the override doesn't work. It'd be nice to handle such cases more reliably. --- src/kernel-install/kernel-install.in | 75 +++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 7 deletions(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 48072e346b1..8ed9bfbb33c 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -103,19 +103,44 @@ layout= initrd_generator= if [ -r "/etc/kernel/install.conf" ]; then - . /etc/kernel/install.conf + install_conf="/etc/kernel/install.conf" elif [ -r "/usr/lib/kernel/install.conf" ]; then - . /usr/lib/kernel/install.conf + install_conf="/usr/lib/kernel/install.conf" +else + install_conf= fi +if [ -n "$install_conf" ]; then + [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "Reading $install_conf…" + . "$install_conf" + # FIXME: This may override configuration in environment variables, e.g. $BOOT_ROOT. +fi + +[ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && [ -n "$layout" ] && \ + echo "$install_conf configures layout=$layout" +[ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && [ -n "$initrd_generator" ] && \ + echo "$install_conf configures initrd_generator=$initrd_generator" + +[ -n "$MACHINE_ID" ] && [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ + echo "machine-id $MACHINE_ID set via environment or install.conf" +[ -n "$BOOT_ROOT" ] && [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ + echo "BOOT_ROOT=$BOOT_ROOT set via environment or install.conf" + # If /etc/machine-id is initialized we'll use it, otherwise we'll use a freshly # generated one. If the user configured an explicit machine ID to use in # /etc/machine-info to use for our purpose, we'll use that instead (for # compatibility). -[ -z "$MACHINE_ID" ] && [ -r /etc/machine-info ] && . /etc/machine-info && MACHINE_ID="$KERNEL_INSTALL_MACHINE_ID" -[ -z "$MACHINE_ID" ] && [ -r /etc/machine-id ] && read -r MACHINE_ID Date: Thu, 19 May 2022 15:34:32 +0200 Subject: [PATCH 4/5] kernel-install: fix detection of entry-token if $BOOT_ROOT is configured If $BOOT_ROOT is specified, but entry-token not, we'd skip the detection altogether, effectively defaulting to entry-token=machine-id. The case where $BOOT_ROOT was not specied, but entry-token was configured was handled correctly. This patch makes the handling of both symmetrical, i.e. will only set what wasn't configured. --- src/kernel-install/kernel-install.in | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 8ed9bfbb33c..9fa12f6fe81 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -170,11 +170,18 @@ fi # $ENTRY_TOKEN can be any string that fits into a VFAT filename, though # typically is just the machine ID. -[ -z "$BOOT_ROOT" ] && for suff in $ENTRY_TOKEN_SEARCH; do - for pref in "/efi" "/boot" "/boot/efi"; do +if [ -n "$BOOT_ROOT" ]; then + # If this was already configured, don't try to guess + BOOT_ROOT_SEARCH="$BOOT_ROOT" +else + BOOT_ROOT_SEARCH="/efi /boot /boot/efi" +fi + +for suff in $ENTRY_TOKEN_SEARCH; do + for pref in $BOOT_ROOT_SEARCH; do if [ -d "$pref/$suff" ]; then - BOOT_ROOT="$pref" - ENTRY_TOKEN="$suff" + [ -z "$BOOT_ROOT" ] && BOOT_ROOT="$pref" + [ -z "$ENTRY_TOKEN" ] && ENTRY_TOKEN="$suff" [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ echo "$pref/$suff exists, using BOOT_ROOT=$BOOT_ROOT, ENTRY_TOKEN=$ENTRY_TOKEN" From 1b43f868934e971480249a6e0fa2f45da906ea2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 May 2022 22:22:44 +0200 Subject: [PATCH 5/5] kernel-install: restore priority of check for /boot/loader/entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before 9e82a74cb0f08a288f9db228a0b5bec8a7188cdb, we had a check like the following: if [[ -d /efi/loader/entries ]] || [[ -d /efi/$MACHINE_ID ]]; then ENTRY_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION" elif [[ -d /boot/loader/entries ]] || [[ -d /boot/$MACHINE_ID ]]; then ENTRY_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION" elif [[ -d /boot/efi/loader/entries ]] || [[ -d /boot/efi/$MACHINE_ID ]]; then ENTRY_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION" … In stock Fedora 34-, /efi isn't used, but grub creates /boot/loader/entries and installs kernels and initrds directly in /boot. Thus the second arm of the check wins, and we end up with BOOT_ROOT=/boot. After 9e82a74cb0f08a288f9db228a0b5bec8a7188cdb, we iterate over the inner directory first and over the second directory later: [ -d /efi/ ] [ -d /boot/efi/ ] [ -d /boot/ ] [ -d /efi/Default ] [ -d /boot/efi/Default ] [ -d /boot/Default ] [ -d /efi/loader/entries ] [ -d /boot/efi/loader/entries ] [ -d /boot/loader/entries ] This was partially reverted by 447a822f8ee47b63a4cae00423c4d407bfa5e516 which removed Default from the list, and a5307e173bf86d695fe85b8e15e91126e8618a14, which moved checks for /boot up, so we ended up with: [ -d /efi/ ] [ -d /boot/ ] [ -d /boot/efi/ ] [ -d /efi/loader/entries ] [ -d /boot/loader/entries ] [ -d /boot/efi/loader/entries ] 6637cf9db67237857279262d93ee0e39023c5b85 added autodetection of an entry token, so we end up checking the following suffixes: , $IMAGE_ID, $ID, Default But the important unchanged characteristic is that we iterate over the suffix first. Sadly this breaks Fedora, because we find /boot/efi/ before we could find /boot/loader/entries. It seems that every possible aspect of behaviour matters for somebody, so we need to keep the original order of detection. With the patch: [ -d /efi/ ] ... [ -d /efi/loader/entries ] [ -d /boot/ ] ... [ -d /boot/loader/entries ] [ -d /boot/efi/ ] ... [ -d /boot/efi/loader/entries ] Note that we need to check for "loader/entries" too, even though it is not an entry-token candidate, so that we get the same detection priority as before. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2071034. --- src/kernel-install/kernel-install.in | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/kernel-install/kernel-install.in b/src/kernel-install/kernel-install.in index 9fa12f6fe81..f43c6b8b42f 100755 --- a/src/kernel-install/kernel-install.in +++ b/src/kernel-install/kernel-install.in @@ -177,8 +177,8 @@ else BOOT_ROOT_SEARCH="/efi /boot /boot/efi" fi -for suff in $ENTRY_TOKEN_SEARCH; do - for pref in $BOOT_ROOT_SEARCH; do +for pref in $BOOT_ROOT_SEARCH; do + for suff in $ENTRY_TOKEN_SEARCH; do if [ -d "$pref/$suff" ]; then [ -z "$BOOT_ROOT" ] && BOOT_ROOT="$pref" [ -z "$ENTRY_TOKEN" ] && ENTRY_TOKEN="$suff" @@ -189,21 +189,18 @@ for suff in $ENTRY_TOKEN_SEARCH; do else [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "$pref/$suff not found…" fi + + if [ -d "$pref/loader/entries" ]; then + [ -z "$BOOT_ROOT" ] && BOOT_ROOT="$pref" + [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ + echo "$pref/loader/entries exists, using BOOT_ROOT=$BOOT_ROOT" + break 2 + else + [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "$pref/loader/entries not found…" + fi done done -[ -z "$BOOT_ROOT" ] && for pref in "/efi" "/boot" "/boot/efi"; do - if [ -d "$pref/loader/entries" ]; then - BOOT_ROOT="$pref" - - [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && \ - echo "$pref/loader/entries exists, using BOOT_ROOT=$BOOT_ROOT" - break - else - [ "$KERNEL_INSTALL_VERBOSE" -gt 0 ] && echo "$pref/loader/entries not found…" - fi -done - [ -z "$BOOT_ROOT" ] && for pref in "/efi" "/boot/efi"; do if mountpoint -q "$pref"; then BOOT_ROOT="$pref"