[Merge] ~juliank/grub/+git/ubuntu:resilient-boot into ~ubuntu-core-dev/grub/+git/ubuntu:ubuntu
Steve Langasek
steve.langasek at canonical.com
Fri Apr 3 00:06:04 UTC 2020
Review: Needs Fixing
I see that you commented in the spec that you were moving the invocation of multiple targets into a wrapper script instead of modifying the C code. FTR this comment did not result in any notifications to me.
The problem with doing this in a wrapper script, and why I originally insisted on this being done in grub-install, is that grub-install is also the tool that on EFI systems populates the BootXXXX and BootOrder nvram variables. If we are not passing an ordered list of target devices to grub-install, then it does not know what to do with the nvram contents wrt recording primary and fallback options. So doing this in a wrapper script cannot actually provide the bootloader resiliency that we are looking for here.
Diff comments:
> diff --git a/debian/grub-common.install.in b/debian/grub-common.install.in
> index d5e5325..bcd2143 100644
> --- a/debian/grub-common.install.in
> +++ b/debian/grub-common.install.in
> @@ -2,6 +2,7 @@
> ../../debian/grub.d etc
> ../../debian/init-select.cfg etc/default/grub.d
> ../../debian/grub-check-signatures usr/share/grub/
> +../../debian/grub-multi-install usr/libexec/
This location is contrary to policy (via the FHS). Should be /usr/lib/grub/
> ../../debian/canonical-uefi-ca.crt usr/share/grub/
>
> etc/grub.d
> diff --git a/debian/grub-multi-install b/debian/grub-multi-install
> new file mode 100755
> index 0000000..e4fc4a3
> --- /dev/null
> +++ b/debian/grub-multi-install
> @@ -0,0 +1,389 @@
> +#!/bin/bash
> +#
> +# Install to multiple ESPs
> +
> +set -e
> +
> +# Most of this is copy-paste from grub postinst, sigh.
> +
> +. /usr/share/debconf/confmodule
> +
> +###############################################################################
> +# COPY FROM POSTINST
> +###############################################################################
why is this a copy, rather than a move of the code?
> +# This only works on a Linux system with udev running. This is probably the
> +# vast majority of systems where we need any of this, though, and we fall
> +# back reasonably gracefully if we don't have it.
> +cached_available_ids=
> +available_ids()
> +{
> + local id path
> +
> + if [ "$cached_available_ids" ]; then
> + echo "$cached_available_ids"
> + return
> + fi
> +
> + [ -d /dev/disk/by-id ] || return
> + cached_available_ids="$(
> + for path in /dev/disk/by-id/*; do
> + [ -e "$path" ] || continue
> + printf '%s %s\n' "$path" "$(readlink -f "$path")"
> + done | sort -k2 -s -u | cut -d' ' -f1
> + )"
> + echo "$cached_available_ids"
> +}
> +
> +# Returns non-zero and no output if no mapping can be found.
> +device_to_id()
> +{
> + local id
> + for id in $(available_ids); do
> + if [ "$(readlink -f "$id")" = "$(readlink -f "$1")" ]; then
> + echo "$id"
> + return 0
> + fi
> + done
> + # Fall back to the plain device name if there's no by-id link for it.
> + if [ -e "$1" ]; then
> + echo "$1"
> + return 0
> + fi
> + return 1
> +}
> +
> +# for Linux
> +sysfs_size()
> +{
> + local num_sectors sector_size size
> + # Try to find out the size without relying on a partitioning tool being
> + # installed. This isn't too hard on Linux 2.6 with sysfs, but we have to
> + # try a couple of variants on detection of the sector size.
> + if [ -e "$1/size" ]; then
> + num_sectors="$(cat "$1/size")"
> + sector_size=512
> + if [ -e "$1/queue/logical_block_size" ]; then
> + sector_size="$(cat "$1/queue/logical_block_size")"
> + elif [ -e "$1/queue/hw_sector_size" ]; then
> + sector_size="$(cat "$1/queue/hw_sector_size")"
> + fi
> + size="$(expr "$num_sectors" \* "$sector_size" / 1000 / 1000)"
> + fi
> + [ "$size" ] || size='???'
> + echo "$size"
> +}
> +
> +# for kFreeBSD
> +camcontrol_size()
> +{
> + local num_sectors sector_size size=
> +
> + if num_sectors="$(camcontrol readcap "$1" -q -s -N)"; then
> + sector_size="$(camcontrol readcap "$1" -q -b)"
> + size="$(expr "$num_sectors" \* "$sector_size" / 1000 / 1000)"
> + fi
> +
> + [ "$size" ] || size='???'
> + echo "$size"
> +}
> +
> +maybe_udevadm()
> +{
> + if which udevadm >/dev/null 2>&1; then
> + udevadm "$@" || true
> + fi
> +}
> +
> +# Parse /proc/mounts and find out the mount for the given device.
> +# The device must be a real device in /dev, not a symlink to one.
> +get_mounted_device()
> +{
> + mountpoint="$1"
> + cat /proc/mounts | while read -r line; do
> + set -f
> + set -- $line
> + set +f
> + if [ "$2" = "$mountpoint" ]; then
> + echo "$1"
> + break
> + fi
> + done
> +}
> +
> +###############################################################################
> +# New or modified helpers
> +###############################################################################
> +
> +# Fixed: Return nothing if the argument is empty
> +get_mountpoint()
> +{
> + local relpath boot_mountpoint
> +
> + if [ -z "$1" ]; then
> + return
> + fi
> +
> + relpath="$(grub-mkrelpath "$1")"
> + boot_mountpoint="${1#$relpath}"
> + echo "${boot_mountpoint:-/}"
> +}
> +
> +
> +# Returns value in $RET, like a debconf command.
> +#
> +# Merged version of describe_disk and describe_partition, as disks can't be
> +# valid ESPs on their own, so we can't render them as an entry.
> +multi_describe_partition()
> +{
> + local disk part id path sysfs_path diskbase partbase size
> + local disk_basename disk_size model
> + disk="$1"
> + part="$2"
> + id="$3"
> + path="$4"
> +
> + # BEGIN: Stolen from describe_disk
> + model=
> + case $(uname -s) in
> + Linux)
> + sysfs_path="$(maybe_udevadm info -n "$disk" -q path)"
> + if [ -z "$sysfs_path" ]; then
> + sysfs_path="/block/$(printf %s "${disk#/dev/}" | sed 's,/,!,g')"
> + fi
> + disk_size="$(sysfs_size "/sys$sysfs_path")"
> +
> + model="$(maybe_udevadm info -n "$disk" -q property | sed -n 's/^ID_MODEL=//p')"
> + if [ -z "$model" ]; then
> + model="$(maybe_udevadm info -n "$disk" -q property | sed -n 's/^DM_NAME=//p')"
> + if [ -z "$model" ]; then
> + model="$(maybe_udevadm info -n "$disk" -q property | sed -n 's/^MD_NAME=//p')"
> + if [ -z "$model" ] && which dmsetup >/dev/null 2>&1; then
> + model="$(dmsetup info -c --noheadings -o name "$disk" 2>/dev/null || true)"
> + fi
> + fi
> + fi
> + ;;
> + GNU/kFreeBSD)
> + disk_basename=$(basename "$disk")
> + disk_size="$(camcontrol_size "$disk_basename")"
> + model="$(camcontrol inquiry "$disk_basename" | sed -ne "s/^pass0: <\([^>]*\)>.*/\1/p")"
> + ;;
> + esac
> +
> + [ "$model" ] || model='???'
> +
> + # END: Stolen from describe_disk
> +
> + sysfs_path="$(maybe_udevadm info -n "$part" -q path)"
> + if [ -z "$sysfs_path" ]; then
> + diskbase="${disk#/dev/}"
> + diskbase="$(printf %s "$diskbase" | sed 's,/,!,g')"
> + partbase="${part#/dev/}"
> + partbase="$(printf %s "$partbase" | sed 's,/,!,g')"
> + sysfs_path="/block/$diskbase/$partbase"
> + fi
> + size="$(sysfs_size "/sys$sysfs_path")"
> +
> + db_subst grub2/efi_partition_description DEVICE "$part"
> + db_subst grub2/efi_partition_description SIZE "$size"
> + db_subst grub2/efi_partition_description PATH "$path"
> + db_subst grub2/efi_partition_description DISK_MODEL "$model"
> + db_subst grub2/efi_partition_description DISK_SIZE "$disk_size"
> + db_metaget grub2/efi_partition_description description
> +}
> +
> +
> +# Parse /proc/mounts and find out the mount for the given device.
> +# The device must be a real device in /dev, not a symlink to one.
> +multi_find_mount_point()
> +{
> + real_device="$1"
> + cat /proc/mounts | while read -r line; do
> + set -f
> + set -- $line
> + set +f
> + if [ "$1" = "$real_device" -a "$3" = "vfat" ]; then
> + echo "$2"
> + break
> + fi
> + done
> +}
> +
> +# Return all devices that are a valid ESP
> +multi_usable_partitions()
> +{
> + local last_partition path partition partition_id
> + local ID_PART_ENTRY_TYPE ID_PART_ENTRY_SCHEME
> +
> + last_partition=
> + (
> + for partition in /dev/disk/by-id/*; do
> + eval "$(udevadm info -q property -n "$partition" | grep -E '^ID_PART_ENTRY_(TYPE|SCHEME)=')"
> + if [ -z "$ID_PART_ENTRY_TYPE" -o -z "$ID_PART_ENTRY_SCHEME" -o \
> + \( "$ID_PART_ENTRY_SCHEME" != gpt -a "$ID_PART_ENTRY_SCHEME" != dos \) -o \
> + \( "$ID_PART_ENTRY_SCHEME" = gpt -a "$ID_PART_ENTRY_TYPE" != c12a7328-f81f-11d2-ba4b-00a0c93ec93b \) -o \
> + \( "$ID_PART_ENTRY_SCHEME" = dos -a "$ID_PART_ENTRY_TYPE" != 0xef \) ]; then
> + continue
> + fi
> + # unify the partition id
> + partition_id="$(device_to_id "$partition" || true)"
> + real_device="$(readlink -f "$partition")"
> + path="$(multi_find_mount_point $real_device)"
> + echo "$path:$partition_id"
> + done
> + ) | sort -t: -k2 -u
> +}
> +
> +###############################################################################
> +# MAGIC SCRIPT
> +###############################################################################
> +FALLBACK_MOUNTPOINT=/var/lib/grub/esp
> +
> +# Get configured value
> +question=grub2/efi_install_devices
> +priority=high
> +device_map="$(grub-mkdevicemap -m - | grep -v '^(fd[0-9]\+)' || true)"
> +devices="$(echo "$device_map" | cut -f2)"
> +db_get grub2/efi_install_devices
> +valid=1
> +for device in $RET; do
> + if [ ! -e "${device%,}" ]; then
> + valid=0
> + break
> + fi
> +done
> +
> +# If /boot/efi points to a device that's not in the list, trigger the
> +# install_devices_disks_changed prompt below, but add the device behind
> +# /boot/efi to the defaults.
> +boot_efi_device=$(get_mounted_device /boot/efi || true)
> +if [ "$boot_efi_device" ]; then
> + for device in $RET; do
> + real_device="$(readlink -f "$device" || true)"
> + if [ "$real_device" = "$boot_efi_device" ]; then
> + boot_efi_device=""
> + break
> + fi
> + done
> +
> + if [ "$boot_efi_device" ]; then
> + boot_efi_device="$(device_to_id "$boot_efi_device" || true)"
> + if [ "$RET" ]; then
> + RET="$RET, $boot_efi_device"
> + else
> + RET="$boot_efi_device"
> + fi
> + valid=0
> + fi
> +fi
> +
> +
> +if [ "$valid" = 0 ]; then
> + question=grub2/efi_install_devices_disks_changed
> + priority=critical
> + db_set "$question" "$RET"
> + db_fset "$question" seen false
> + db_fset grub2/efi_install_devices_empty seen false
> +fi
> +
> +while :; do
> + ids=
> + descriptions=
> + partitions="$(multi_usable_partitions)"
> + for device in $devices; do
> + disk_id="$(device_to_id "$device" || true)"
> + if [ "$disk_id" ]; then
> + for partition_pair in $partitions; do
> + partition_id="${partition_pair#*:}"
> + if [ "${partition_id#$disk_id-part}" != "$partition_id" ]; then
> + ids="${ids:+$ids, }$partition_id"
> + multi_describe_partition "$(readlink -f "$device")" "$(readlink -f "$partition_id")" "$partition_id" "$(get_mountpoint "${partition_pair%%:*}")"
> + RET="$(printf %s "$RET" | sed 's/,/\\,/g')"
> + descriptions="${descriptions:+$descriptions, }$RET"
> + fi
> + done
> + fi
> + done
> + db_subst "$question" RAW_CHOICES "$ids"
> + db_subst "$question" CHOICES "$descriptions"
> + db_input "$priority" "$question" || true
> + db_go
> + db_get "$question"
> +
> +
> + # Run the installer
> + failed_devices=
> + for i in `echo $RET | sed -e 's/, / /g'` ; do
> + real_device="$(readlink -f "$i")"
> + mntpoint=$(multi_find_mount_point $real_device)
> + if [ -z "$mntpoint" ]; then
> + mntpoint=$FALLBACK_MOUNTPOINT
> + mount $real_device $mntpoint
> + fi
> + echo "Installing grub to $mntpoint." >&2
> + if grub-install --efi-directory=$mntpoint "$@" ; then
> + # We just installed GRUB 2; then also generate grub.cfg.
> + touch /boot/grub/grub.cfg
> + else
> + failed_devices="$failed_devices $real_device"
> + fi
> +
> + if [ "$mntpoint" = "$FALLBACK_MOUNTPOINT" ]; then
> + umount $mntpoint
> + fi
> + done
> +
> + if [ "$question" != grub2/efi_install_devices ] && [ "$RET" ]; then
> + # XXX cjwatson 2019-02-26: The description of
> + # grub2/efi_install_devices_disks_changed ought to explain that
> + # selecting no devices will leave the configuration unchanged
> + # so that you'll be prompted again next time, but it's a bit
> + # close to the Debian 10 release to be introducing new
> + # translatable text. For now, it should be sufficient to
> + # avoid losing configuration data.
> + db_set grub2/efi_install_devices "$RET"
> + db_fset grub2/efi_install_devices seen true
> + fi
> +
> + if [ "$failed_devices" ]; then
> + db_subst grub2/efi_install_devices_failed FAILED_DEVICES "$failed_devices"
> + db_fset grub2/efi_install_devices_failed seen false
> + if db_input critical grub2/efi_install_devices_failed; then
> + db_go
> + db_get grub2/efi_install_devices_failed
> + if [ "$RET" = true ]; then
> + break
> + else
> + db_fset "$question" seen false
> + db_fset grub2/efi_install_devices_failed seen false
> + continue
> + fi
> + else
> + break # noninteractive
> + fi
> + fi
> +
> + db_get "$question"
> + if [ -z "$RET" ]; then
> + # Reset the seen flag if the current answer is false, since
> + # otherwise we'll loop with no indication of why.
> + db_get grub2/efi_install_devices_empty
> + if [ "$RET" = false ]; then
> + db_fset grub2/efi_install_devices_empty seen false
> + fi
> + if db_input critical grub2/efi_install_devices_empty; then
> + db_go
> + db_get grub2/efi_install_devices_empty
> + if [ "$RET" = true ]; then
> + break
> + else
> + db_fset "$question" seen false
> + db_fset grub2/efi_install_devices_empty seen false
> + fi
> + else
> + break # noninteractive
> + fi
> + else
> + break
> + fi
> +done
--
https://code.launchpad.net/~juliank/grub/+git/ubuntu/+merge/381462
Your team Ubuntu Core Development Team is subscribed to branch ~ubuntu-core-dev/grub/+git/ubuntu:ubuntu.
More information about the Ubuntu-reviews
mailing list