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

Specify nrext64 option when creating XFS filesystem #3511

Merged
merged 4 commits into from
Nov 29, 2023

Conversation

mulkieran
Copy link
Member

Closes #3504

@mulkieran mulkieran self-assigned this Nov 28, 2023
@mulkieran
Copy link
Member Author

To be very safe, we need to check the version of mkfs.xfs

@mulkieran mulkieran changed the title Issue stratisd 3504 Specify nrext64 option when creating XFS filesystem Nov 28, 2023
@mulkieran mulkieran requested a review from bgurney-rh November 28, 2023 21:14
@mulkieran mulkieran marked this pull request as ready for review November 28, 2023 21:14
@mulkieran
Copy link
Member Author

Comment fix, nothing more.

@mulkieran
Copy link
Member Author

  • an error string fix, some were just empty strings before...

// package availabe that necessarily adheres to the xfsprogs versioning
// scheme.
let use_nrext64_option = match get_mkfs_xfs_version().and_then(|v| {
v.split('.')
Copy link
Member

Choose a reason for hiding this comment

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

We may not want to add this now, but this crate would simplify this code a lot. https://docs.rs/semver/latest/semver/

Copy link
Member Author

Choose a reason for hiding this comment

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

We already depend on semver transitively. I can try using semver, but I was kind of against doing so originally, because it uses the specific crate semver rules, and I was worried that someday a version of xfsprogs would use some non-standard format and break the version parsing part. But then, I guess, we would just log the error and add in the command-line option anyway, so there's really no good reason to avoid using semver. I'll give it a go.

Copy link
Member

Choose a reason for hiding this comment

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

Okay sounds good!

stratisd never uses it anymore, not since the size of the MDV was
increased.

Signed-off-by: mulhern <[email protected]>
@mulkieran mulkieran removed the request for review from bgurney-rh November 29, 2023 20:22
@mulkieran
Copy link
Member Author

Tests check out...

@mulkieran mulkieran merged commit 0473843 into stratis-storage:master Nov 29, 2023
43 checks passed
@mulkieran mulkieran deleted the issue_stratisd_3504 branch November 29, 2023 20:23
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.

Avoid creating XFS filesystems with 64 bit extent count
3 participants