Skip to content

zfs: revamp and test compatible Kernel versions#352399

Merged
adamcstephens merged 2 commits intoNixOS:masterfrom
amarshall:zfs-kernel-versions
Aug 1, 2025
Merged

zfs: revamp and test compatible Kernel versions#352399
adamcstephens merged 2 commits intoNixOS:masterfrom
amarshall:zfs-kernel-versions

Conversation

@amarshall
Copy link
Member

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

Add a 👍 reaction to pull requests you find important.

@nix-owners nix-owners bot requested a review from RaitoBezarius October 30, 2024 15:09
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. labels Oct 30, 2024
@adamcstephens
Copy link
Contributor

@ofborg build zfs_2_1 zfs_unstable

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 1, 2024
@amarshall amarshall changed the title zfs: revamp and test compatibile Kernel versions, expose new kernelIsCompatible zfs: revamp and test compatibile Kernel versions Dec 12, 2024
@amarshall amarshall removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 12, 2024
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Dec 12, 2024
@amarshall
Copy link
Member Author

amarshall commented Dec 12, 2024

Rebased, resolved conflicts, reformatted, re-ran tests. I also removed amarshall@bc6f676 from this for now, as I’m not really sure it’s necessary anymore (and easy enough add back or PR later).

@amarshall amarshall changed the title zfs: revamp and test compatibile Kernel versions zfs: revamp and test compatible Kernel versions Dec 12, 2024
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 12, 2024
@amarshall amarshall removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 12, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Dec 13, 2024
@amarshall
Copy link
Member Author

@adamcstephens Thoughts here? Would really like this to be able to reduce how much one has to proactively think about Kernel versions when doing ZFS package updates.

@adamcstephens
Copy link
Contributor

I’ll take a look at the few pending PRs this weekend. Sorry for the delay.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make this a boolean instead? something like ignoreKernelCompatibility

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 could, but I suppose it’s useful to be able to mark as broken when it fails to build at all on newer Kernel versions, even on zfs_unstable. At least, this is what the previous kernelCompatible meant for zfs_unstable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any additional thoughts? Also rebased, resolved conflicts, and re-ran tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where should we document overriding this, to allow building outside the limits?

Copy link
Member Author

Choose a reason for hiding this comment

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

My initial thought is that it’s just for zfs_unstable (as we’ve implicitly had this in the past, but now with this the “…MaxSupported…` is verified against the code, so isn’t sufficient). But I’m also fine with adding it to the wiki, along with the other unsupported-or-not-recommended-but-doable stuff.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 31, 2024
@amarshall amarshall force-pushed the zfs-kernel-versions branch from 06544b4 to b1331d3 Compare January 14, 2025 01:22
@amarshall
Copy link
Member Author

Rebased and resolved merge conflicts.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 14, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. and removed 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. labels Jan 14, 2025
@amarshall amarshall force-pushed the zfs-kernel-versions branch from b1331d3 to fe29639 Compare January 24, 2025 23:42
@amarshall amarshall force-pushed the zfs-kernel-versions branch 2 times, most recently from 90ec1ef to b018d6b Compare January 24, 2025 23:50
@adamcstephens
Copy link
Contributor

FYI, upstream is now hard failing on incompatible versions during configure.

openzfs/zfs@410287f

@amarshall
Copy link
Member Author

amarshall commented Feb 19, 2025

I think the only thing that changes is that now this needs to set --enable-linux-experimental for zfs_unstable? Or I guess whenever kernelMaxBuildableMajorMinor != kernelMaxSupportedMajorMinor, or something else? I don’t think it changes wanting these changes in general, since it does make testing the Nix-side compat specification of meta.broken any easier (as far as I can tell anyway). Does that match your thinking?

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 16, 2025
@amarshall amarshall force-pushed the zfs-kernel-versions branch from b018d6b to c16503f Compare June 13, 2025 12:34
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 13, 2025
@amarshall
Copy link
Member Author

Updated for upstream changes with the flag. Since upstream now explicitly prevents building by default with unsupported Kernel versions and we also don’t support doing so, this is opt-in only and not even zfs_unstable will have it (after all, the flag has been needed to build against unsupported Kernels for several months and we haven’t had it and no one has complained afaik).

@amarshall amarshall mentioned this pull request Jun 13, 2025
13 tasks
@github-actions github-actions bot added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Jun 13, 2025
@wolfgangwalther
Copy link
Contributor

Re-running CI due to a odd failure fixed in #416448

@georgewhewell
Copy link
Contributor

Updated for upstream changes with the flag. Since upstream now explicitly prevents building by default with unsupported Kernel versions and we also don’t support doing so, this is opt-in only and not even zfs_unstable will have it (after all, the flag has been needed to build against unsupported Kernels for several months and we haven’t had it and no one has complained afaik).

fwiw i've many times had to force nixos to build zfs on unsupported kernel to support new hardware or apply patches and i imagine i will do again. +1 for making this easier- maybe even worth adding documentation how to do this

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 16, 2025
Copy link
Contributor

@adamcstephens adamcstephens left a comment

Choose a reason for hiding this comment

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

I think we should merge this, but the conflict will need to be resolved first.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jun 25, 2025
Switch to specifying the Kernel compatibility in the same form as in the
ZFS source’s `META` file. This then allows checking it against what’s in
`META` to remove the need to manually check, and so reduce the cognitive
load and change of mistakes when performing updates.

As the “max” version is indeed a compatible version, we need to
increment that for the comparison when using `versionOlder`.

One side-effect here is that it makes it not possible to override the
specified compatibility. We will “fix” this soon.
This will allow building ZFS against an unsupported Kernel version.
While possible, it is not supported by upstream or Nixpkgs. As such, we
don’t enable it by default, even for zfs_unstable. Enabling should be a
deliberate choice by the user to opt into potential data corruption bugs.
@amarshall amarshall force-pushed the zfs-kernel-versions branch from c16503f to 3d589fc Compare August 1, 2025 12:30
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 1, 2025
@amarshall
Copy link
Member Author

Resolved conflicts.

@adamcstephens adamcstephens merged commit f284689 into NixOS:master Aug 1, 2025
25 of 27 checks passed
@akirak
Copy link
Member

akirak commented Aug 3, 2025

@amarshall This PR has dropped support for ZFS on Linux 6.15, which had been previously working. OpenZFS officially supports 6.15:

https://endoflife.date/openzfs

I think the value of kernelMaxSupportedMajorMinor should have been 6.16 here:

https://github.com/amarshall/nixpkgs/blob/5d2750268b5a9464354645382d5a9d63ac0ec09f/pkgs/os-specific/linux/zfs/unstable.nix#L13

@amarshall amarshall deleted the zfs-kernel-versions branch August 3, 2025 17:58
@amarshall
Copy link
Member Author

This PR has dropped support for ZFS on Linux 6.15

What makes you think that? nix-instantiate --eval -A linuxKernel.packages.linux_6_15.zfs_2_3.meta.broken shows
false on current master.

I think the value of kernelMaxSupportedMajorMinor should have been 6.16 here:

No, it should be 6.15. The attr is now the actual max supported version (≤) not the next biggest (<).

@adamcstephens
Copy link
Contributor

The latest kernel is now 6.16, perhaps that's the issue?

On master:

𑁱 nom build -f . linuxKernel.packages.linux_6_15.zfs_2_3 --print-out-paths
Finished at 14:12:37 after 6s
/nix/store/h3wqx846byvzijr9922wq068gx48alqa-zfs-kernel-2.3.3-6.15.9

𑁱 nom build -f . linuxPackages_latest.zfs_2_3 --print-out-paths
error:
<SNIP>
       error: Package ‘zfs-kernel-2.3.3-6.16’ in /home/adam/git/nixpkgs/pkgs/os-specific/linux/zfs/generic.nix:324 is marked as broken, refusing to evaluate.

@akirak
Copy link
Member

akirak commented Aug 3, 2025

The latest kernel is now 6.16, perhaps that's the issue?

You are right. After updating nixpkgs, the system fails to build.

What makes you think that? nix-instantiate --eval -A linuxKernel.packages.linux_6_15.zfs_2_3.meta.broken shows
false on current master.

Sorry about my confusion. Somehow I was unable to find out this method.

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

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants