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.

Sign up for GitHub

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

Jump to bottom

nixos/swap: add discardPolicy option #107728

Merged
merged 1 commit into from Jun 15, 2021
Merged

nixos/swap: add discardPolicy option #107728

merged 1 commit into from Jun 15, 2021

Conversation

nessdoor
Copy link
Contributor

Add option for activating discards on swap partitions (none, once, pages and both).

Motivation for this change

Especially when hibernating, many gigs get written to disk. I would have liked to see the possibility of specifying a discard policy in NixOS.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ajs124 ajs124 mentioned this pull request Feb 26, 2021
Closed
10 tasks
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/511

futile
futile approved these changes May 12, 2021
View reviewed changes
Copy link
Contributor

@futile futile left a comment •
edited

Choose a reason for hiding this comment

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

LGTM!

Edit: I'm not sure whether it makes sense to add a test for this, otherwise this corresponds to the parameter description given in the swapon(8) man page.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/376

@nessdoor
Copy link
Contributor Author

Tested by building the following derivation manually under different swap options:
nix-build -A config.system.build.etc

Don't know if this suffices

ThibautMarty
ThibautMarty reviewed May 17, 2021
View reviewed changes
Copy link
Member

@ThibautMarty ThibautMarty left a comment •
edited

Choose a reason for hiding this comment

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

LGTM.

The change is backward compatible.

There are one more possible option for swap devices in fstab (at least on my current system) : nofail. That could be added later if it's needed. They are not too many options so listing them explicitly (rather than using something like extraOptions) is fine.

nixos/modules/tasks/filesystems.nix Outdated Show resolved Hide resolved
nixos/modules/config/swap.nix Outdated Show resolved Hide resolved
@nessdoor
Copy link
Contributor Author

@ThibautMarty I think I applied all your suggestions, retested and everything works. Thank you very much

ThibautMarty
ThibautMarty reviewed May 18, 2021
View reviewed changes
Copy link
Member

@ThibautMarty ThibautMarty left a comment

Choose a reason for hiding this comment

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

Last nitpicking and we it's good for me.

nixos/modules/config/swap.nix Outdated Show resolved Hide resolved
@nessdoor
nixos/swap: add discardPolicy option
b922fa9 
Add option for activating discards on swap partitions (none, once, pages and both).
@nessdoor
Copy link
Contributor Author

@ThibautMarty maybe this time I didn't forget anything

@SuperSandro2000 SuperSandro2000 merged commit 2b49e4e into NixOS:master Jun 15, 2021
@futile
Copy link
Contributor

futile commented Jun 15, 2021

🎉

@kurnevsky kurnevsky mentioned this pull request Jul 6, 2021
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@futile futile futile approved these changes

@ajs124 ajs124 ajs124 approved these changes

@ThibautMarty ThibautMarty ThibautMarty approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@nessdoor @nixos-discourse @futile @ajs124 @ThibautMarty @SuperSandro2000 @Janik-Haag