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

Better handle filesystem optional features. #66

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cwize1
Copy link
Contributor

@cwize1 cwize1 commented Jan 7, 2025

Filesystems (e.g. ext4, xfs) sometimes receive new features. When these features are enabled, anything that loads the filesystem (e.g. kernel, grub, etc.) must support the new feature or it will refuse to load it.

When formatting a new partition, there are a few considerations for if a filesystem feature should be enabled:

  • Does the version of mkfs support that feature?
  • Does the build host kernel support that feature?
  • Does the target OS support that feature?

This change ensure that all these considerations are handled correctly for the ext4 and xfs filesystem types.


Checklist

  • Tests added/updated
  • Documentation updated (if needed)
  • Code conforms to style guidelines

Filesystems (e.g. ext4, xfs) sometimes receive new features. When these
features are enabled, anything that loads the filesystem (e.g. kernel,
grub, etc.) must support the new feature or it will refuse to load it.

When formatting a new partition, there are a few considerations for if a
filesystem feature should be enabled:

- Does the version of mkfs support that feature?
- Does the build host kernel support that feature?
- Does the target OS support that feature?

This change ensure that all these considerations are handled correctly
for the ext4 and xfs filesystem types.
@cwize1 cwize1 requested a review from a team as a code owner January 7, 2025 22:22
liulanze
liulanze previously approved these changes Jan 8, 2025
@@ -433,7 +423,7 @@ func WaitForDevicesToSettle() error {
}

// CreatePartitions creates partitions on the specified disk according to the disk config
func CreatePartitions(diskDevPath string, disk configuration.Disk, rootEncryption configuration.RootEncryption,
func CreatePartitions(targetOs targetos.TargetOs, diskDevPath string, disk configuration.Disk, rootEncryption configuration.RootEncryption,
Copy link
Member

Choose a reason for hiding this comment

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

An opinion here, since this is a new repo, there is a chance that as time goes on, we might end up adding more and more options here.
How about we have a struct with default values, and on top of that struct we implement a method CreateParititions.
This might help us in only modifying the code at a single place where we have the initial struct and the rest of the code might not need any modifications :)
Just an opinion

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.

4 participants