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
lvm: cleanups #93024
lvm: cleanups #93024
Conversation
- use installTargets again ("install", and "install_systemd_{generators,units,configuration}" when udev is not null) - The call to the blkid binary in lvm2's 13-dm-disk.rules file disappeared (so we don't need to patch in blkid anymore). lvm seems to rely on udev's internal blkid functionality. - Call /run/current-system/systemd/bin/udevadm instead of ${systemd}/bin/udevadm in the lvm activation generator. This is not necessary to break the dependency cycle (as we don't use that file when building without udev), but a good idea anyways - We want to trigger the udevadm of the current system, not the one that lvm was built with.
Otherwise, we end up in a dependency cycle: systemd -> cryptsetup -> lvm -> fetchgit -> git -> openssh -> libfido2 -> hidapi -> libusb -> udev=systemd
This seems to be mostly used to simplify LV management tasks from a web interface (https://www.redhat.com/archives/linux-lvm/2008-September/msg00029.html), and is as fat as the `lvm` binary itself
This shuts up some warnings.
@@ -552,16 +555,26 @@ in { | |||
+ " mkpart primary 2048M -1s" # PV2 | |||
+ " set 2 lvm on", | |||
"udevadm settle", | |||
"sleep 1", |
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.
Maybe add a comment why the sleep is necessary here.
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.
It's already in the commit message.
Also, it's not really "needed".
There's some racyness causing some waiting every now and then, and I at least felt like this will make it less likely to trigger. The test still succeeds, it's just less likely to take longer.
Introduce a pkgs.lvm2_dmeventd that contains dmeventd support, and enable if services.lvm.dmeventd.enable is true.
This is already handled by the lvm module, which now properly adds the lvm systemd generators. Initially introduced in c0fd887.
Also, add some sleep statements in between, which seems to at least feel like it causes > WARNING: Device /dev/vda* not initialized in udev database even after waiting 10000000 microseconds. To occur less frequently. This eventually still succeeds after some amount of waiting, I suspect some racyness in the way lvm's udev-triggered scripts trigger other units.
Make this more consistent with how these flags look like in the rest of nixpkgs.
Adressed @Mic92's comments. |
LGTM once the tests finish |
Well, tests weren't in passthru. I pushed a commit adding them, good idea! |
Tests succeeded on ofborg x86_64-linux, and failed on aarch64 due to another unrelated error/flakyness:
|
It was broken by commit d3a991d (NixOS#93024). Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This broke multipath-tools; the fix in #93949 is needed. |
It was broken by commit d3a991d (NixOS#93024). Signed-off-by: Anders Kaseorg <andersk@mit.edu> (cherry picked from commit a3a1e27)
lvm2 support was broken when lvm2 got converted to a multiple-output derivation: NixOS#93024 NixOS@d3a991d The `runtimeDependencies` attribute doesn't specifically look for a `lib` output, so it uses the main `out` output which no longer contains the library object files. Since TSM loads the `libdevmapper.so` library dynamically (likely with `dlfcn.h` functions), the breakage couldn't be detected at build time. The commit at hand simply uses `getLib` to pick the correct output.
This includes quite some fixes to lvm2 accumulated while working on enabling cryptsetup support in systemd (#66856). I intend to rebase #66856 on top of the changes here.
lvm2 is now a multi-output derivation, which helps for closure size reduction, but also becomes necessary as soon as you try to build systemd with cryptsetup support (that requires the libdevmapper component of lvm2) - due to lvm itself also containing references to libsystemd (via their shipped systemd generators).
Properly splitting this avoids getting rid of hacks like
systemd_with_lvm2
.This also contains quite some cleanups in the unit itself, making it more maintainable in the longer run.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)