Bug#767037: Grub EFI fallback - patches for review
On Sun, Dec 21, 2014 at 08:24:08PM +, Steve McIntyre wrote: >On Sun, Dec 21, 2014 at 10:49:59AM +, Ian Campbell wrote: >>On Sat, 2014-12-20 at 09:45 +0100, David Härdeman wrote: >>> one option that doesn't seem to have been considered would be to create >>> a separate package (let's call it UEFIx) that installs an UEFI binary to >>> EFI/boot/bootx64.efi. That binary could then do what the UEFI BIOS >>> should've done (i.e. look at the EFI vars for bootorder, bootnext, etc >>> and then go on to load the right bootloader). >> >>Interesting idea, does this stub bootloader already exist, or is it >>something someone would need to write? (Either way I think it's likely >>too late for Jessie, but perhaps something to think about for Stretch) > >Exactly. :-/ I tried writing a stub bootloader. It works fine in a TianoCore QEMU environmentunfortunately it's a no go on my HP laptop (8570p). The HP UEFI BIOS helpfully deletes the BootOrder variable altogether :/ So...it was a promising idea, but one that won't work :( -- David Härdeman -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Sun, Dec 21, 2014 at 10:49:59AM +, Ian Campbell wrote: >On Sat, 2014-12-20 at 09:45 +0100, David Härdeman wrote: >> one option that doesn't seem to have been considered would be to create >> a separate package (let's call it UEFIx) that installs an UEFI binary to >> EFI/boot/bootx64.efi. That binary could then do what the UEFI BIOS >> should've done (i.e. look at the EFI vars for bootorder, bootnext, etc >> and then go on to load the right bootloader). > >Interesting idea, does this stub bootloader already exist, or is it >something someone would need to write? (Either way I think it's likely >too late for Jessie, but perhaps something to think about for Stretch) Exactly. :-/ >I'd also have some worries about packages installing to /boot/EFI since >that is by definition going to be a VFAT filesystem and I'm not >confident that dpkg will work fully/safely without all the POSIX-ish >semantics (hardlinks, atomic updates and the like), might want to handle >that by installing via the postinst instead of shipping in /boot/EFI. Definitely, yes. dpkg directly on VFAT is a no-go. -- Steve McIntyre, Cambridge, UK.st...@einval.com Can't keep my eyes from the circling sky, Tongue-tied & twisted, Just an earth-bound misfit, I... -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Sat, Dec 20, 2014 at 09:45:30AM +0100, David Härdeman wrote: >Hi, Hi! >one option that doesn't seem to have been considered would be to create >a separate package (let's call it UEFIx) that installs an UEFI binary to >EFI/boot/bootx64.efi. That binary could then do what the UEFI BIOS >should've done (i.e. look at the EFI vars for bootorder, bootnext, etc >and then go on to load the right bootloader). > >That way you'll have a solution that'll work across the different >bootloaders (grub-efi, gummiboot, etc), requires no changes to existing >bootloaders and which will only have an effect if explicitly installed >(adding d-i rescue code to optionally install the package should be >pretty straightforward as well). It also means that efibootmgr will work >as expected on both buggy and non-buggy machines. > >I realize you're alredy pretty well ahead on a different solution and >that it's late in the Jessie game, but I thought I should at least throw >this idea into the ring (it's basically what Matthew originally >suggested in http://mjg59.dreamwidth.org/4125.html). What you're suggesting is a good plan; I've even spoken with Matthew and some other upstream EFI maintainers. The shim package includes a fallback.efi which they recommend to install in the removable media path, and it does pretty much what you suggest. However, despite assurances months ago that we'd get a shim upload for Jessie that still hasn't happened. :-( I now think it's now way too late to add a new package like that for Jessie, hence I've been continuing down this route. -- Steve McIntyre, Cambridge, UK.st...@einval.com "I used to be the first kid on the block wanting a cranial implant, now I want to be the first with a cranial firewall. " -- Charlie Stross -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Sat, 2014-12-20 at 09:45 +0100, David Härdeman wrote: > one option that doesn't seem to have been considered would be to create > a separate package (let's call it UEFIx) that installs an UEFI binary to > EFI/boot/bootx64.efi. That binary could then do what the UEFI BIOS > should've done (i.e. look at the EFI vars for bootorder, bootnext, etc > and then go on to load the right bootloader). Interesting idea, does this stub bootloader already exist, or is it something someone would need to write? (Either way I think it's likely too late for Jessie, but perhaps something to think about for Stretch) I'd also have some worries about packages installing to /boot/EFI since that is by definition going to be a VFAT filesystem and I'm not confident that dpkg will work fully/safely without all the POSIX-ish semantics (hardlinks, atomic updates and the like), might want to handle that by installing via the postinst instead of shipping in /boot/EFI. Ian. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
Hi, one option that doesn't seem to have been considered would be to create a separate package (let's call it UEFIx) that installs an UEFI binary to EFI/boot/bootx64.efi. That binary could then do what the UEFI BIOS should've done (i.e. look at the EFI vars for bootorder, bootnext, etc and then go on to load the right bootloader). That way you'll have a solution that'll work across the different bootloaders (grub-efi, gummiboot, etc), requires no changes to existing bootloaders and which will only have an effect if explicitly installed (adding d-i rescue code to optionally install the package should be pretty straightforward as well). It also means that efibootmgr will work as expected on both buggy and non-buggy machines. I realize you're alredy pretty well ahead on a different solution and that it's late in the Jessie game, but I thought I should at least throw this idea into the ring (it's basically what Matthew originally suggested in http://mjg59.dreamwidth.org/4125.html). -- David Härdeman -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Mon, Dec 08, 2014 at 07:29:56PM +, Ian Campbell wrote: >On Mon, 2014-12-08 at 01:36 +, Steve McIntyre wrote: >> >The current package in sid (-17) is unblocked and I think ought to >> >transition tomorrow (or perhaps Tuesday depending on TZ). I propose to >> >upload -18 with this change shortly after that happens. Will you take >> >care of the unblock request or at least provide me some text with the >> >rationale etc. >> >> I'll ask for the unblock, and I'll also upload grub-installer the same >> day. > >Upload == done. and grub-installer 1.102 is in incoming now too. Christian, I'm sorry to do this to you, but we have some more template translations... :-/ I'm also hoping to get some more (small) changes done in the next week, for more UEFI goodness. >> >-17 included some patches from Colin to make the 32-bit linuxefi command >> >work. >> >> Yup, saw that. I'm looking into 32-bit EFI stuff right now. Using a >> Mac atm *shudder* > >Urk! Exactly! -- Steve McIntyre, Cambridge, UK.st...@einval.com < Aardvark> I dislike C++ to start with. C++11 just seems to be handing rope-creating factories for users to hang multiple instances of themselves. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Mon, 2014-12-08 at 01:36 +, Steve McIntyre wrote: > >The current package in sid (-17) is unblocked and I think ought to > >transition tomorrow (or perhaps Tuesday depending on TZ). I propose to > >upload -18 with this change shortly after that happens. Will you take > >care of the unblock request or at least provide me some text with the > >rationale etc. > > I'll ask for the unblock, and I'll also upload grub-installer the same > day. Upload == done. > >-17 included some patches from Colin to make the 32-bit linuxefi command > >work. > > Yup, saw that. I'm looking into 32-bit EFI stuff right now. Using a > Mac atm *shudder* Urk! Ian. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Sun, Dec 07, 2014 at 04:38:37PM +, Ian Campbell wrote: >Control: tag -1 +pending >On Wed, 2014-12-03 at 15:27 +, Steve McIntyre wrote: >> >> Cool. I don't (think I) have push access to the git repo, so if you >> could do the honours and apply, that would be lovely. :-) > >Done, patches are now in pkg-grub/grub.git#master. Cool, ta! >The current package in sid (-17) is unblocked and I think ought to >transition tomorrow (or perhaps Tuesday depending on TZ). I propose to >upload -18 with this change shortly after that happens. Will you take >care of the unblock request or at least provide me some text with the >rationale etc. I'll ask for the unblock, and I'll also upload grub-installer the same day. >There are a boatload of updates to debian/po/*. How is that handled? I >committed the automated thing but am I supposed to prod some process >somewhere else too? I've *no* idea - it confused me. :-/ >Anyway, I suppose there will need to be a second upload at some point >with the results of those translations. Perhaps that will be a good time >to fix #771249 too. I guess so, yes. >> I'm also wanting to get this into Jessie if we can, along with the >> 32-bit EFI work that's next...! > >-17 included some patches from Colin to make the 32-bit linuxefi command >work. Yup, saw that. I'm looking into 32-bit EFI stuff right now. Using a Mac atm *shudder* -- Steve McIntyre, Cambridge, UK.st...@einval.com "The problem with defending the purity of the English language is that English is about as pure as a cribhouse whore. We don't just borrow words; on occasion, English has pursued other languages down alleyways to beat them unconscious and rifle their pockets for new vocabulary." -- James D. Nicoll -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
Control: tag -1 +pending On Wed, 2014-12-03 at 15:27 +, Steve McIntyre wrote: > On Wed, Dec 03, 2014 at 09:42:20AM +, Ian Campbell wrote: > >On Wed, 2014-12-03 at 01:55 +, Steve McIntyre wrote: > >> On Tue, Dec 02, 2014 at 08:36:31AM +, Ian Campbell wrote: > >> >On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote: > >> > > >> >Starting with grub-install-fallback.patch: > >> > > >> >> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001 > >> >> debian/patches/grub-install-extra-removable.patch | 115 > >> >> ++ > >> > > >> >Could you send this to grub-de...@gnu.org? Or at least provide a commit > >> >log for the upstream bit inline in the patch for whoever does end up > >> >forwarding it. > >> > >> Sure, no problem. I've added a header for now. As my stuff is > >> intermingled with other changes in the Debian tree, I'm not sure how > >> well that will work upstream... > > > >Ah yes, if it is dependent on other non-upstream stuff then probably no > >point sending off in isolation. > > ACK. It's not *functionally* dependent, but it's intermingled in the > patches. > > >> Rebased patch V2 against current git master attached... > > > >Looks good to me. > > Cool. I don't (think I) have push access to the git repo, so if you > could do the honours and apply, that would be lovely. :-) Done, patches are now in pkg-grub/grub.git#master. The current package in sid (-17) is unblocked and I think ought to transition tomorrow (or perhaps Tuesday depending on TZ). I propose to upload -18 with this change shortly after that happens. Will you take care of the unblock request or at least provide me some text with the rationale etc. There are a boatload of updates to debian/po/*. How is that handled? I committed the automated thing but am I supposed to prod some process somewhere else too? Anyway, I suppose there will need to be a second upload at some point with the results of those translations. Perhaps that will be a good time to fix #771249 too. > I'm also wanting to get this into Jessie if we can, along with the > 32-bit EFI work that's next...! -17 included some patches from Colin to make the 32-bit linuxefi command work. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Wed, Dec 03, 2014 at 04:18:23PM +, Steve McIntyre wrote: > >A more generic fix would be to add to a list of filesystems that need >unmounting, and trap to a new shell function that unmounts that >list. Not too hard, I think - I'll see if I can do that and get it >tested today. > >Frankly, I'd also like to move mountvirtfs and that new function over >to a more central d-i scripts location and cut down on the duplicated >code. That's definitely something for post-jessie, as it's going to >potentially cut across a lot of the d-i packages. >>The unmount is wanted or the leaving of /boot/efi mounted is? (I could >>see an argument either way actually). > >I need to make sure that /target/boot/efi is unmounted; otherwise >exiting and re-entering the rescue menu fails. > >Updated patch coming soon... And here it is. Differences from v1 are: * s/UEFI/EFI/ in messages for consistency * s/step_force_efi/step_force_efi_removable/ * Better handling of mounting and unmounting -- Steve McIntyre, Cambridge, UK.st...@einval.com You lock the door And throw away the key There's someone in my head but it's not me >From cb00fb6bcae21d0628bd11e959629adae9c8fe39 Mon Sep 17 00:00:00 2001 From: Steve McIntyre Date: Wed, 3 Dec 2014 17:50:17 + Subject: [PATCH] Add support for using the UEFI removable media path Either during installation (low priority or preseeding), or as an extra rescue-mode option to help people fix their systems post-install once they realise they need to. (#767037) --- debian/changelog| 10 debian/grub-installer.templates | 43 ++ grub-installer | 14 + rescue.d/81grub-efi-force-removable | 93 +++ rescue.d/81grub-efi-force-removable.tst |3 + 5 files changed, 163 insertions(+) create mode 100755 rescue.d/81grub-efi-force-removable create mode 100755 rescue.d/81grub-efi-force-removable.tst diff --git a/debian/changelog b/debian/changelog index 6d94005..2879e27 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,13 @@ +grub-installer (1.102) unstable; urgency=medium + + [ Steve McIntyre ] + * Add extra support for forcing installation to the EFI +removable media path, either during installation (low priority or +preseeding), or as an extra rescue-mode option to help people fix +their systems post-install once they realise they need to. (#767037) + + -- Steve McIntyre <93...@debian.org> Mon, 01 Dec 2014 02:49:36 + + grub-installer (1.101) unstable; urgency=medium [ Steve McIntyre ] diff --git a/debian/grub-installer.templates b/debian/grub-installer.templates index e439ad0..e294afb 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_removable +Type: text +# :sl1: +_Description: Checking whether to force usage of the removable media path + +Template: grub-installer/progress/step_mount_filesystems +Type: text +# :sl1: +_Description: Mounting filesystems + +Template: grub-installer/progress/step_update_debconf_efi_removable +Type: text +# :sl1: +_Description: Configuring grub-efi for future usage of the removable media path + Template: debian-installer/grub-installer/title Type: text # Main menu item @@ -242,3 +257,31 @@ _Description: Failed to mount /target/proc Check /var/log/syslog or see virtual console 4 for the details. . Warning: Your system may be unbootable! + +Template: rescue/menu/grub-efi-force-removable +Type: text +# Rescue menu item +# :sl2: +_Description: Force GRUB installation to the EFI removable media path + +Template: grub-installer/force-efi-extra-removable +Type: boolean +Default: false +# :sl1: +_Description: Force GRUB installation to the EFI removable media path? + It seems that this computer is configured to boot via EFI, but maybe + that configuration will not work for booting from the hard + drive. Some EFI firmware implementations do not meet the EFI + specification (i.e. they are buggy!) and do not support proper + configuration of boot options from system hard drives. + . + A workaround for this problem is to install an extra copy of the EFI + version of the GRUB boot loader to a fallback location, the + "removable media path". Almost all EFI systems, no matter how buggy, + will boot GRUB that way. + . + Warning: If the installer failed to detect another operating system + that is present on your computer that also depends on this fallback, + installing GRUB there will make that operating system temporarily + unbootable. GRUB can be manually configured later to boot it if + necessary. diff --git a/grub-installer b/grub-installer index 4c12998..ef81dbf 100755 --- a/grub-installer +++ b/grub-installer @@ -785,6 +785,20 @@ if [ -z "$frdisk" ]; then fi fi + # Should we force a copy of
Bug#767037: Grub EFI fallback - patches for review
On Wed, Dec 03, 2014 at 09:44:08AM +, Ian Campbell wrote:
>On Wed, 2014-12-03 at 02:10 +, Steve McIntyre wrote:
>> >
>> >> +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.
>>
>> True. This (buggy) code is already in use elsewhere in grub-installer,
>> I've just borrowed it. :-/
>
>Hrm, yes.
A more generic fix would be to add to a list of filesystems that need
unmounting, and trap to a new shell function that unmounts that
list. Not too hard, I think - I'll see if I can do that and get it
tested today.
Frankly, I'd also like to move mountvirtfs and that new function over
to a more central d-i scripts location and cut down on the duplicated
code. That's definitely something for post-jessie, as it's going to
potentially cut across a lot of the d-i packages.
>> Right. I've absolutely *no* idea how well any of the existing EFI
>> stuff will work with BSD...!
>
>Me neither.
Again, I'm tempted to leave that alone for now then.
>> >The main invocation would invoke this with a --target="foo-efi". Not
>> >sure if this matters or not.
>>
>> Nope, the code in grub-install already picks up on the right platform
>> by default. I could add this too, but I'm not convinved it's necessary.
>
>Lets leave it then.
Agreed.
>> >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?
>>
>> Maybe, yeah. I'll take a look.
>>
>> >> + 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.
>>
>> Correct, and that's definitely wanted.
>
>The unmount is wanted or the leaving of /boot/efi mounted is? (I could
>see an argument either way actually).
I need to make sure that /target/boot/efi is unmounted; otherwise
exiting and re-entering the rescue menu fails.
Updated patch coming soon...
--
Steve McIntyre, Cambridge, UK.st...@einval.com
Support the Campaign for Audiovisual Free Expression: http://www.eff.org/cafe/
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Wed, Dec 03, 2014 at 09:42:20AM +, Ian Campbell wrote: >On Wed, 2014-12-03 at 01:55 +, Steve McIntyre wrote: >> On Tue, Dec 02, 2014 at 08:36:31AM +, Ian Campbell wrote: >> >On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote: >> > >> >Starting with grub-install-fallback.patch: >> > >> >> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001 >> >> debian/patches/grub-install-extra-removable.patch | 115 >> >> ++ >> > >> >Could you send this to grub-de...@gnu.org? Or at least provide a commit >> >log for the upstream bit inline in the patch for whoever does end up >> >forwarding it. >> >> Sure, no problem. I've added a header for now. As my stuff is >> intermingled with other changes in the Debian tree, I'm not sure how >> well that will work upstream... > >Ah yes, if it is dependent on other non-upstream stuff then probably no >point sending off in isolation. ACK. It's not *functionally* dependent, but it's intermingled in the patches. >> Rebased patch V2 against current git master attached... > >Looks good to me. Cool. I don't (think I) have push access to the git repo, so if you could do the honours and apply, that would be lovely. :-) I'm also wanting to get this into Jessie if we can, along with the 32-bit EFI work that's next...! -- Steve McIntyre, Cambridge, UK.st...@einval.com Getting a SCSI chain working is perfectly simple if you remember that there must be exactly three terminations: one on one end of the cable, one on the far end, and the goat, terminated over the SCSI chain with a silver-handled knife whilst burning *black* candles. --- Anthony DeBoer -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Wed, 2014-12-03 at 02:10 +, Steve McIntyre wrote:
> On Tue, Dec 02, 2014 at 08:51:24AM +, Ian Campbell wrote:
> >On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote:
> >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?
>
> The .tst file is run as a test:
>
> [ -f /target/boot/grub/grub.cfg ] && ( grep -q /boot/efi /target/etc/fstab )
>
> So, the target system must have grub installed and a mention of
> /boot/efi in /etc/fstab. Assuming that it does, an extra rescue option
> of "Force GRUB installation to the EFI removable media path" will show
> up as an option for the user. If the user selects it, the help text in
> grub-installer/force-efi-extra-removable is shown, then they can
> select to set the option.
>
Neat, thanks for explaining.
> >
> >> +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.
>
> True. This (buggy) code is already in use elsewhere in grub-installer,
> I've just borrowed it. :-/
Hrm, yes.
> >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.
>
> True; I've not seen any such errors, as it seems they're hidden at
> that point.
>
> >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).
>
> Right. I've absolutely *no* idea how well any of the existing EFI
> stuff will work with BSD...!
Me neither.
> >> +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.
>
> Nope, the code in grub-install already picks up on the right platform
> by default. I could add this too, but I'm not convinved it's necessary.
Lets leave it then.
> >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?
>
> Maybe, yeah. I'll take a look.
>
> >> + 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.
>
> Correct, and that's definitely wanted.
The unmount is wanted or the leaving of /boot/efi mounted is? (I could
see an argument either way actually).
Ian.
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Wed, 2014-12-03 at 01:55 +, Steve McIntyre wrote: > On Tue, Dec 02, 2014 at 08:36:31AM +, Ian Campbell wrote: > >On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote: > > > >Starting with grub-install-fallback.patch: > > > >> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001 > >> debian/patches/grub-install-extra-removable.patch | 115 > >> ++ > > > >Could you send this to grub-de...@gnu.org? Or at least provide a commit > >log for the upstream bit inline in the patch for whoever does end up > >forwarding it. > > Sure, no problem. I've added a header for now. As my stuff is > intermingled with other changes in the Debian tree, I'm not sure how > well that will work upstream... Ah yes, if it is dependent on other non-upstream stuff then probably no point sending off in isolation. > Rebased patch V2 against current git master attached... Looks good to me. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Tue, Dec 02, 2014 at 08:51:24AM +, Ian Campbell wrote:
>On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote:
>
>grub-installer-rescue-UEFI-removable.patch:
>
>> 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?
Yeah, fair point.
>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?
The .tst file is run as a test:
[ -f /target/boot/grub/grub.cfg ] && ( grep -q /boot/efi /target/etc/fstab )
So, the target system must have grub installed and a mention of
/boot/efi in /etc/fstab. Assuming that it does, an extra rescue option
of "Force GRUB installation to the EFI removable media path" will show
up as an option for the user. If the user selects it, the help text in
grub-installer/force-efi-extra-removable is shown, then they can
select to set the option.
>
>> +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.
True. This (buggy) code is already in use elsewhere in grub-installer,
I've just borrowed it. :-/
>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.
True; I've not seen any such errors, as it seems they're hidden at
that point.
>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).
Right. I've absolutely *no* idea how well any of the existing EFI
stuff will work with BSD...!
>> +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.
Nope, the code in grub-install already picks up on the right platform
by default. I could add this too, but I'm not convinved it's necessary.
>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?
Maybe, yeah. I'll take a look.
>> + 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.
Correct, and that's definitely wanted.
--
Steve McIntyre, Cambridge, UK.st...@einval.com
< Aardvark> I dislike C++ to start with. C++11 just seems to be
handing rope-creating factories for users to hang multiple
instances of themselves.
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Tue, Dec 02, 2014 at 08:36:31AM +, Ian Campbell wrote:
>On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote:
>
>Starting with grub-install-fallback.patch:
>
>> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001
>> debian/patches/grub-install-extra-removable.patch | 115
>> ++
>
>Could you send this to grub-de...@gnu.org? Or at least provide a commit
>log for the upstream bit inline in the patch for whoever does end up
>forwarding it.
Sure, no problem. I've added a header for now. As my stuff is
intermingled with other changes in the Debian tree, I'm not sure how
well that will work upstream...
>> +@@ -829,6 +838,27 @@ fill_core_services (const char *core_ser
>> + free (sysv_plist);
>> + }
>> +
>> ++static void
>> ++also_install_removable(const char *src, const char *base_efidir, const
>> char *efi_suffix_upper)
>> ++{
>> ++ char *efi_file = NULL;
>> ++ char *dst = NULL;
>> ++ char *dir = NULL;
>> ++
>> ++ if (!efi_suffix_upper)
>> ++grub_util_error ("%s", _("You've found a bug"));
>
>There are one or two of these in the upstream code base already, but it
>is a bit unfriendly to the bug-reported/triagger.
>
>I see an existing instance of
>_("you found a bug ... (%s:%d)\n"), file, line)
>which is a bit nicer at least. Plain old assert() sees some usage too.
OK. Again, I was just following the surrounding (grotty) style. :-)
I'll tweak.
>The Debian-specific bits all look sensible to me, FWIW. There will be a
>minor conflict with the patches in #770412 but nothing insurmountable.
Cool, ta!
>> [...]
>> + also depend on this path. If so, uou will need to ensure that GRUB is
>
>Typo: "uou".
ACK, already fixed after KiBi's feedback.
Rebased patch V2 against current git master attached...
--
Steve McIntyre, Cambridge, UK.st...@einval.com
"The problem with defending the purity of the English language is that
English is about as pure as a cribhouse whore. We don't just borrow words; on
occasion, English has pursued other languages down alleyways to beat them
unconscious and rifle their pockets for new vocabulary." -- James D. Nicoll
>From a863157a9020edfb6d1fa8379703026cd86c45c9 Mon Sep 17 00:00:00 2001
From: Steve McIntyre
Date: Wed, 3 Dec 2014 01:51:57 +
Subject: [PATCH] Add support for forcing EFI installation to the removable
media path
Add an extra option to grub-install "--force-extra-removable". On EFI
platforms, this will cause an extra copy of the grub-efi image to be
written to the appropriate removable media patch
/boot/efi/EFI/BOOT/BOOT$ARCH.EFI as well. This will help with broken
UEFI implementations where the firmware does not work when configured
with new boot paths.
Also added new debconf logic to add this extra option to grub-install
calls when grub2/force_efi_extra_removable is set true. This allows
other programs like d-i / grub-installer to configure this for general
use.
Provides part of the fix for #767037
---
debian/changelog | 8 ++
debian/config.in | 1 +
debian/patches/grub-install-extra-removable.patch | 133 ++
debian/patches/series | 1 +
debian/postinst.in| 6 +-
debian/templates.in | 11 ++
6 files changed, 159 insertions(+), 1 deletion(-)
create mode 100644 debian/patches/grub-install-extra-removable.patch
diff --git a/debian/changelog b/debian/changelog
index f0eac36..70dd9aa 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+grub2 (2.02~beta2-18) UNRELEASED; urgency=medium
+
+ [ Steve McIntyre ]
+ * Add support for forcing an extra copy of grub-efi to the removable
+media path /boot/efi/EFI/BOOT/BOOT$ARCH.EFI (#767037)
+
+ -- Steve McIntyre <93...@debian.org> Wed, 03 Dec 2014 01:23:18 +
+
grub2 (2.02~beta2-17) unstable; urgency=medium
[ Colin Watson ]
diff --git a/debian/config.in b/debian/config.in
index fcc0dd4..d2afbcb 100644
--- a/debian/config.in
+++ b/debian/config.in
@@ -73,4 +73,5 @@ esac
db_input ${priority} grub2/linux_cmdline || true
db_input medium grub2/linux_cmdline_default || true
+db_input low grub2/force_efi_extra_removable || true
db_go
diff --git a/debian/patches/grub-install-extra-removable.patch b/debian/patches/grub-install-extra-removable.patch
new file mode 100644
index 000..693e885
--- /dev/null
+++ b/debian/patches/grub-install-extra-removable.patch
@@ -0,0 +1,133 @@
+From: Steve McIntyre <93...@debian.org>
+Date: Wed, 3 Dec 2014 01:25:12 +
+Subject: Add support for forcing EFI installation to the removable media path
+
+Add an extra option to grub-install "--force-extra-removable". On EFI
+platforms, this will cause an extra copy of the grub-efi image to be
+written to the appropriate removable media patch
+/boot/efi/EFI/BOOT/BOOT$ARCH.EFI as well. This will help with broken
+UEFI imp
Bug#767037: Grub EFI fallback - patches for review
On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote:
grub-installer-rescue-UEFI-removable.patch:
> 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.
Ian.
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Mon, 2014-12-01 at 13:57 +, Steve McIntyre wrote:
Starting with grub-install-fallback.patch:
> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001
> debian/patches/grub-install-extra-removable.patch | 115
> ++
Could you send this to grub-de...@gnu.org? Or at least provide a commit
log for the upstream bit inline in the patch for whoever does end up
forwarding it.
> +@@ -829,6 +838,27 @@ fill_core_services (const char *core_ser
> + free (sysv_plist);
> + }
> +
> ++static void
> ++also_install_removable(const char *src, const char *base_efidir, const char
> *efi_suffix_upper)
> ++{
> ++ char *efi_file = NULL;
> ++ char *dst = NULL;
> ++ char *dir = NULL;
> ++
> ++ if (!efi_suffix_upper)
> ++grub_util_error ("%s", _("You've found a bug"));
There are one or two of these in the upstream code base already, but it
is a bit unfriendly to the bug-reported/triagger.
I see an existing instance of
_("you found a bug ... (%s:%d)\n"), file, line)
which is a bit nicer at least. Plain old assert() sees some usage too.
The Debian-specific bits all look sensible to me, FWIW. There will be a
minor conflict with the patches in #770412 but nothing insurmountable.
> [...]
> + also depend on this path. If so, uou will need to ensure that GRUB is
Typo: "uou".
Ian.
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#767037: Grub EFI fallback - patches for review
On Montag, 1. Dezember 2014, Cyril Brulebois wrote: > Looking at the EFI/UEFI history, I hope nobody will think EFI is the > deprecated, pre-UEFI implementation. I wouldn't be so sure. Up until this very moment I thought EFI was something different than UEFI... > With that in mind, and with all EFI > occurrences we already have (including “mandatory” names), it looks to > my non-expert eye that EFI might be the name to standardize over. Maybe > Colin could confirm that. this sounds fine to me, with the above in mind however, I would suggest to explain this (at least in the manual) once. signature.asc Description: This is a digitally signed message part.
Bug#767037: Grub EFI fallback - patches for review
Steve McIntyre (2014-12-01):
> Hmmm, you're right. There's some existing inconsistencies already,
> which don't help. In various places we already use "EFI" (e.g. in the
> GRUB package names, EFI System Partition etc.) but in others it's
> UEFI. Maybe we'd be better with just EFI everywhere...?
Looking at the EFI/UEFI history, I hope nobody will think EFI is the
deprecated, pre-UEFI implementation. With that in mind, and with all EFI
occurrences we already have (including “mandatory” names), it looks to
my non-expert eye that EFI might be the name to standardize over. Maybe
Colin could confirm that.
> >Did you send the templates for review through debian-l10n-english?
>
> Nope. They're currently entirely my own set of mistakes written in the
> early hours... :-)
:p
>
>
> >> +@@ -846,6 +876,7 @@ main (int argc, char *argv[])
> >> + char *relative_grubdir;
> >> + char **efidir_device_names = NULL;
> >> + grub_device_t efidir_grub_dev = NULL;
> >> ++ char *base_efidir = NULL;
> >> + char *efidir_grub_devname;
> >> + int efidir_is_mac = 0;
> >> + int is_prep = 0;
> >> +@@ -878,6 +909,9 @@ main (int argc, char *argv[])
> >> + bootloader_id = xstrdup ("grub");
> >> + }
> >> +
> >> ++ if (removable && force_extra_removable)
> >> ++grub_util_error (_("Invalid to use both --removable and
> >> --force_extra_removable"));
> >> ++
> >> + if (!grub_install_source_directory)
> >> + {
> >> + if (!target)
> >> +@@ -1087,6 +1121,8 @@ main (int argc, char *argv[])
> >> + if (!efidir_is_mac && grub_strcmp (fs->name, "fat") != 0)
> >> + grub_util_error (_("%s doesn't look like an EFI partition.\n"), efidir);
> >> +
> >> ++ base_efidir = xstrdup(efidir);
>
> >Needs a matching free()?
>
> I wish it was that easy - the code in grub-install.c is a mix of
> styles here. Some allocations are freed immediately, while some stay
> around for the life of the program. This needs to be one of the
> latter, I think.
OK. I suspected that way after having looked a bit, but thought asking
wouldn't hurt.
>
>
> >> diff --git a/rescue.d/81grub-efi-force-removable
> >> b/rescue.d/81grub-efi-force-removable
> >> new file mode 100644
> >> index 000..b35b724
> >> --- /dev/null
> >> +++ b/rescue.d/81grub-efi-force-removable
> >> @@ -0,0 +1,91 @@
> >> +#! /bin/sh -e
> >> +
> >> +. /usr/share/debconf/confmodule
> >> +
> >> +. /usr/share/grub-installer/functions.sh
> >> +
> >> +log () {
> >> + logger -t grub-installer "grub-efi-force-removable $@"
> >> +}
> >> +
> >> +error () {
> >> + log "error: $@"
> >> +}
> >> +
> >> +die () {
> >> + local template="$1"
> >> + shift
> >> +
> >> + error "$@"
> >> + db_input critical "$template" || [ $? -eq 30 ]
> >> + db_go || true
> >> + exit 1
> >> +}
> >> +
> >> +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
> >> + fi
> >> +}
> >> +
> >> +db_progress START 0 3 grub-installer/progress/title
> >> +db_progress INFO grub-installer/progress/step_force_efi
> >> +
> >> +# Should we also install grub-efi to the removable media path?
> >> +# Ask the user
> >> +log "Prompting user about removable media path"
> >> +db_input high grub-installer/force-efi-extra-removable
> >> +if ! db_go; then
> >> + # back up to menu
> >> + db_progress STOP
> >> + exit 10
> >> +fi
> >> +db_get grub-installer/force-efi-extra-removable
> >> +if [ "$RET" != true ]; then
> >> + db_progress STOP
> >> + exit 0
> >> +fi
> >> +
> >> +db_progress STEP 1
> >> +db_progress INFO grub-installer/progress/step_mount_filesystems
> >> +
> >> +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
> >
> >in-target?
>
> Hmmm, maybe. I saw a lot of calls to chroot directly in the
> grub-installer code, and almost none using in-target. There aren't any
> other uses of in-target in the rescue scripts either. I was just
> following existing convention, but I'm happy to switch if it makes
> sense.
Nah, matching the surrounding code looks like a good idea.
>
>
> Thanks for the review!
You're very welcome.
> Are you happy with the general approach I'm using?
Well, given a few bug reports and (ISTR) some feedback from
us
Bug#767037: Grub EFI fallback - patches for review
On Mon, Dec 01, 2014 at 03:52:51PM +0100, Cyril Brulebois wrote:
>Steve McIntyre (2014-12-01):
>> Control: severity 767037 serious
>> Control: tag 767037 +patch
>>
>> [ Raising severity to serious as I've heard more and more reports of
>> the problems here recently. ]
>>
>> Hi folks,
>>
>> i have two patches attached here, one for grub and one for
>> grub-installer. They implement the logic I described below and are
>> reasonably self-contained and non-intrusive. Please check them over
>> and give feedback, I'd like to get these in ASAP.
>
>Some comments inline + there seems to be little to no consistency as far
>as “{U,}EFI removable path” is concerned. Maybe make that uniform across
>the patches to reduce user bewilderment?
Hmmm, you're right. There's some existing inconsistencies already,
which don't help. In various places we already use "EFI" (e.g. in the
GRUB package names, EFI System Partition etc.) but in others it's
UEFI. Maybe we'd be better with just EFI everywhere...?
>Did you send the templates for review through debian-l10n-english?
Nope. They're currently entirely my own set of mistakes written in the
early hours... :-)
>> +@@ -846,6 +876,7 @@ main (int argc, char *argv[])
>> + char *relative_grubdir;
>> + char **efidir_device_names = NULL;
>> + grub_device_t efidir_grub_dev = NULL;
>> ++ char *base_efidir = NULL;
>> + char *efidir_grub_devname;
>> + int efidir_is_mac = 0;
>> + int is_prep = 0;
>> +@@ -878,6 +909,9 @@ main (int argc, char *argv[])
>> + bootloader_id = xstrdup ("grub");
>> + }
>> +
>> ++ if (removable && force_extra_removable)
>> ++grub_util_error (_("Invalid to use both --removable and
>> --force_extra_removable"));
>> ++
>> + if (!grub_install_source_directory)
>> + {
>> + if (!target)
>> +@@ -1087,6 +1121,8 @@ main (int argc, char *argv[])
>> + if (!efidir_is_mac && grub_strcmp (fs->name, "fat") != 0)
>> +grub_util_error (_("%s doesn't look like an EFI partition.\n"), efidir);
>> +
>> ++ base_efidir = xstrdup(efidir);
>Needs a matching free()?
I wish it was that easy - the code in grub-install.c is a mix of
styles here. Some allocations are freed immediately, while some stay
around for the life of the program. This needs to be one of the
latter, I think.
>> --- a/debian/templates.in
>> +++ b/debian/templates.in
>> @@ -12,6 +12,17 @@ _Description: Linux default command line:
>> The following string will be used as Linux parameters for the default menu
>> entry but not for the recovery mode.
>>
>> +Template: grub2/force_efi_extra_removable
>> +Type: boolean
>> +Default: false
>> +_Description: Force extra installation to the EFI removable path?
>> + Some UEFI-based systems are buggy and do not handle new bootloaders
>> correctly.
>> + If you force extra installation of GRUB to the EFI removable path, it
>> should
>> + make sure that this system will boot Debian correctly despite such a
>> problem.
>> + However, this may remove the ability to boot any other operating systems
>> that
>> + also depend on this path. If so, uou will need to ensure that GRUB is
>
>you
ACK, good catch.
>> diff --git a/rescue.d/81grub-efi-force-removable
>> b/rescue.d/81grub-efi-force-removable
>> new file mode 100644
>> index 000..b35b724
>> --- /dev/null
>> +++ b/rescue.d/81grub-efi-force-removable
>> @@ -0,0 +1,91 @@
>> +#! /bin/sh -e
>> +
>> +. /usr/share/debconf/confmodule
>> +
>> +. /usr/share/grub-installer/functions.sh
>> +
>> +log () {
>> +logger -t grub-installer "grub-efi-force-removable $@"
>> +}
>> +
>> +error () {
>> +log "error: $@"
>> +}
>> +
>> +die () {
>> +local template="$1"
>> +shift
>> +
>> +error "$@"
>> +db_input critical "$template" || [ $? -eq 30 ]
>> +db_go || true
>> +exit 1
>> +}
>> +
>> +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
>> +fi
>> +}
>> +
>> +db_progress START 0 3 grub-installer/progress/title
>> +db_progress INFO grub-installer/progress/step_force_efi
>> +
>> +# Should we also install grub-efi to the removable media path?
>> +# Ask the user
>> +log "Prompting user about removable media path"
>> +db_input high grub-installer/force-efi-extra-removable
>> +if ! db_go; then
>> +# back up to menu
>> +db_progress STOP
>> +exit 10
>> +fi
>> +db_get grub-installer/force-efi-extra-removable
>> +if [ "$RET" != true ]; then
>> +db_progress STOP
>> +exit 0
>> +fi
>> +
>> +db_progress STEP 1
>> +db_progress INFO grub-installer/progress/step_mount_filesystems
>> +
>> +log "Mounting fi
Bug#767037: Grub EFI fallback - patches for review
Steve McIntyre (2014-12-01):
> Control: severity 767037 serious
> Control: tag 767037 +patch
>
> [ Raising severity to serious as I've heard more and more reports of
> the problems here recently. ]
>
> Hi folks,
>
> i have two patches attached here, one for grub and one for
> grub-installer. They implement the logic I described below and are
> reasonably self-contained and non-intrusive. Please check them over
> and give feedback, I'd like to get these in ASAP.
Some comments inline + there seems to be little to no consistency as far
as “{U,}EFI removable path” is concerned. Maybe make that uniform across
the patches to reduce user bewilderment?
Did you send the templates for review through debian-l10n-english?
> I saw almost zero response to my previous mail... :-/
>
> For this to work sensibly, we will need both patches applying.
>
> On Thu, Nov 20, 2014 at 01:49:55AM +, Steve McIntyre wrote:
> >
> >Hi folks,
> >
> >All of these bugs look to be the same issue: dealing with broken UEFI
> >implementations that don't find/boot a *correctly* installed grub-efi
> >loader in \EFI\debian\grubx64.efi for some reason. There's multiple
> >parts to fixing this for Debian, I think, and I'll spell them out
> >here. Please comment and tell me I'm wrong if you think I am!
> >
> > 1. Add extra support in the grub-efi*(?) packages. When we
> >install/update a grub-efi boot image (grub*.efi), add the option
> >of *also* installing that image to the removable media path:
> >\EFI\boot\boot$ARCH.efi. This code should *not* be enabled by
> >default (as correctly-functioning UEFI implementations will not
> >need it), but we should add a debconf variable to enable it. Thus,
> >this can be configured elsewhere or even pre-seeded at
> >installation time if desired. As newer, future versions of grub
> >are installed, we should ensure that the copy of grub in the
> >removable media path is updated in sync.
> >
> >(Why do we need to update the removable media path copy? To ensure
> >that the loader portion there and the rest of the grub modules,
> >config etc. remain in sync. Otherwise, badness...)
> >
> > 2. Add support (question, template, etc.) in grub-installer to set
> >the new debconf variable. This should be at low priority so most
> >people won't see it, but it's available in expert mode or (again)
> >for pre-seed use.
> >
> > 3. Add an extra path through the grub-installer code for *rescue*
> >mode: "Did you install a UEFI system but your Debian system did
> >not boot?" which can set the new debconf variable and then call
> >the normal "reinstall grub" code. We'll need to make sure we warn
> >people that this will over-write any existing UEFI bootloaders on
> >their system, so if they still want to use their old Windows
> >install dual-boot etc. they need to make sure it's configured for
> >booting via Grub.
> >
> >Ideally, it would be lovely if we can somehow guess/determine
> >automatically that there is a Debian UEFI installation on the
> >system and offer this new rescue option automatically only where
> >it makes sense. Not sure how hard/easy that would be right now,
> >before investigating the code further.
> >
> > 4. Add documentation to the installation guide to take people through
> >this process: "If you have a broken UEFI implementation on your
> >computer, then here's how to recognise it and here's what to do to
> >work around it.."
> >
> >Go on, what have I missed / misunderstood / got wrong?
> >
> >--
> >Steve McIntyre, Cambridge, UK.
> >st...@einval.com
> > Armed with "Valor": "Centurion" represents quality of Discipline,
> > Honor, Integrity and Loyalty. Now you don't have to be a Caesar to
> > concord the digital world while feeling safe and proud.
> >
> >
> >--
> >To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org
> >with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
> >Archive: https://lists.debian.org/20141120014955.ga28...@einval.com
> >
> >
> --
> Steve McIntyre, Cambridge, UK.st...@einval.com
> "...In the UNIX world, people tend to interpret `non-technical user'
> as meaning someone who's only ever written one device driver." -- Daniel Pead
> >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001
> From: Steve McIntyre
> Date: Mon, 1 Dec 2014 11:37:06 +
> Subject: [PATCH] Add support for forcing EFI installation to the removable
> media path
>
> Add an extra option to grub-install "--force-extra-removable". On EFI
> platforms, this will cause an extra copy of the grub-efi image to be
> written to the appropriate removable media patch
> /boot/efi/EFI/BOOT/BOOT$ARCH.EFI as well. This will help with broken
> UEFI implementations where the firmware does not work when configured
> with new boot paths.
>
> Also added new debconf logic to a
Bug#767037: Grub EFI fallback - patches for review
Control: severity 767037 serious Control: tag 767037 +patch [ Raising severity to serious as I've heard more and more reports of the problems here recently. ] Hi folks, i have two patches attached here, one for grub and one for grub-installer. They implement the logic I described below and are reasonably self-contained and non-intrusive. Please check them over and give feedback, I'd like to get these in ASAP. I saw almost zero response to my previous mail... :-/ For this to work sensibly, we will need both patches applying. On Thu, Nov 20, 2014 at 01:49:55AM +, Steve McIntyre wrote: > >Hi folks, > >All of these bugs look to be the same issue: dealing with broken UEFI >implementations that don't find/boot a *correctly* installed grub-efi >loader in \EFI\debian\grubx64.efi for some reason. There's multiple >parts to fixing this for Debian, I think, and I'll spell them out >here. Please comment and tell me I'm wrong if you think I am! > > 1. Add extra support in the grub-efi*(?) packages. When we >install/update a grub-efi boot image (grub*.efi), add the option >of *also* installing that image to the removable media path: >\EFI\boot\boot$ARCH.efi. This code should *not* be enabled by >default (as correctly-functioning UEFI implementations will not >need it), but we should add a debconf variable to enable it. Thus, >this can be configured elsewhere or even pre-seeded at >installation time if desired. As newer, future versions of grub >are installed, we should ensure that the copy of grub in the >removable media path is updated in sync. > >(Why do we need to update the removable media path copy? To ensure >that the loader portion there and the rest of the grub modules, >config etc. remain in sync. Otherwise, badness...) > > 2. Add support (question, template, etc.) in grub-installer to set >the new debconf variable. This should be at low priority so most >people won't see it, but it's available in expert mode or (again) >for pre-seed use. > > 3. Add an extra path through the grub-installer code for *rescue* >mode: "Did you install a UEFI system but your Debian system did >not boot?" which can set the new debconf variable and then call >the normal "reinstall grub" code. We'll need to make sure we warn >people that this will over-write any existing UEFI bootloaders on >their system, so if they still want to use their old Windows >install dual-boot etc. they need to make sure it's configured for >booting via Grub. > >Ideally, it would be lovely if we can somehow guess/determine >automatically that there is a Debian UEFI installation on the >system and offer this new rescue option automatically only where >it makes sense. Not sure how hard/easy that would be right now, >before investigating the code further. > > 4. Add documentation to the installation guide to take people through >this process: "If you have a broken UEFI implementation on your >computer, then here's how to recognise it and here's what to do to >work around it.." > >Go on, what have I missed / misunderstood / got wrong? > >-- >Steve McIntyre, Cambridge, UK.st...@einval.com > Armed with "Valor": "Centurion" represents quality of Discipline, > Honor, Integrity and Loyalty. Now you don't have to be a Caesar to > concord the digital world while feeling safe and proud. > > >-- >To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org >with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org >Archive: https://lists.debian.org/20141120014955.ga28...@einval.com > > -- Steve McIntyre, Cambridge, UK.st...@einval.com "...In the UNIX world, people tend to interpret `non-technical user' as meaning someone who's only ever written one device driver." -- Daniel Pead >From e384e597914b6e1b1dcbf96ef6782cf9bcc2313b Mon Sep 17 00:00:00 2001 From: Steve McIntyre Date: Mon, 1 Dec 2014 11:37:06 + Subject: [PATCH] Add support for forcing EFI installation to the removable media path Add an extra option to grub-install "--force-extra-removable". On EFI platforms, this will cause an extra copy of the grub-efi image to be written to the appropriate removable media patch /boot/efi/EFI/BOOT/BOOT$ARCH.EFI as well. This will help with broken UEFI implementations where the firmware does not work when configured with new boot paths. Also added new debconf logic to add this extra option to grub-install calls when grub2/force_efi_extra_removable is set true. This allows other programs like d-i / grub-installer to configure this for general use. --- debian/changelog | 4 + debian/config.in | 1 + debian/patches/grub-install-extra-removable.patch | 115 ++ debian/patches/series | 1 + debian/postinst.in
