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

rbd: Use assume_storage_prezeroed when formatting #4996

Draft
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

black-dragon74
Copy link
Member

Describe what this PR does

Instead of passing lazy_itable_init=1 and lazy_journal_init=1 to mkfs.ext4,
pass assume_storage_prezeroed=1 which is stronger and allows the filesystem
to skip inode table zeroing completely instead of simply doing it lazily.

Closes: #4948

Instead of passing lazy_itable_init=1 and lazy_journal_init=1 to
mkfs.ext4, pass assume_storage_prezeroed=1 which is
stronger and allows the filesystem to skip inode table zeroing
completely instead of simply doing it lazily.

Closes: ceph#4948

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
@black-dragon74 black-dragon74 added the DNM DO NOT MERGE label Dec 3, 2024
@mergify mergify bot added the component/rbd Issues related to RBD label Dec 3, 2024
@@ -101,7 +101,7 @@ var (
xfsHasReflink = xfsReflinkUnset

mkfsDefaultArgs = map[string][]string{
"ext4": {"-m0", "-Enodiscard,lazy_itable_init=1,lazy_journal_init=1"},
"ext4": {"-m0", "-Enodiscard,assume_storage_prezeroed=1"},
Copy link
Member

Choose a reason for hiding this comment

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

make sure that you only do this when mkfs.ext4 supports the option, you'll need to add some form of detection, ideally with something like sync.Once.

Copy link
Contributor

Choose a reason for hiding this comment

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

assume_storage_prezeroed option became available in e2fsprogs 1.47.0 last year, so it should probably be "discovered" similar to xfsSupportsReflink().

My suggestion to do it similar to xfsSupportsReflink() isn't viable because mkfs.ext4 doesn't include extended options in its help output, but trying with assume_storage_prezeroed=1 and falling back to lazy_itable_init=1,lazy_journal_init=1 upon parsing out Bad option(s) specified: assume_storage_prezeroed error should work.

Copy link
Member Author

@black-dragon74 black-dragon74 Dec 27, 2024

Choose a reason for hiding this comment

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

I and Madhu had a discussion on this. Since this flag is part of e2fsprogs >= 1.47 and we have control over which version of it we bundle in the container, do we really need this check?

Sidenote: Detection is possible by parsing the semver of e2fsprogs. It is standard flag so it must be there?

Copy link
Contributor

Choose a reason for hiding this comment

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

The check for the version isn't always reliable in the face of downstream backports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/rbd Issues related to RBD DNM DO NOT MERGE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass assume_storage_prezeroed option when formatting ext4 if possible
3 participants