Skip to content

Comments

[master] Allow opt-out of discard on disk format#64310

Open
jseba wants to merge 3 commits intosaltstack:masterfrom
jseba:jseba/nodiscard_format
Open

[master] Allow opt-out of discard on disk format#64310
jseba wants to merge 3 commits intosaltstack:masterfrom
jseba:jseba/nodiscard_format

Conversation

@jseba
Copy link

@jseba jseba commented May 19, 2023

What does this PR do?

This allows disk images to be formatted explicitly non-sparsely by enabling callers to disable the automatic discard of blocks during filesystem creation.

This is only supported for ext and xfs filesystems.

What issues does this PR fix or reference?

Fixes:

Current behavior of formatting disk images results in e.g. a 20GB disk image being reduced to 44MB on the parent filesystem, which shows up as potential free space when other things are looking at the parent's disk usage. If the parent filesystem no longer has blocks to service the disk image's usage, it gets ENOSPC errors when it is not at 100% usage.

New Behavior

Callers to disk.format and blockdev.formatted can specify discard=False for ext and xfs filesystems to disable their respective mkfs tools from doing a block discard during filesystem creation. This ensures the disk image is still reserving blocks on the parent filesystem for itself if it was created non-sparsely.

Merge requirements satisfied?

  • Docs
  • Changelog
  • Tests written/updated

Commits signed with GPG?

No

@jseba jseba requested a review from a team as a code owner May 19, 2023 18:42
@jseba jseba requested review from dwoz and removed request for a team May 19, 2023 18:42
@welcome
Copy link

welcome bot commented May 19, 2023

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title Allow opt-out of discard on disk format [master] Allow opt-out of discard on disk format May 19, 2023
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

This looks good, but just needs a changelog entry (https://docs.saltproject.io/en/latest/topics/development/changelog.html)

@jseba jseba temporarily deployed to ci May 22, 2023 20:58 — with GitHub Actions Inactive
@jseba jseba temporarily deployed to ci May 22, 2023 20:59 — with GitHub Actions Inactive
@jseba jseba temporarily deployed to ci May 22, 2023 20:59 — with GitHub Actions Inactive
@jseba jseba temporarily deployed to ci May 22, 2023 23:01 — with GitHub Actions Inactive
@jseba jseba temporarily deployed to ci May 22, 2023 23:01 — with GitHub Actions Inactive
@jseba jseba temporarily deployed to ci May 22, 2023 23:01 — with GitHub Actions Inactive
@jseba jseba force-pushed the jseba/nodiscard_format branch from b756990 to cf72788 Compare June 30, 2023 17:16
@jseba
Copy link
Author

jseba commented Jun 30, 2023

I added the changelog entry, also went ahead and rebased on master

Ch3LL
Ch3LL previously approved these changes Aug 1, 2023
@Ch3LL Ch3LL temporarily deployed to ci August 1, 2023 21:40 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 1, 2023 21:40 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 1, 2023 21:46 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 1, 2023 21:46 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 1, 2023 21:59 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 1, 2023 22:02 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 2, 2023 03:16 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 2, 2023 03:16 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 2, 2023 03:16 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 2, 2023 03:16 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 2, 2023 03:16 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci August 2, 2023 03:16 — with GitHub Actions Inactive
@Ch3LL
Copy link
Contributor

Ch3LL commented Aug 24, 2023

Looks like there are related test failures that need to be resolved. Also can you ensure to rebase as there was recent test fixes merged in.

@Ch3LL Ch3LL temporarily deployed to ci October 17, 2023 20:58 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 17, 2023 20:59 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 17, 2023 20:59 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 17, 2023 20:59 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 00:37 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 00:37 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 00:37 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 00:37 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 00:37 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 00:38 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 02:28 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 02:28 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 02:28 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 02:28 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 02:28 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 02:29 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 20:33 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 20:33 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 20:33 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 20:34 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 20:34 — with GitHub Actions Inactive
@Ch3LL Ch3LL temporarily deployed to ci October 18, 2023 20:34 — with GitHub Actions Inactive
@dwoz dwoz added this to the Argon v3008.0 milestone Dec 18, 2023
@dwoz dwoz requested a review from a team as a code owner March 16, 2025 22:09
Copy link
Contributor

@twangboy twangboy left a comment

Choose a reason for hiding this comment

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

This also needs to be rebased and conflicts resolved


This option is dangerous, use it with caution.

discard
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a .. version-added:: marker

@twangboy twangboy added pending-changes The pull request needs additional changes before it can be merged test:full Run the full test suite and removed Argon v3008.0 labels Jul 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-failing-test pending-changes The pull request needs additional changes before it can be merged test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants