-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
zfsUnstable: 2.2.3-unstable-2024-02-12 -> 2.2.3 #291402
Conversation
@@ -17,7 +17,7 @@ callPackage ./generic.nix args { | |||
# check the release notes for compatible kernels | |||
kernelCompatible = | |||
if stdenv'.isx86_64 || removeLinuxDRM | |||
then kernel.kernelOlder "6.9" | |||
then kernel.kernelOlder "6.8" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are patches in 2.2.3 that already make it compile with 6.8. I’ve ran them on my machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it’s safe to leave support for zfsUnstable even if it’s not listed as supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we either align with released Kernels and what upstream zfs claims to support, or we should just make zfsUnstable have kernelCompatible = true
(as it is unstable, after all). Between those two is murky. Further, there is at least one open PR upstream for functional fix for 6.8 rc release, so even if it compiles, it might have functional regressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the problems tho is that folks are motivated to use zfsUnstable often to deal with OpenZFS’s tendency to lag behind current kernels quite a bit (where certain new kernel features are required to keep a new device running (I have been here), but once nixpkgs deprecates an old kernel users are required to go back to LTS kernels which straight up may not boot) & the official version support bump in the staging branches happens right before release, not while staging is being worked on so it’s always been a bit of a guessing game.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upstream hasn't certified 6.8 on any of the branches. And understandably so, since 6.8 is not released. This is a downside of external modules. I didn't double check the version compatibility before merging the previous PR, and regret doing so.
I appreciate there is a desire to run RC kernels, and this is named "unstable", but that doesn't mean we should offer a package that ignores upstream's recommendations on something as critical as a filesystem.
If there's something we need to change to allow easier overriding of zfs to enable unsupported configurations, let's improve that. But we need to make such configurations more opt-in than just choosing the "unstable" package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One can always bypass kernelCompatible with allowBroken or applying patches to nixpkgs. Some relevant prior discussion from 2021.
Linux 6.8 is not released, and upstream Zfs does not proclaim support for it yet, even though there may be *some* compat patches merged.
77c9160
to
12df2df
Compare
Now exists: https://github.com/openzfs/zfs/commits/zfs-2.2.4-staging/ That 6.8 commit was merged to the main branch, but it doesn’t yet appear in this newly-created staging branch which will be targeting 6.8+. |
@toastal Sure the branch exists, but it is equivalent to 2.2.3 right now, so there’s nothing to change here (yet). |
We should close this in favor of #297808 as outdated since there (finally) is work in the 2.2.4-staging branch. |
Description of changes
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.