(k)vdo: init #154981
(k)vdo: init #154981
Conversation
ajs124
force-pushed
the
feat/lvm2-vdo
branch
from
c9cb313
to
93d0580
Compare
| @@ -7,17 +7,18 @@ in { | |||
| options.services.lvm = { | |||
| package = mkOption { | |||
| type = types.package; | |||
| default = if cfg.dmeventd.enable then pkgs.lvm2_dmeventd else pkgs.lvm2; | |||
There was a problem hiding this comment.
Does this need a corresponding change to LVM2 package choice somewhere else?
There was a problem hiding this comment.
Good question. I don't think so. lvm2_dmeventd is only referenced by the dmraid package and in this line, AFAICT.
There was a problem hiding this comment.
Well, I am not sure what dmeventd does, but it seems plausible that when it enabled you need to use a build of LVM that includes it…
There was a problem hiding this comment.
Ah, sure. lvm2_vdo also includes dmeventd support. The package option is just set lower in the module now, in lines 44 and 89.
I think this should work, but if you think something is wrong here or this code is unclear, I'm open to suggestions.
There was a problem hiding this comment.
Isn't it dependent on vdo, so that dmeventd-only case is not handled?
The problem is that I of course don't use dmeventd so I do not know what is a good enough easy check…
There was a problem hiding this comment.
I think it is a link to the wrong file? (package not test)
There was a problem hiding this comment.
There was a problem hiding this comment.
Hm, you said that dmeventd gets used when resizing happens? Does it happen here?
There was a problem hiding this comment.
No. I meant we as in @helsinki-systems use it for that. I can put that into the thin test or vdo test in the next few days/weeks.
There was a problem hiding this comment.
Those were more "few" weeks than I intended, but now it's done.
|
@ofborg test lvm2 |
|
@ofborg test lvm2.lvm-vdo-linux-5_15 |
|
Hm, maybe add a comment in tthe test how to run the test quickly? (in the sense of being able to run it ffor whatever reason on some update without figuring out how the tetst matrix generation works) |
|
@ofborg test lvm2.lvm-thinpool-linux-latest |
| let | ||
| tests = let callTest = p: lib.flip (import p) { inherit system pkgs; }; in { | ||
| thinpool = { test = callTest ./thinpool.nix; kernelFilter = lib.id; }; | ||
| # we would like to test all versions, but the kernel module currently does not compile against the other versions |
There was a problem hiding this comment.
Can't we mark the kernel module as broken then for other kernel versions? Took me an hour to see my error…
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
|
Ouch, sorry about that. We should probably do that, yes. Not sure why I didn't do it like that in the first place.
Can you maybe open a PR for that? If not, I'll see if I can get around to it in the next few days.
Am 14. Mai 2022 16:14:33 MESZ schrieb Norbert Melzer ***@***.***>:
***@***.*** commented on this pull request.
…
> @@ -0,0 +1,27 @@
+{ system ? builtins.currentSystem
+, config ? { }
+, pkgs ? import ../../.. { inherit system config; }
+, lib ? pkgs.lib
+, kernelVersionsToTest ? [ "4.19" "5.4" "5.10" "5.15" "latest" ]
+}:
+
+# For quickly running a test, the nixosTests.lvm2.lvm-thinpool-linux-latest attribute is recommended
+let
+ tests = let callTest = p: lib.flip (import p) { inherit system pkgs; }; in {
+ thinpool = { test = callTest ./thinpool.nix; kernelFilter = lib.id; };
+ # we would like to test all versions, but the kernel module currently does not compile against the other versions
Can't we mark the kernel module as broken then for other kernel versions? Took me an hour to see my error…
-- >
Reply to this email directly or view it on GitHub:
#154981 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
| }) | ||
| (mkIf cfg.boot.vdo.enable { | ||
| boot = { | ||
| initrd = { |
There was a problem hiding this comment.
Can you add a test in the installer.nix tests to make sure this initrd stuff actually works?
There was a problem hiding this comment.
I think that's basically covered in https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/lvm2/systemd-stage-1.nix
Motivation for this change
I'm thinking about using this (cc @Mr-Pi)
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes