[go: up one dir, main page]
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

zfs: fix evaluation and build on aarch64 systems #237873

Merged
merged 1 commit into from
Jul 8, 2023
Merged

zfs: fix evaluation and build on aarch64 systems #237873

merged 1 commit into from
Jul 8, 2023

Conversation

Mic92
Copy link
Member
@Mic92 Mic92 commented Jun 15, 2023 •
edited
Loading

This is blocking: nix-community/nixos-images#115

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jun 15, 2023
@Mic92 Mic92 force-pushed the fix-aarch64-zfs branch 2 times, most recently from 0e9c101 to d21b935 Compare June 15, 2023 06:42
@Mic92 Mic92 mentioned this pull request Jun 15, 2023
Merged
12 tasks
@Mic92
Copy link
Member Author
Mic92 commented Jun 15, 2023

As of now latestCompatibleLinuxPackages points to broken version on aarch64-linux.

@Mic92 Mic92 mentioned this pull request Jun 15, 2023
Merged
12 tasks
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 labels Jun 15, 2023
lopsided98
lopsided98 suggested changes Jun 15, 2023
View reviewed changes
Copy link
Contributor
@lopsided98 lopsided98 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I tried to test this PR with the following config, which I think is fairly standard.

boot.kernelPackages = config.boot.zfs.package.latestCompatibleLinuxPackages;
boot.zfs.removeLinuxDRM = true;

This still results in using 6.1, but also attempts to apply the patch which of course fails. This PR overrides the zfs package with removeLinuxDRM before it is passed to extraModulePackages, but this has no effect on the value of boot.zfs.package or pkgs.zfs, which are normally used to choose the proper kernel packages.

nixos/modules/tasks/filesystems/zfs.nix
# this symbol.
# In the meantime, we restore what was once a working piece of code
# in the kernel.
boot.kernelPatches = lib.optional (cfgZfs.removeLinuxDRM && pkgs.stdenv.hostPlatform.system == "aarch64-linux") {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should only apply the patch if the kernel is at least 6.2

Copy link
Member Author

Choose a reason for hiding this comment

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

Its not possible to check the kernel version here. This will lead to infinite recursion.

@Mic92
Copy link
Member Author
Mic92 commented Jun 16, 2023 •
edited
Loading

Yes. This is a convoluted mess:

boot.kernelPackages = lib.mkDefault (pkgs.zfs.override {
  removeLinuxDRM = pkgs.hostPlatform.isAarch64;
}).latestCompatibleLinuxPackages;

boot.zfs.removeLinuxDRM = true;

If you have a better idea... I don't have one. There is too many dependencies between the nixos module system, kernel packages and zfs.

@RaitoBezarius @Mic92
zfs: add option to restore kernel_neon for linux 6.2 support on aarch64
d4cab20 
Introduced in torvalds/linux@aaeca98
with the usual disdain for ZFS.

We have been there in the past with
<https://www.phoronix.com/news/NixOS-Linux-5.0-ZFS-FPU-Drop> /
NixOS#61076.

This fixes ZFS on aarch64 until the next breakage.

See openzfs/zfs#14555 for original upstream
issue.
@Mic92
Copy link
Member Author
Mic92 commented Jun 16, 2023

I tried to test this PR with the following config, which I think is fairly standard.

boot.kernelPackages = config.boot.zfs.package.latestCompatibleLinuxPackages;
boot.zfs.removeLinuxDRM = true;

This still results in using 6.1, but also attempts to apply the patch which of course fails. This PR overrides the zfs package with removeLinuxDRM before it is passed to extraModulePackages, but this has no effect on the value of boot.zfs.package or pkgs.zfs, which are normally used to choose the proper kernel packages.

I made this more explicit in the option description. Please have a look.

@RaitoBezarius RaitoBezarius mentioned this pull request Jun 16, 2023
Closed
12 tasks
@Mic92
Copy link
Member Author
Mic92 commented Jul 1, 2023

friendly bump @RaitoBezarius @lopsided98

@RaitoBezarius
Copy link
Member

I will take responsibility.

@RaitoBezarius RaitoBezarius merged commit ed2f009 into NixOS:master Jul 8, 2023
@github-actions
Copy link
Contributor
github-actions bot commented Jul 8, 2023

Backport failed for release-23.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin release-23.05
git worktree add -d .worktree/backport-237873-to-release-23.05 origin/release-23.05
cd .worktree/backport-237873-to-release-23.05
git checkout -b backport-237873-to-release-23.05
ancref=$(git merge-base f8dc6d000bdb41a51a16feb86b53352b5fc58f31 d4cab20b3a76fef4b9a487c37107df6351ebce33)
git cherry-pick -x $ancref..d4cab20b3a76fef4b9a487c37107df6351ebce33

@Mic92 Mic92 deleted the fix-aarch64-zfs branch August 4, 2023 19:59
@howdoicomputer
Copy link

Are there plans to backport this to 23.05?

@RaitoBezarius
Copy link
Member

We are too close to 23.11 that I can take care of this. You are free to open a backport PR as long as it complies with the backport policies.

@tpwrules tpwrules mentioned this pull request Oct 21, 2023
Closed
@truatpasteurdotfr
Copy link

hi, is this fixed for 23.11 ?

@Luflosi
Copy link
Contributor
Luflosi commented Mar 29, 2024

Yes, this is fixed in NixOS 23.11. You have to enable the boot.zfs.removeLinuxDRM option though: https://search.nixos.org/options?channel=23.11&show=boot.zfs.removeLinuxDRM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaitoBezarius RaitoBezarius RaitoBezarius left review comments

@lopsided98 lopsided98 lopsided98 requested changes

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Mic92 @RaitoBezarius @howdoicomputer @truatpasteurdotfr @Luflosi @lopsided98