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

nixos boot extra-utils: ensure we use working lvm2 #96290

Closed
wants to merge 1 commit into from
Closed

nixos boot extra-utils: ensure we use working lvm2 #96290

wants to merge 1 commit into from

Conversation

vcunat
Copy link
Member

@vcunat vcunat commented Aug 25, 2020

This fixes the problem for me, though it's a bit hacky that we basically
re-link cryptsetup to a different lvm2 "version".
The recursion probably isn't needed now, but I think theoretically
it might be useful.

Also show those more results/failures in the build log.

Motivation for this change

Fixes #96197.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@vcunat
nixos boot extra-utils: ensure we use working lvm2
00c9d1f 
This fixes the problem for me, though it's a bit hacky that we basically
re-link cryptsetup to a different lvm2 "version".
The recursion probably isn't needed now, but I think theoretically
it might be useful.

Also show those more results/failures in the build log.
@vcunat vcunat mentioned this pull request Aug 25, 2020
Closed
jonringer
jonringer approved these changes Aug 25, 2020
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

I am able to run nixos tests again

@flokli
Copy link
Contributor

flokli commented Aug 25, 2020 •
edited

lvm2 provides libdevmapper, which is used to build cyptsetup (and systemd uses libcryptsetup for its cryptsetup support).
We finally build a lvm2 with udev support (which is used in the normal system).
Also, see the commit message in 72c8ed0.

For initrd, it probably doesn't matter much which lvm2 we use, as systemd isn't available there.

@vcunat
Copy link
Member Author

vcunat commented Aug 25, 2020 •
edited

The problem was that if we use the less featurful one, dmsetup won't load due to missing symbol(s). EDIT: so one alternative would be to have both/all without udev support (for initrd), if I get you right.

@flokli
Copy link
Contributor

flokli commented Aug 25, 2020 •
edited

I assume we currently only have one dmsetup available, from lvm2.bin.

We might be able to just use dmsetup from lvm2.override { udev = null; }'s bin output - and only end up with one lvm2*.lib store path.

@flokli
Copy link
Contributor

flokli commented Aug 25, 2020

@vcunat something like this:

diff --git a/nixos/modules/system/boot/stage-1.nix b/nixos/modules/system/boot/stage-1.nix
index ec5eadf69c3..aab9e3cdec6 100644
--- a/nixos/modules/system/boot/stage-1.nix
+++ b/nixos/modules/system/boot/stage-1.nix
@@ -87,7 +87,9 @@ let
   # copy what we need.  Instead of using statically linked binaries,
   # we just copy what we need from Glibc and use patchelf to make it
   # work.
-  extraUtils = pkgs.runCommandCC "extra-utils"
+  extraUtils = let
+    # Use lvm2 without udev support, which is the same lvm2 we already have in the closure anyways
+    lvm2 = pkgs.lvm2.override { udev = null; }; in pkgs.runCommandCC "extra-utils"
     { nativeBuildInputs = [pkgs.buildPackages.nukeReferences];
       allowedReferences = [ "out" ]; # prevent accidents like glibc being included in the initrd
     }
@@ -111,8 +113,8 @@ let
       copy_bin_and_libs ${pkgs.utillinux}/sbin/blkid
 
       # Copy dmsetup and lvm.
-      copy_bin_and_libs ${getBin pkgs.lvm2}/bin/dmsetup
-      copy_bin_and_libs ${getBin pkgs.lvm2}/bin/lvm
+      copy_bin_and_libs ${getBin lvm2}/bin/dmsetup
+      copy_bin_and_libs ${getBin lvm2}/bin/lvm
 
       # Add RAID mdadm tool.
       copy_bin_and_libs ${pkgs.mdadm}/sbin/mdadm

@vcunat
Copy link
Member Author

vcunat commented Aug 25, 2020

Yes, sounds good from what I know.

FRidh added a commit that referenced this pull request Aug 26, 2020
@FRidh
extra-utils: build a full lvm2 without udev support, fixes #96197
f6286de 
dmsetup was missing symbols.
#96290 (comment)
@FRidh
Copy link
Member

FRidh commented Aug 26, 2020

I've pushed @flokli 's suggestion as f6286de. Thanks!

@FRidh FRidh closed this Aug 26, 2020
@jonringer
Copy link
Contributor

since you pushed to master, we should probably push master to staging-next, staging-next to staging. Currently all other branches are broken

@FRidh
Copy link
Member

FRidh commented Aug 26, 2020

already done 😉

jerith666 pushed a commit to jerith666/nixpkgs that referenced this pull request Sep 25, 2020
@FRidh @jerith666
extra-utils: build a full lvm2 without udev support, fixes NixOS#96197
6ec9a82 
dmsetup was missing symbols.
NixOS#96290 (comment)

(cherry picked from commit f6286de)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NixOS Tests broken in master and staging
4 participants
@vcunat @flokli @FRidh @jonringer