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

grub2: set zfsSupport false by default #30192

Closed
wants to merge 1 commit into from
Closed

grub2: set zfsSupport false by default #30192

wants to merge 1 commit into from

Conversation

jokogr
Copy link
Contributor

@jokogr jokogr commented Oct 7, 2017

Motivation for this change

Fixes #11533

A basic, ext4-based system with grub2 was pulling zfs and python2 derivations.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@grahamc
Copy link
Member

grahamc commented Oct 7, 2017

Can you run the installer tests with this? nix-build ./nixos/tests/installer.nix

@Mic92
Copy link
Member

Mic92 commented Oct 7, 2017

Do we want a separate derivation for grubZfs. Personally I use systemd-boot, but I know some people are relying on zfs support in grub. Is there a way of recognizing if zfs support would be required in grub?

@jokogr
Copy link
Contributor Author

jokogr commented Oct 7, 2017

@grahamc give me some time until tonight, my workstation is building some other stuff first.

@Mic92 I would like to have a separate one to keep the image size small.

Please check

nixpkgs/nixos/modules/system/boot/loader/grub/grub.nix

Line 12 in 631ec73

else if cfg.zfsSupport then pkgs.grub2.override { zfsSupport = true; }

where pkg.grub2 is

nixpkgs/pkgs/top-level/all-packages.nix

Lines 2233 to 2235 in 631ec73

grub2 = grub2_full;
grub2_full = callPackage ../tools/misc/grub/2.0x.nix { };

and 2.0.nix defines zfsSupport as default

nixpkgs/pkgs/tools/misc/grub/2.0x.nix

Line 5 in eb9f427

, zfsSupport ? true

Am I not thinking it right?

@Mic92
Copy link
Member

Mic92 commented Oct 7, 2017

@jokogr looks good to me then.

@dezgeg
Copy link
Contributor

dezgeg commented Oct 7, 2017 •
edited

Can't it still potentially break existing systems, if some hasn't specified boot.loader.grub.zfsSupport in their NixOS configuration?

I would imagine you could set the default to something like:

config.fileSystems."/boot".fsType or null == "zfs" && config.fileSystems."/".fsType or null == "zfs";

and have it still work.

@Mic92
Copy link
Member

Mic92 commented Oct 7, 2017 •
edited

In the zfs module there is a check:

inInitrd = any (fs: fs == "zfs") config.boot.initrd.supportedFilesystems;

which enables grub support.
So the only way this could fail, if only /boot is on zfs but not required for the actual system/initrd.
However dezgeg proposal would be more precise.

@jokogr
Copy link
Contributor Author

jokogr commented Oct 7, 2017

@grahamc on my nixpkgs (nixos-unstable channel + some WIP of mine) it appears there is no network connection and the build fails:

machine# building path(s) ‘/nix/store/c4rxfd89f8g2sbgipv12shfp8fnimgpl-closure’
machine# downloading ‘http://mirror.easyname.at/nongnu/acl/acl-2.2.52.src.tar.gz’... [0/0 KiB, 0.0 KiB/s]
machine# error: unable to download ‘http://mirror.easyname.at/nongnu/acl/acl-2.2.52.src.tar.gz’: Couldn't resolve host name (6)
machine# builder for ‘/nix/store/p1scg3lir7xlfz1yxhhy0lhvxidnrrjn-acl-2.2.52.src.tar.gz.drv’ failed with exit code 1
machine# cannot build derivation ‘/nix/store/l8s1rcwvzsd0qp9w5h0gqxkhpaill6rh-acl-2.2.52.drv’: 1 dependencies couldn't be built
machine# building path(s) ‘/nix/store/mhia1qb8hh4jkaasripsfzn2ggk5jgcp-attr-2.4.47.src.tar.gz’
machine# cannot build derivation ‘/nix/store/2n23afqa7ybfa9rf3caym5xy25whi45g-systemd-234.drv’: 1 dependencies couldn't be built
machine# cannot build derivation ‘/nix/store/4qw0x6jniwr8zkajxpjbqhkdk1xpryn0-libusb-1.0.20.drv’: 1 dependencies couldn't be built
machine# cannot build derivation ‘/nix/store/8h6pp4pk62vkwp8q6vzyil7xxw0xy5s4-libusb-compat-0.1.5.drv’: 1 dependencies couldn't be built
machine# cannot build derivation ‘/nix/store/rgwzj0m0xl6ardwa5qgkmdh1b4xjin9s-grub-2.02.drv’: 1 dependencies couldn't be built
machine# cannot build derivation ‘/nix/store/zzbs4jha6qr10rxvgrh0n0wyc872ahl1-grub-config.xml.drv’: 1 dependencies couldn't be built
machine# cannot build derivation ‘/nix/store/nbzmwwklzr607l1p9s4lpixfjhnijlg0-system-path.drv’: 1 dependencies couldn't be built
machine# cannot build derivation ‘/nix/store/g1gzwsxpx4db9pff0i468k7cpi2hsgn8-nixos-system-nixos-18.03.git.0d71033.drv’: 1 dependencies couldn't be built
machine# error: build of ‘/nix/store/g1gzwsxpx4db9pff0i468k7cpi2hsgn8-nixos-system-nixos-18.03.git.0d71033.drv’ failed
machine: exit status 1
machine: output: 
error: command `nixos-install < /dev/null >&2' did not succeed (exit code 1)
command `nixos-install < /dev/null >&2' did not succeed (exit code 1)
cleaning up
killing machine (pid 140)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/tmp/nix-build-vm-test-run-installer-zfs-root.drv-0/vde1.ctl': Directory not empty
builder for ‘/nix/store/h0iq1v82sai4j662m7b4rd6q2ls8v7gl-vm-test-run-installer-zfs-root.drv’ failed with exit code 255
error: build of ‘/nix/store/h0iq1v82sai4j662m7b4rd6q2ls8v7gl-vm-test-run-installer-zfs-root.drv’ failed

Any ideas how I could move on?

@jokogr
Copy link
Contributor Author

jokogr commented Oct 28, 2017 •
edited

@grahamc the tests succeed after adding to the test the following line (thanks niksnut):

virtualisation.pathsInNixDB = [ (pkgs.grub2.override { zfsSupport = true; }) ];

@grahamc
Copy link
Member

grahamc commented Nov 3, 2017

@jokogr why do we need to make that change to the paths in nix db? If the logic behind enabling ZFS was correct, it seems weird we'd need to do anything special for the test to work.

@jokogr
Copy link
Contributor Author

jokogr commented Nov 3, 2017

@grahamc the test vms do not have internet access and thus they cannot build any package. Since my nixpkgs do not have grub2 with zfsSupport, the test vm tries to build it and fails.

@grahamc
Copy link
Member

grahamc commented Nov 3, 2017

Ok, then that should be part of this PR :)

@jokogr
Copy link
Contributor Author

jokogr commented Nov 3, 2017

@grahamc ok, I will do so, then.

Should we also have this package in the official binary cache, so that people who want to use Grub and ZFS do not have to build it during installation? If so, should I make another change?

@GrahamcOfBorg
Copy link

Success on x86_64-linux

Attempted: tests.installer

No partial log is available.

@GrahamcOfBorg
Copy link

Success on aarch64-linux

Attempted: tests.installer

No partial log is available.

@jokogr
Copy link
Contributor Author

jokogr commented Jun 18, 2018

So, not just python2, but nfs-utils, too

Regarding this: "Should we also have this package in the official binary cache, so that people who want to use Grub and ZFS do not have to build it during installation?"

How to include in the official cache two versions of Grub2? Otherwise, tests would fail.

@jokogr jokogr closed this Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jokogr @grahamc @Mic92 @dezgeg @GrahamcOfBorg