Bug#988826: grub-installer: reinstalling GRUB on a BIOS setup yields an error in rescue mode despite being successful

[Cyril Brulebois <kibi@debian.org>]

Package: grub-installer
Version: 1.175
Severity: important
X-Debbugs-Cc: Steve McIntyre <93sam@debian.org>

Hi,

Calling this important since it gives a wrong indication to users trying
to rescue their systems. During smoke testing for #987377, once the hang
is fixed, I've toyed with other options like reinstalling the bootloader.

I was surprised to find out that the rescue mode interface reports there
was an error while grepping -i for GRUB in /var/log/syslog only returned
the usual success messages from grub-install, yet we get an error code
set to 1…


A wild guess (unconfirmed at this stage, it's a little late in the day)
is a regression coming from this commit:
  https://salsa.debian.org/installer-team/grub-installer/-/commit/5eada0008eede06c97d55adca1a9eb1eb9447aee

Namely, rescue.d/80grub-reinstall gets this new code (excerpt):

    chroot /target mount /boot/efi || true
    EXTRA_PATHS="$EXTRA_PATHS /target/boot/efi"
    trap "umount $EXTRA_PATHS" HUP INT QUIT KILL PIPE TERM EXIT

The first line does generate an error/warning message that's harmless
(seen in /var/log/syslog, not in the interface) for BIOS systems.

But the suspected culprit is the unconditional addition of
/target/boot/efi (that's unlikely to exist on such systems) to
EXTRA_PATHS, coupled with the trap call. Since the umount there isn't
guarded with a “|| true”, and since the trap is deployed for EXIT as
well, it might be that we're trying to exit successfully, but end up
triggering an error due to the ENOENT when unmounting.

If that's confirmed, it might be better to guard this differently, e.g.
via:

    if [ -e /target/boot/efi ]; then
        chroot /target mount /boot/efi || true
        EXTRA_PATHS="$EXTRA_PATHS /target/boot/efi"
    fi

so that we can still throw errors when unmounting other items of the
EXTRA_PATHS list, without purposefully generating an error that we would
ignored via “|| true”, also ignoring possible other errors in the
process…

That's just a vague gut feeling, feel free to adjust in the best way
you can imagine. :)


Cheers,
-- 
Cyril Brulebois (kibi@debian.org)            <https://debamax.com/>
D-I release manager -- Release team member -- Freelance Consultant