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

storage: Be smart about default filesystem type #19854

Merged

Conversation

mvollmer
Copy link
Member

@mvollmer mvollmer commented Jan 18, 2024

No description provided.

@mvollmer mvollmer force-pushed the storage-better-default-fsys-type branch 3 times, most recently from eaf4cdd to cee85e1 Compare January 19, 2024 08:07
@mvollmer mvollmer marked this pull request as ready for review January 19, 2024 11:33
@mvollmer mvollmer force-pushed the storage-better-default-fsys-type branch from cee85e1 to 75b4b53 Compare January 19, 2024 13:43
@mvollmer mvollmer requested a review from jelly January 22, 2024 08:47
@jelly
Copy link
Member

jelly commented Jan 22, 2024

The code looks good to me, but I have questions about changing the default behavior from recommending xfs to now silently falling back to ext4. I wonder what the argument for the default being ext4 is? I know xfs is the RHEL blessed fs.

jelly
jelly previously approved these changes Jan 22, 2024
@mvollmer
Copy link
Member Author

I wonder what the argument for the default being ext4 is? I know xfs is the RHEL blessed fs.

This fallback should be exceedingly rare. Normally, the default is taken from the root filesystem, which is xfs on RHEL.

Only if Cockpit does not support the filesystem type of the root filesystem (or can't find the root fs due to bugs) will it fall back to ext4.

We can of course have a different fallback since it shouldn't really matter. I chose ext4 because I think it's the most standard Linux filesystem.

@jelly
Copy link
Member

jelly commented Jan 23, 2024

Since my approval the pixel tests have changed, I'm happy to re-approve once that's fixed.

@@ -155,6 +157,17 @@ export function format_dialog(client, path, start, size, enable_dos_extended) {
}
}

function find_root_fsys_block() {
const root = client.anaconda?.mount_point_prefix || "/";
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

@jelly jelly merged commit 37fe4a4 into cockpit-project:main Jan 23, 2024
91 of 93 checks passed
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