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

disk: AddPartitionsForBootMode() helper function (COMPOSER-2355) #1024

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

achilleas-k
Copy link
Member

This is part of #926 which I'm slowly splitting into smaller, bite-sized PRs.


The PR introduces a helper function, AddPartitionsForBootMode(). The function takes a partition table and boot mode and adds partitions to satisfy the boot mode requirements:

  • BIOS/legacy: adds a 1 MiB BIOS boot partition.
  • UEFI: adds a 200 MiB EFI system partition.
  • Hybrid: adds both.

Add a function that takes a partition table and adds partitions to
satisfy the boot mode requirements:
- BIOS/legacy: adds a 1 MiB BIOS boot partition.
- UEFI: adds a 200 MiB EFI system partition.
- Hybrid: adds both.
Define two new const strings: DosBIOSBootID and DosESPID, which are used
for the BIOS boot and the EFI system partitions respectively on dos/mbr
partition tables.

In the new helper functions, use the partition table type to determine
which ID to use.  This means that the functions will return an error
when the partition table type is not set (or is set to an unknown
value).
@achilleas-k achilleas-k changed the title Disk/pt/bootmode disk: AddPartitionsForBootMode() helper function Nov 7, 2024
mvo5
mvo5 previously approved these changes Nov 7, 2024
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thank you, this looks very nice! One tiny suggestion about a possible unhappy test but not a blocker, it's not really criticial.

pkg/disk/partition_table.go Outdated Show resolved Hide resolved
The previous error message was printing an invalid enum value as a
string, which would cause a panic in the BootMode.String() method.

Update the error in the AddPartitionsForBootMode() function and add a
test.
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Super nice!

@mvo5 mvo5 enabled auto-merge November 7, 2024 16:05
@mvo5 mvo5 added this pull request to the merge queue Nov 7, 2024
Merged via the queue into osbuild:main with commit 863bb93 Nov 7, 2024
19 checks passed
@achilleas-k achilleas-k deleted the disk/pt/bootmode branch November 7, 2024 17:50
@achilleas-k achilleas-k changed the title disk: AddPartitionsForBootMode() helper function disk: AddPartitionsForBootMode() helper function (COMPOSER-2355) Nov 13, 2024
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.

2 participants