[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Grub EFI fallback - patches for review

On Mon, 2014-12-01 at 13:57 +0000, Steve McIntyre wrote:


> diff --git a/debian/grub-installer.templates b/debian/grub-installer.templates
> index e439ad0..a6af2ec 100644
> --- a/debian/grub-installer.templates
> +++ b/debian/grub-installer.templates
> @@ -209,6 +209,21 @@ Type: text
>  # :sl1:
>  _Description: Updating /etc/kernel-img.conf...
> +Template: grub-installer/progress/step_force_efi

Perhaps add _removable at the end of the name?

I didn't review the text since that seems to have been done already.

> diff --git a/rescue.d/81grub-efi-force-removable b/rescue.d/81grub-efi-force-removable

I don't know much about rescue mode, is this offering an automatic fixup
for this issue? Does it appear in a menu to be selected rather than
asking everyone booting rescue on an EFI system? 

> +mountvirtfs () {
> +       fstype="$1"
> +       path="$2"
> +       if grep -q "[[:space:]]$fstype\$" /proc/filesystems && \
> +          ! grep -q "^[^ ]\+ \+$path " /proc/mounts; then
> +               mkdir -p "$path" || \
> +                       die grub-installer/mounterr "Error creating $path"
> +               mount -t "$fstype" "$fstype" "$path" || \
> +                       die grub-installer/mounterr "Error mounting $path"
> +               trap "umount $path" HUP INT QUIT KILL PIPE TERM EXIT

trap doesn't stack, does it? You call mountvirtfs twice, so only the
second umount will actually happen on error.

Also you umount explicitly on the exit path, but don't cancel this trap,
so I guess you'll see some noise from umount the second time.

I know we've established that in-target isn't widely used in this
particular bit of code -- but it does take care of all this sort of
thing automatically and (presumably!) correctly, as well as being only a
single place to fix if it is wrong (e.g. in-target handles BSD
explicitly too).
> +log "Mounting filesystems"
> +# If we're installing grub-efi, it wants /sys mounted in the
> +# target. Maybe /proc too?
> +mountvirtfs proc /target/proc
> +mountvirtfs sysfs /target/sys
> +chroot /target mount /boot/efi || true
> +
> +db_progress STEP 1
> +db_progress INFO grub-installer/progress/step_install_loader
> +# Do the installation now
> +log "Running grub-install"
> +if ! chroot /target grub-install --force-extra-removable; then

The main invocation would invoke this with a --target="foo-efi". Not
sure if this matters or not.

In order to avoid having to repeat all the logic twice perhaps you could
arrange to do the debconf-set-selections thing first and then run
dpkg-reconfigure or something in the target to force the main postinst
to rerun and reinstall?

> +       db_input critical grub-installer/grub-install-failed || true
> +       db_go || true
> +       db_progress STOP
> +       exit 1

You don't umount /target/boot/efi on this path.


Reply to: