Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create_disk: move /boot to partition 3; move BIOS-BOOT to 1 and always create it #1820

Merged
merged 5 commits into from
Nov 12, 2020

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Oct 30, 2020

The offset of the BIOS boot partition gets encoded into the boot sector, and that offset is currently 512 MiB, which is awkward if we're going to support rearranging boot-related partitions. Move the BIOS boot partition to partition 1 at the beginning of the disk, and move /boot to partition 3 on all platforms (including ppc64le, where it was overridden to partition 2).

In addition, create an unused BIOS boot partition on 4Kn disks. This reduces the number of distinct partition layouts we need to support.

For coreos/fedora-coreos-config#718, but shouldn't be required by it. Closes coreos/fedora-coreos-tracker#669.

@openshift-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@jlebon
Copy link
Member

jlebon commented Oct 30, 2020

The offset of the BIOS boot partition gets encoded into the boot sector, and that offset is currently 512 MiB, which is awkward if we're going to support rearranging boot-related partitions.

Just to make sure I follow, can you expand on this a little more? Is it that by having it be the first partition, it makes it less likely that it would be moved around and so we avoid having to update the MBR?

@bgilbert
Copy link
Contributor Author

We can't update the boot sector unless we want to rerun grub-install from the initrd or hand-hack the bits, neither of which is appealing. So a recreated BIOS-BOOT partition needs to remain at the same offset as the original. Putting it at the beginning of the disk is relatively natural. Putting it at 512 MiB, as we do now, means we'd need to hardcode that constant into the FCCT desugaring code that lays out the new partitions.

@bgilbert bgilbert changed the title WIP: create_disk: move BIOS-BOOT to partition 1 and /boot to 3 create_disk: move BIOS-BOOT to partition 1 and /boot to 3; always create BIOS-BOOT Nov 9, 2020
@bgilbert bgilbert changed the title create_disk: move BIOS-BOOT to partition 1 and /boot to 3; always create BIOS-BOOT create_disk: move /boot to partition 3; move BIOS-BOOT to 1 and always create it Nov 9, 2020
@bgilbert bgilbert marked this pull request as ready for review November 9, 2020 04:58
@bgilbert
Copy link
Contributor Author

bgilbert commented Nov 9, 2020

Ready for review.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment, but LGTM overall!

I think it's worth raising visibility on this somehow because this will break anyone/anything which currently uses e.g. /dev/sda1 for the boot partition instead of by-label (I'm definitely guilty of that myself). WDYT about a ticket in the tracker which says that we want to move the /boot partition and linking to this PR (or better, having this PR close it)?

src/create_disk.sh Outdated Show resolved Hide resolved
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Somewhat superficial) LGTM

@bgilbert
Copy link
Contributor Author

/test sanity

@bgilbert
Copy link
Contributor Author

Updated!

@jlebon
Copy link
Member

jlebon commented Nov 12, 2020

Awesome, thanks for opening the tracker issue!
Restarted CI.

/lgtm

The offset of the BIOS boot partition gets encoded into the boot sector,
and that offset is currently 512 MiB, which is awkward if we're going
to support rearranging boot-related partitions.  Move the BIOS boot
partition to partition 1 at the beginning of the disk, and move /boot to
partition 3 on all platforms (including ppc64le, where it was overridden
to partition 2).
Reduce the number of partition layouts we create, in order to simplify
the maintenance of Ignition configs that affect the default partition
table.  Continue not installing BIOS GRUB in the 4Kn image, since it'll
never be invoked.
@bgilbert
Copy link
Contributor Author

Rebased.

@jlebon
Copy link
Member

jlebon commented Nov 12, 2020

/lgtm

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bgilbert, cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [bgilbert,cgwalters,jlebon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit caff19a into coreos:master Nov 12, 2020
@bgilbert bgilbert deleted the raid branch November 13, 2020 01:53
dustymabe added a commit to dustymabe/fedora-coreos-config that referenced this pull request Dec 3, 2020
The boot filesystem was moved to partition 3 recently. Let's
update the code to be a little more dynamic about finding the
boot paritition so we can be forward and backward compatible.

For more context see:

- coreos/fedora-coreos-tracker#669
- coreos/coreos-assembler#1820
cgwalters pushed a commit to coreos/fedora-coreos-config that referenced this pull request Dec 3, 2020
The boot filesystem was moved to partition 3 recently. Let's
update the code to be a little more dynamic about finding the
boot paritition so we can be forward and backward compatible.

For more context see:

- coreos/fedora-coreos-tracker#669
- coreos/coreos-assembler#1820
kelvinfan001 pushed a commit to kelvinfan001/fedora-coreos-config that referenced this pull request Dec 14, 2020
The boot filesystem was moved to partition 3 recently. Let's
update the code to be a little more dynamic about finding the
boot paritition so we can be forward and backward compatible.

For more context see:

- coreos/fedora-coreos-tracker#669
- coreos/coreos-assembler#1820
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move /boot filesystem to partition 3
5 participants