Only enforce /boot location limit on non-EFI systems (#2391443)#1437
Merged
vojtechtrefny merged 1 commit intostoraged-project:mainfrom Nov 28, 2025
Merged
Conversation
We currently require the /boot partition to be placed in a way the last sector is under 2 TiB. This is an old check that predates EFI and GPT and should not apply on these systems.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSkips the legacy 2 TiB end-sector constraint for /boot partition placement when running on EFI systems, while preserving the limit on non-EFI systems. Flow diagram for applying /boot 2_TiB limit based on EFI detectionflowchart TD
A[get_best_free_space_region called] --> B[Is /boot partition requested? boot flag]
B -- No --> C[Skip /boot size constraint]
C --> D[Evaluate free space region normally]
B -- Yes --> E[Call is_efi]
E -- System is EFI --> F[Do not enforce 2_TiB /boot end-sector limit]
F --> D
E -- System is not EFI --> G[Set max_boot to 2_TiB]
G --> H[Compute free_start from free_geom and sectorSize]
H --> I[Compute req_end as free_start + req_size]
I --> J[Check that req_end <= max_boot]
J --> K[If req_end exceeds max_boot, reject this free space region]
K --> D
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Using
is_efi()here ties the /boot constraint to the environment where blivet runs rather than the target system; if this code can operate on non-local/alternate targets (e.g., image creation, chroot installs), consider plumbing a target-boot-mode flag intoget_best_free_space_regioninstead of callingis_efi()directly. - It might be helpful to add a brief code comment explaining why the 2 TiB limit is skipped on EFI (i.e., historical BIOS/MBR constraint) so future readers don’t reintroduce or refactor this check without understanding the context.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `is_efi()` here ties the /boot constraint to the environment where blivet runs rather than the target system; if this code can operate on non-local/alternate targets (e.g., image creation, chroot installs), consider plumbing a target-boot-mode flag into `get_best_free_space_region` instead of calling `is_efi()` directly.
- It might be helpful to add a brief code comment explaining why the 2 TiB limit is skipped on EFI (i.e., historical BIOS/MBR constraint) so future readers don’t reintroduce or refactor this check without understanding the context.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We currently require the /boot partition to be placed in a way the last sector is under 2 TiB. This is an old check that predates EFI and GPT and should not apply on these systems.
Summary by Sourcery
Bug Fixes: