Page MenuHomeFreeBSD

Fix install-boot.sh to work with rootgen.sh
ClosedPublic

Authored by bcran on Apr 20 2019, 8:50 PM.

Details

Summary

tools/boot/install-boot.sh was assuming that if a device was passed in,
it should operate on the current system and run efibootmgr etc. to
update the boot manager. However, rootgen.sh passes a md(4) device and
not a fixed disk.

Add a -n option to install-boot.sh to tell it to operate in 'offline'
mode and not try to update the current system.

Also, source install-boot.sh in rootgen.sh to allow it to find and
call make_esp_file etc.

Test Plan

Ran tools/boot/rootgen.sh and verified it appeared to work correctly.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bcran created this revision.Apr 20 2019, 8:50 PM
bcran updated this revision to Diff 56439.Apr 20 2019, 8:54 PM
  • Revert unrelated change
tsoome added inline comments.Apr 20 2019, 8:55 PM
tools/boot/install-boot.sh
359 ↗(On Diff #56438)

IMO the -n is a bit bad (confusing) choice there; -n often means "do not write" or perform dry run.

maybe better once would be -i for operate on image file.

imp added a comment.Apr 20 2019, 10:42 PM

I like this, but command line switches are too easy to quibble over, so I'll just have my say and let you decide.

I've also started to create a 'fallback loader' path to try to hash out a good design. I'm thinking that a good design might be to have loader.efi read in \efi\freebsd\loader.env and parse it like the command line with foo=bar variables. This would let us say uefi_rootdev=HD(3,GPT,0f91d7c6-2de5-11e8-b5f1-3cfdfe9d5250,0x619430,0x600000) and have that become / for the new system. The only downside to this is that we have to encode the size and offset of the partition into the device path which doesn't play well with the auto resize code, though we could hack that to update this env :)

tools/boot/install-boot.sh
359 ↗(On Diff #56438)

I tend to agree, but not sure -i is the right arg.
Maybe it would be better to have this be -e or -m to force running of efibootmgr instead?
On the other hand, if we take a step back, we are trying to distinguish two cases here: updating a system in place and installing into an image that will be transported elsewhere. 'offline' is really a poor substitute for this system vs elsewhere, so maybe some other word would be helpful here to describe it, and that may suggest the arg to use.

bcran added inline comments.Apr 21 2019, 12:40 AM
tools/boot/install-boot.sh
359 ↗(On Diff #56438)

I agree with -n being a bit confusing, but -i is also often 'interactive'. I might choose -m as a semi-random option that could also be 'memory disk'.
I'd prefer to leave the existing behavior as is, and _not_ require people to add a new option to operate in-place.

In D19992#429679, @imp wrote:

I've also started to create a 'fallback loader' path to try to hash out a good design. I'm thinking that a good design might be to have loader.efi read in \efi\freebsd\loader.env and parse it like the command line with foo=bar variables. This would let us say uefi_rootdev=HD(3,GPT,0f91d7c6-2de5-11e8-b5f1-3cfdfe9d5250,0x619430,0x600000) and have that become / for the new system. The only downside to this is that we have to encode the size and offset of the partition into the device path which doesn't play well with the auto resize code, though we could hack that to update this env :)

I don't understand how loader.env fits into the idea of a fallback loader, but it's perhaps better to wait for your design document.

imp added a comment.Apr 21 2019, 12:42 AM
In D19992#429679, @imp wrote:

I've also started to create a 'fallback loader' path to try to hash out a good design. I'm thinking that a good design might be to have loader.efi read in \efi\freebsd\loader.env and parse it like the command line with foo=bar variables. This would let us say uefi_rootdev=HD(3,GPT,0f91d7c6-2de5-11e8-b5f1-3cfdfe9d5250,0x619430,0x600000) and have that become / for the new system. The only downside to this is that we have to encode the size and offset of the partition into the device path which doesn't play well with the auto resize code, though we could hack that to update this env :)

I don't understand how loader.env fits into the idea of a fallback loader, but it's perhaps better to wait for your design document.

It's a persistent communication mechanism that can be used to configure behavior in the boot loader that might otherwise be controlled via UEFI variables.

In D19992#429688, @imp wrote:

It's a persistent communication mechanism that can be used to configure behavior in the boot loader that might otherwise be controlled via UEFI variables.

Oh, nice. It sounds like a similar concept to the fallback mechanism Linux uses (https://www.rodsbooks.com/efi-bootloaders/fallback.html).

bcran updated this revision to Diff 56443.Apr 21 2019, 5:00 AM
  • Change the install-boot.sh option to -u to cause it to run efibootmgr etc.
bcran marked an inline comment as done.Apr 21 2019, 5:01 AM
bcran added inline comments.
tools/boot/install-boot.sh
359 ↗(On Diff #56438)

I've changed it to -u to cause it to update the current system (i.e. run efibootmgr).

Could people review this again for the changes I've made, please?

imp accepted this revision.Apr 23 2019, 5:29 PM
This revision is now accepted and ready to land.Apr 23 2019, 5:29 PM
tsoome accepted this revision.Apr 23 2019, 5:43 PM
bcran updated this revision to Diff 56596.Apr 24 2019, 6:41 PM
This revision now requires review to proceed.Apr 24 2019, 6:41 PM
ian accepted this revision.Apr 24 2019, 10:14 PM
This revision is now accepted and ready to land.Apr 24 2019, 10:14 PM
This revision was automatically updated to reflect the committed changes.