bsdinstall: when installing UEFI, place the bootloader on all disks
Needs ReviewPublic
Actions

Authored by asomers on Oct 10 2023, 8:16 PM.

Details

Reviewers

dteske
brd
jrtc27

Summary

Previously, bsdinstall would partition all disks used by the zroot
pool, but would only format and install the boot partition on one.

Reported by: Julien Cigar <julien@perdition.city> and many others
MFC after: 1 week
Sponsored by: Axcient

Test Plan

Running in production with mirrored zroot pools

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 57364
Build 54252: arc lint + arc unit

Event Timeline

asomers created this revision.Oct 10 2023, 8:16 PM

Herald added subscribers: jrtc27, imp. · View Herald TranscriptOct 10 2023, 8:16 PM

asomers requested review of this revision.Oct 10 2023, 8:16 PM

Harbormaster completed remote builds in B53920: Diff 128521.Oct 10 2023, 8:16 PM

jrtc27 requested changes to this revision.Oct 10 2023, 8:26 PM

jrtc27 added inline comments.

usr.sbin/bsdinstall/scripts/bootconfig
182	UFS autopartitioning doesn't have a thing, so this glob will evaluate to a literal /dev/gpt/efiboot*. Encoding this ZFS specificity in here also seems a bit ugly.

This revision now requires changes to proceed.Oct 10 2023, 8:26 PM

asomers added inline comments.Oct 10 2023, 8:29 PM

usr.sbin/bsdinstall/scripts/bootconfig
182	UFS autopartitioning doesn't have a thing, so this glob will evaluate to a literal /dev/gpt/efiboot*. Encoding this ZFS specificity in here also seems a bit ugly. Good point. We should check for a case like that. For that matter, is there anything to do here for UFS? Do we even support installing to gmirror+UFS ?

jrtc27 added inline comments.Oct 10 2023, 8:36 PM

usr.sbin/bsdinstall/scripts/bootconfig
182	Only via manual partitioning as far as I know, at which point you'd need more infrastructure to mark any secondary ESPs which we just don't have an interface for.

guest-jsollvander added a subscriber: guest-jsollvander.Oct 11 2023, 7:24 AM

I see that bootconfig used to have code for adding ESPs to all ZFS/UFS boot disks, but it was removed in https://reviews.freebsd.org/D28897. @imp reviewed that one, maybe you could share your opinion here if we should bring parts of that code back? I think all disks that are configured as boot disks by the installer should a working ESP after installation. If the wrong boot disks die then you are left with a system without any ESPs at all.

imp added inline comments.Oct 18 2023, 3:57 PM

usr.sbin/bsdinstall/scripts/bootconfig
183	Where do we actually label things like this? And isn't this super fragile?
190	This wins the award for the ugliest kludge to make this work. I'd have been tempted to pass in mntpt at the very least and mount the alternatives on different mount points to avoid 'stacking' of the mount points which the cleanup routine wouldn't unwind.

jrtc27 added inline comments.Oct 18 2023, 3:59 PM

usr.sbin/bsdinstall/scripts/bootconfig
183	See my comment above (zfsboot script)

Add ESP to all ZFS boot disks, pretty edition

Harbormaster completed remote builds in B54161: Diff 129371.Oct 24 2023, 10:41 PM

This latest version fixes the script with UFS, and makes the bootconfig script less ZFS-specific.

ping @jrtc27 does the latest version look better to you?

rcm added a subscriber: rcm.Dec 11 2023, 5:13 PM

guest-patmaddox added a subscriber: guest-patmaddox.Dec 13 2023, 10:02 PM

linnemannr_gmail.com added a subscriber: linnemannr_gmail.com.Dec 21 2023, 4:58 PM

linnemannr_gmail.com added inline comments.

usr.sbin/bsdinstall/scripts/bootconfig
148–152	This seems unnecessary to me. We're not concerned with the actual number of devices (which can be gotten without a shell loop using wc maybe?), just that we have a plurality of them. This can be determined just by looking for whitespace in $ESPS.

bsdinstall: all ESPs created by zfsboot should have a bootloader

Harbormaster completed remote builds in B57364: Diff 137675.Apr 25 2024, 5:16 PM

To be clear: it wasn't me doing the following work, it was a colleague:

This problem bit us on a production server on this past Friday. The host had been recently updated from FreeBSD 14.0 to 14.1 - it had been rebooted multiple times since and during that update. On Friday morning, it did not come back from a reboot. Getting it back was troublesome. It was eventually discovered that one of the two boot drives was not 'intact'. While it had an EFI partition, that partition did not contain a filesystem. Once the filesystem was created and the files copied over (while booting from a live CD), we recovered the host and went on with our weekend.

Sorry to bother people, but could we revive this review? I was recently talking to some people who noticed this the hard way and it would be great to get something merged that fixes it, even if it isn't perfect. I think this looks good enough and is definitely not "the ugliest kludge to make this work"

mfburdett_gmail.com added a subscriber: mfburdett_gmail.com.Jan 19 2025, 8:05 AM

What's the state of this? Has it been resolved in HEAD or do we still have this problem where not all zpool mirror disks get the ESP boot code (only the first one)?

AFAIK this is still a problem

Yea, I think if we put it on the partitions that we create, and only those, the issues that caused it to be remvoed before will be solved. I think this does that, but I've not looked closely enough to know for sure.

This did not land in time for 14.3, but I'd like to make sure this gets fixed for 15.0 and 14.4. Can someone take ownership of this and resolve any issues so it can land?

In D42150#1164372, @cperciva wrote:

This did not land in time for 14.3, but I'd like to make sure this gets fixed for 15.0 and 14.4. Can someone take ownership of this and resolve any issues so it can land?

I can volunteer for this. There's only a few people that have all the right contexts for this.

In D42150#1164398, @imp wrote:

I can volunteer for this. There's only a few people that have all the right contexts for this.

Thank you!

In D42150#1164413, @cperciva wrote:

In D42150#1164398, @imp wrote:

I can volunteer for this. There's only a few people that have all the right contexts for this.

Thank you!

I think it just installs it on all the devices that were used to create the root pool, so that wouldn't need a prompt. But if it's more invasive than that, it does need a prompt.