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

filesystem: Use the first fitting gap for ESP, instead of the largest gap #1764

Closed
wants to merge 1 commit into from

Conversation

medicalwei
Copy link
Contributor

@medicalwei medicalwei commented Aug 16, 2023

This is to handle the issue, that when using recovery (reset) partition to install to the same disk, it:

  1. removes all partitions except recovery partition on the target disk, then
  2. picks the largest gap to install ESP, usually after the recovery partition.

This should not affect disk installation where the disk partitions are all removed, but affects installations where there are some partitions exist but not removed.

Copy link
Collaborator

@mwhudson mwhudson left a comment

Choose a reason for hiding this comment

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

Thanks for this, it looks ok on my phone. Will look properly tomorrow

gap = gaps.largest_gap(device, in_extended=False)
if gap is not None and gap.size >= size and gap.is_usable:
# find the first available gap (not always the largest one)
gap = next(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I generally prefer explicit for loops to this kind of thing but well

Copy link
Collaborator

@dbungert dbungert left a comment

Choose a reason for hiding this comment

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

This is an improvement but I think we should relocate this to gaps.py at some point. Can easily imagine other cases of wanting a gap that supports a certain size instead of always demanding the largest.

Note that this code doesn't check the criteria where a gap would be unusable (TOO_MANY_PRIMARY_PARTS) but I guess that's not a scenario here.

@dbungert
Copy link
Collaborator

@medicalwei Thanks for the PR, but I think it's obsolete given the implementation in #1765

@medicalwei
Copy link
Contributor Author

Closing this in favor of #1765, thanks!

@medicalwei medicalwei closed this Aug 18, 2023
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.

3 participants