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

Properly update MBR when installing with saved partitions #378

Merged
merged 4 commits into from Sep 21, 2020
Merged

Properly update MBR when installing with saved partitions #378

merged 4 commits into from Sep 21, 2020

Conversation

bgilbert
Copy link
Contributor

When installing with saved partitions, we were failing to clear the MBR boot code during install and on error, and failing to write the new boot code afterward. Fixes regressions in #340.

Also update the size of the protective MBR partition after install, for consistency with the GPT.

https://bugzilla.redhat.com/show_bug.cgi?id=1879690

GPT::write_protective_mbr_into() only writes the partition table, leaving
the MBR boot code alone.  Manually zero out the remainder of the MBR to
prevent attempted boots via the bootloader we're overwriting.

Fixes: 8420896 ("install: on error, just write a GPT")
Fixes: ace675b ("download: defer validity using saved partitions rather than zeroes")
In the case of saved partitions, image_copy_default() was writing a
merged GPT and copying any data after it, but wasn't copying the MBR.
With the previous commit, this would result in a BIOS-unbootable install;
before the previous commit, this would result in a BIOS boot using the
bootloader previously installed to the disk.

Fixes: 66ffb81 ("download: restore saved partitions while writing back first MiB")
If we merge saved partitions, we update the installed GPT to match the
size of the target disk; otherwise, we don't.  We were not updating the
protective MBR to match the target disk size in either case, which is
inconsistent.  Have merge() update the protective MBR as well.
Copy link
Contributor

@lucab lucab left a comment

Choose a reason for hiding this comment

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

LGTM (and thanks for the unit tests)

Copy link
Member

@ashcrow ashcrow left a comment

Choose a reason for hiding this comment

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

👍

@bgilbert bgilbert merged commit 42a3e0d into coreos:master Sep 21, 2020
@bgilbert bgilbert deleted the mbr branch September 21, 2020 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants