From 06ed2c46fb92ef241c90e374cd10782690ec3b8d Mon Sep 17 00:00:00 2001 From: Andrew Marshall Date: Tue, 24 Sep 2024 14:17:42 -0400 Subject: [PATCH 1/4] nixos/zfs: remove incorrect conditional use of spl kmod This was incorrectly getting `lib.version` which is e.g. `"24.11pre-git"`, but should have been the ZFS package version. However, the condition, at least per the comment, is reversed and should be instead `versionOlder cfgZfs.package.version "2.2.0"`. However, the entire premise seems to be incorrect, as ZFS 2.2.6 includes the spl module. Since the previous condition here was effectively always true, it would initially seem the best move is to remove the conditional altogether and always include the spl kmod. However, going back to 4360a87c45f69f7444c7b87546705de7fbc8681f where this condition was added, the intent appears to be that spl was no longer needed here in the-pre-release ZFS (long since in all supported versions), due to it being merged into ZFS mainline. Given that intent and that our boot tests on all versions succeed without including it in the initrd, remove it. --- nixos/modules/tasks/filesystems/zfs.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nixos/modules/tasks/filesystems/zfs.nix b/nixos/modules/tasks/filesystems/zfs.nix index 1bcfd6ccba93..294e2e100ef4 100644 --- a/nixos/modules/tasks/filesystems/zfs.nix +++ b/nixos/modules/tasks/filesystems/zfs.nix @@ -592,8 +592,7 @@ in }; boot.initrd = mkIf inInitrd { - # spl has been removed in ≥ 2.2.0. - kernelModules = [ "zfs" ] ++ lib.optional (lib.versionOlder "2.2.0" version) "spl"; + kernelModules = [ "zfs" ]; extraUtilsCommands = mkIf (!config.boot.initrd.systemd.enable) '' copy_bin_and_libs ${cfgZfs.package}/sbin/zfs From 33bd85a6c8ef052d7476f4d0ee603d2ebb6fb555 Mon Sep 17 00:00:00 2001 From: Andrew Marshall Date: Tue, 24 Sep 2024 14:21:39 -0400 Subject: [PATCH 2/4] nixos/zfs: remove top-level `with`s As it is generally considered an anti-pattern. Removing them here in fact exposed one bug (see previous commit). --- nixos/modules/tasks/filesystems/zfs.nix | 225 ++++++++++++------------ 1 file changed, 111 insertions(+), 114 deletions(-) diff --git a/nixos/modules/tasks/filesystems/zfs.nix b/nixos/modules/tasks/filesystems/zfs.nix index 294e2e100ef4..1145338fba83 100644 --- a/nixos/modules/tasks/filesystems/zfs.nix +++ b/nixos/modules/tasks/filesystems/zfs.nix @@ -2,9 +2,6 @@ # # TODO: zfs tunables -with utils; -with lib; - let cfgZfs = config.boot.zfs; @@ -17,7 +14,7 @@ let cfgZED = config.services.zfs.zed; selectModulePackage = package: config.boot.kernelPackages.${package.kernelModuleAttribute}; - clevisDatasets = attrNames (filterAttrs (device: _: any (e: e.fsType == "zfs" && (fsNeededForBoot e) && (e.device == device || hasPrefix "${device}/" e.device)) config.system.build.fileSystems) config.boot.initrd.clevis.devices); + clevisDatasets = lib.attrNames (lib.filterAttrs (device: _: lib.any (e: e.fsType == "zfs" && (utils.fsNeededForBoot e) && (e.device == device || lib.hasPrefix "${device}/" e.device)) config.system.build.fileSystems) config.boot.initrd.clevis.devices); inInitrd = config.boot.initrd.supportedFilesystems.zfs or false; inSystem = config.boot.supportedFilesystems.zfs or false; @@ -28,17 +25,17 @@ let zfsAutoSnap = "${autosnapPkg}/bin/zfs-auto-snapshot"; - datasetToPool = x: elemAt (splitString "/" x) 0; + datasetToPool = x: lib.elemAt (lib.splitString "/" x) 0; fsToPool = fs: datasetToPool fs.device; - zfsFilesystems = filter (x: x.fsType == "zfs") config.system.build.fileSystems; + zfsFilesystems = lib.filter (x: x.fsType == "zfs") config.system.build.fileSystems; - allPools = unique ((map fsToPool zfsFilesystems) ++ cfgZfs.extraPools); + allPools = lib.unique ((map fsToPool zfsFilesystems) ++ cfgZfs.extraPools); - rootPools = unique (map fsToPool (filter fsNeededForBoot zfsFilesystems)); + rootPools = lib.unique (map fsToPool (lib.filter utils.fsNeededForBoot zfsFilesystems)); - dataPools = unique (filter (pool: !(elem pool rootPools)) allPools); + dataPools = lib.unique (lib.filter (pool: !(lib.elem pool rootPools)) allPools); snapshotNames = [ "frequent" "hourly" "daily" "weekly" "monthly" ]; @@ -89,7 +86,7 @@ let ''; getPoolFilesystems = pool: - filter (x: x.fsType == "zfs" && (fsToPool x) == pool) config.system.build.fileSystems; + lib.filter (x: x.fsType == "zfs" && (fsToPool x) == pool) config.system.build.fileSystems; getPoolMounts = prefix: pool: let @@ -98,36 +95,36 @@ let # Remove the "/" suffix because even though most mountpoints # won't have it, the "/" mountpoint will, and we can't have the # trailing slash in "/sysroot/" in stage 1. - mountPoint = fs: escapeSystemdPath (prefix + (lib.removeSuffix "/" fs.mountPoint)); + mountPoint = fs: utils.escapeSystemdPath (prefix + (lib.removeSuffix "/" fs.mountPoint)); hasUsr = lib.any (fs: fs.mountPoint == "/usr") poolFSes; in map (x: "${mountPoint x}.mount") poolFSes ++ lib.optional hasUsr "sysusr-usr.mount"; - getKeyLocations = pool: if isBool cfgZfs.requestEncryptionCredentials then { + getKeyLocations = pool: if lib.isBool cfgZfs.requestEncryptionCredentials then { hasKeys = cfgZfs.requestEncryptionCredentials; command = "${cfgZfs.package}/sbin/zfs list -rHo name,keylocation,keystatus -t volume,filesystem ${pool}"; } else let - keys = filter (x: datasetToPool x == pool) cfgZfs.requestEncryptionCredentials; + keys = lib.filter (x: datasetToPool x == pool) cfgZfs.requestEncryptionCredentials; in { hasKeys = keys != []; command = "${cfgZfs.package}/sbin/zfs list -Ho name,keylocation,keystatus -t volume,filesystem ${toString keys}"; }; createImportService = { pool, systemd, force, prefix ? "" }: - nameValuePair "zfs-import-${pool}" { + lib.nameValuePair "zfs-import-${pool}" { description = "Import ZFS pool \"${pool}\""; # We wait for systemd-udev-settle to ensure devices are available, # but don't *require* it, because mounts shouldn't be killed if it's stopped. # In the future, hopefully someone will complete this: # https://github.com/zfsonlinux/zfs/pull/4943 - wants = [ "systemd-udev-settle.service" ] ++ optional (config.boot.initrd.clevis.useTang) "network-online.target"; + wants = [ "systemd-udev-settle.service" ] ++ lib.optional (config.boot.initrd.clevis.useTang) "network-online.target"; after = [ "systemd-udev-settle.service" "systemd-modules-load.service" "systemd-ask-password-console.service" - ] ++ optional (config.boot.initrd.clevis.useTang) "network-online.target"; + ] ++ lib.optional (config.boot.initrd.clevis.useTang) "network-online.target"; requiredBy = getPoolMounts prefix pool ++ [ "zfs-import.target" ]; before = getPoolMounts prefix pool ++ [ "shutdown.target" "zfs-import.target" ]; conflicts = [ "shutdown.target" ]; @@ -138,7 +135,7 @@ let Type = "oneshot"; RemainAfterExit = true; }; - environment.ZFS_FORCE = optionalString force "-f"; + environment.ZFS_FORCE = lib.optionalString force "-f"; script = let keyLocations = getKeyLocations pool; in (importLib { @@ -157,10 +154,10 @@ let poolImported "${pool}" || poolImport "${pool}" # Try one last time, e.g. to import a degraded pool. fi if poolImported "${pool}"; then - ${optionalString config.boot.initrd.clevis.enable (concatMapStringsSep "\n" (elem: "clevis decrypt < /etc/clevis/${elem}.jwe | zfs load-key ${elem} || true ") (filter (p: (elemAt (splitString "/" p) 0) == pool) clevisDatasets))} + ${lib.optionalString config.boot.initrd.clevis.enable (lib.concatMapStringsSep "\n" (elem: "clevis decrypt < /etc/clevis/${elem}.jwe | zfs load-key ${elem} || true ") (lib.filter (p: (lib.elemAt (lib.splitString "/" p) 0) == pool) clevisDatasets))} - ${optionalString keyLocations.hasKeys '' + ${lib.optionalString keyLocations.hasKeys '' ${keyLocations.command} | while IFS=$'\t' read ds kl ks; do { if [[ "$ks" != unavailable ]]; then @@ -193,15 +190,15 @@ let ''; }; - zedConf = generators.toKeyValue { - mkKeyValue = generators.mkKeyValueDefault { + zedConf = lib.generators.toKeyValue { + mkKeyValue = lib.generators.mkKeyValueDefault { mkValueString = v: - if isInt v then toString v - else if isString v then "\"${v}\"" + if lib.isInt v then toString v + else if lib.isString v then "\"${v}\"" else if true == v then "1" else if false == v then "0" - else if isList v then "\"" + (concatStringsSep " " v) + "\"" - else err "this value is" (toString v); + else if lib.isList v then "\"" + (lib.concatStringsSep " " v) + "\"" + else lib.err "this value is" (toString v); } "="; } cfgZED.settings; in @@ -209,38 +206,38 @@ in { imports = [ - (mkRemovedOptionModule [ "boot" "zfs" "enableLegacyCrypto" ] "The corresponding package was removed from nixpkgs.") - (mkRemovedOptionModule [ "boot" "zfs" "enableUnstable" ] "Instead set `boot.zfs.package = pkgs.zfs_unstable;`") + (lib.mkRemovedOptionModule [ "boot" "zfs" "enableLegacyCrypto" ] "The corresponding package was removed from nixpkgs.") + (lib.mkRemovedOptionModule [ "boot" "zfs" "enableUnstable" ] "Instead set `boot.zfs.package = pkgs.zfs_unstable;`") ]; ###### interface options = { boot.zfs = { - package = mkOption { - type = types.package; + package = lib.mkOption { + type = lib.types.package; default = pkgs.zfs; - defaultText = literalExpression "pkgs.zfs"; + defaultText = lib.literalExpression "pkgs.zfs"; description = "Configured ZFS userland tools package, use `pkgs.zfs_unstable` if you want to track the latest staging ZFS branch."; }; - modulePackage = mkOption { + modulePackage = lib.mkOption { internal = true; # It is supposed to be selected automatically, but can be overridden by expert users. default = selectModulePackage cfgZfs.package; - type = types.package; + type = lib.types.package; description = "Configured ZFS kernel module package."; }; - enabled = mkOption { + enabled = lib.mkOption { readOnly = true; - type = types.bool; + type = lib.types.bool; default = inInitrd || inSystem; - defaultText = literalMD "`true` if ZFS filesystem support is enabled"; + defaultText = lib.literalMD "`true` if ZFS filesystem support is enabled"; description = "True if ZFS filesystem support is enabled"; }; - allowHibernation = mkOption { - type = types.bool; + allowHibernation = lib.mkOption { + type = lib.types.bool; default = false; description = '' Allow hibernation support, this may be a unsafe option depending on your @@ -248,8 +245,8 @@ in ''; }; - extraPools = mkOption { - type = types.listOf types.str; + extraPools = lib.mkOption { + type = lib.types.listOf lib.types.str; default = []; example = [ "tank" "data" ]; description = '' @@ -267,8 +264,8 @@ in ''; }; - devNodes = mkOption { - type = types.path; + devNodes = lib.mkOption { + type = lib.types.path; default = "/dev/disk/by-id"; description = '' Name of directory from which to import ZFS devices. @@ -278,8 +275,8 @@ in ''; }; - forceImportRoot = mkOption { - type = types.bool; + forceImportRoot = lib.mkOption { + type = lib.types.bool; default = true; description = '' Forcibly import the ZFS root pool(s) during early boot. @@ -296,8 +293,8 @@ in ''; }; - forceImportAll = mkOption { - type = types.bool; + forceImportAll = lib.mkOption { + type = lib.types.bool; default = false; description = '' Forcibly import all ZFS pool(s). @@ -309,8 +306,8 @@ in ''; }; - requestEncryptionCredentials = mkOption { - type = types.either types.bool (types.listOf types.str); + requestEncryptionCredentials = lib.mkOption { + type = lib.types.either lib.types.bool (lib.types.listOf lib.types.str); default = true; example = [ "tank" "data" ]; description = '' @@ -321,8 +318,8 @@ in ''; }; - passwordTimeout = mkOption { - type = types.int; + passwordTimeout = lib.mkOption { + type = lib.types.int; default = 0; description = '' Timeout in seconds to wait for password entry for decrypt at boot. @@ -332,7 +329,7 @@ in }; removeLinuxDRM = lib.mkOption { - type = types.bool; + type = lib.types.bool; default = false; description = '' Patch the kernel to change symbols needed by ZFS from @@ -345,9 +342,9 @@ in }; services.zfs.autoSnapshot = { - enable = mkOption { + enable = lib.mkOption { default = false; - type = types.bool; + type = lib.types.bool; description = '' Enable the (OpenSolaris-compatible) ZFS auto-snapshotting service. Note that you must set the `com.sun:auto-snapshot` @@ -360,10 +357,10 @@ in ''; }; - flags = mkOption { + flags = lib.mkOption { default = "-k -p"; example = "-k -p --utc"; - type = types.str; + type = lib.types.str; description = '' Flags to pass to the zfs-auto-snapshot command. @@ -379,41 +376,41 @@ in ''; }; - frequent = mkOption { + frequent = lib.mkOption { default = 4; - type = types.int; + type = lib.types.int; description = '' Number of frequent (15-minute) auto-snapshots that you wish to keep. ''; }; - hourly = mkOption { + hourly = lib.mkOption { default = 24; - type = types.int; + type = lib.types.int; description = '' Number of hourly auto-snapshots that you wish to keep. ''; }; - daily = mkOption { + daily = lib.mkOption { default = 7; - type = types.int; + type = lib.types.int; description = '' Number of daily auto-snapshots that you wish to keep. ''; }; - weekly = mkOption { + weekly = lib.mkOption { default = 4; - type = types.int; + type = lib.types.int; description = '' Number of weekly auto-snapshots that you wish to keep. ''; }; - monthly = mkOption { + monthly = lib.mkOption { default = 12; - type = types.int; + type = lib.types.int; description = '' Number of monthly auto-snapshots that you wish to keep. ''; @@ -421,16 +418,16 @@ in }; services.zfs.trim = { - enable = mkOption { + enable = lib.mkOption { description = "Whether to enable periodic TRIM on all ZFS pools."; default = true; example = false; - type = types.bool; + type = lib.types.bool; }; - interval = mkOption { + interval = lib.mkOption { default = "weekly"; - type = types.str; + type = lib.types.str; example = "daily"; description = '' How often we run trim. For most desktop and server systems @@ -441,9 +438,9 @@ in ''; }; - randomizedDelaySec = mkOption { + randomizedDelaySec = lib.mkOption { default = "6h"; - type = types.str; + type = lib.types.str; example = "12h"; description = '' Add a randomized delay before each ZFS trim. @@ -455,11 +452,11 @@ in }; services.zfs.autoScrub = { - enable = mkEnableOption "periodic scrubbing of ZFS pools"; + enable = lib.mkEnableOption "periodic scrubbing of ZFS pools"; - interval = mkOption { + interval = lib.mkOption { default = "monthly"; - type = types.str; + type = lib.types.str; example = "quarterly"; description = '' Systemd calendar expression when to scrub ZFS pools. See @@ -467,9 +464,9 @@ in ''; }; - randomizedDelaySec = mkOption { + randomizedDelaySec = lib.mkOption { default = "6h"; - type = types.str; + type = lib.types.str; example = "12h"; description = '' Add a randomized delay before each ZFS autoscrub. @@ -479,9 +476,9 @@ in ''; }; - pools = mkOption { + pools = lib.mkOption { default = []; - type = types.listOf types.str; + type = lib.types.listOf lib.types.str; example = [ "tank" ]; description = '' List of ZFS pools to periodically scrub. If empty, all pools @@ -490,8 +487,8 @@ in }; }; - services.zfs.expandOnBoot = mkOption { - type = types.either (types.enum [ "disabled" "all" ]) (types.listOf types.str); + services.zfs.expandOnBoot = lib.mkOption { + type = lib.types.either (lib.types.enum [ "disabled" "all" ]) (lib.types.listOf lib.types.str); default = "disabled"; example = [ "tank" "dozer" ]; description = '' @@ -508,10 +505,10 @@ in }; services.zfs.zed = { - enableMail = mkOption { - type = types.bool; + enableMail = lib.mkOption { + type = lib.types.bool; default = config.services.mail.sendmailSetuidWrapper != null; - defaultText = literalExpression '' + defaultText = lib.literalExpression '' config.services.mail.sendmailSetuidWrapper != null ''; description = '' @@ -519,9 +516,9 @@ in ''; }; - settings = mkOption { - type = with types; attrsOf (oneOf [ str int bool (listOf str) ]); - example = literalExpression '' + settings = lib.mkOption { + type = with lib.types; attrsOf (oneOf [ str int bool (listOf str) ]); + example = lib.literalExpression '' { ZED_DEBUG_LOG = "/tmp/zed.debug.log"; @@ -549,8 +546,8 @@ in ###### implementation - config = mkMerge [ - (mkIf cfgZfs.enabled { + config = lib.mkMerge [ + (lib.mkIf cfgZfs.enabled { assertions = [ { assertion = cfgZfs.modulePackage.version == cfgZfs.package.version; @@ -569,7 +566,7 @@ in message = "boot.zfs.allowHibernation while force importing is enabled will cause data corruption"; } { - assertion = !(elem "" allPools); + assertion = !(lib.elem "" allPools); message = '' Automatic pool detection found an empty pool name, which can't be used. Hint: for `fileSystems` entries with `fsType = zfs`, the `device` attribute @@ -591,10 +588,10 @@ in ]; }; - boot.initrd = mkIf inInitrd { + boot.initrd = lib.mkIf inInitrd { kernelModules = [ "zfs" ]; extraUtilsCommands = - mkIf (!config.boot.initrd.systemd.enable) '' + lib.mkIf (!config.boot.initrd.systemd.enable) '' copy_bin_and_libs ${cfgZfs.package}/sbin/zfs copy_bin_and_libs ${cfgZfs.package}/sbin/zdb copy_bin_and_libs ${cfgZfs.package}/sbin/zpool @@ -602,12 +599,12 @@ in copy_bin_and_libs ${cfgZfs.package}/lib/udev/zvol_id ''; extraUtilsCommandsTest = - mkIf (!config.boot.initrd.systemd.enable) '' + lib.mkIf (!config.boot.initrd.systemd.enable) '' $out/bin/zfs --help >/dev/null 2>&1 $out/bin/zpool --help >/dev/null 2>&1 ''; - postResumeCommands = mkIf (!config.boot.initrd.systemd.enable) (concatStringsSep "\n" (['' - ZFS_FORCE="${optionalString cfgZfs.forceImportRoot "-f"}" + postResumeCommands = lib.mkIf (!config.boot.initrd.systemd.enable) (lib.concatStringsSep "\n" (['' + ZFS_FORCE="${lib.optionalString cfgZfs.forceImportRoot "-f"}" ''] ++ [(importLib { # See comments at importLib definition. zpoolCmd = "zpool"; @@ -629,21 +626,21 @@ in poolImported "${pool}" || poolImport "${pool}" # Try one last time, e.g. to import a degraded pool. fi - ${optionalString config.boot.initrd.clevis.enable (concatMapStringsSep "\n" (elem: "clevis decrypt < /etc/clevis/${elem}.jwe | zfs load-key ${elem}") (filter (p: (elemAt (splitString "/" p) 0) == pool) clevisDatasets))} + ${lib.optionalString config.boot.initrd.clevis.enable (lib.concatMapStringsSep "\n" (elem: "clevis decrypt < /etc/clevis/${elem}.jwe | zfs load-key ${elem}") (lib.filter (p: (lib.elemAt (lib.splitString "/" p) 0) == pool) clevisDatasets))} - ${if isBool cfgZfs.requestEncryptionCredentials - then optionalString cfgZfs.requestEncryptionCredentials '' + ${if lib.isBool cfgZfs.requestEncryptionCredentials + then lib.optionalString cfgZfs.requestEncryptionCredentials '' zfs load-key -a '' - else concatMapStrings (fs: '' - zfs load-key -- ${escapeShellArg fs} - '') (filter (x: datasetToPool x == pool) cfgZfs.requestEncryptionCredentials)} + else lib.concatMapStrings (fs: '' + zfs load-key -- ${lib.escapeShellArg fs} + '') (lib.filter (x: datasetToPool x == pool) cfgZfs.requestEncryptionCredentials)} '') rootPools))); # Systemd in stage 1 - systemd = mkIf config.boot.initrd.systemd.enable { + systemd = lib.mkIf config.boot.initrd.systemd.enable { packages = [cfgZfs.package]; - services = listToAttrs (map (pool: createImportService { + services = lib.listToAttrs (map (pool: createImportService { inherit pool; systemd = config.boot.initrd.systemd.package; force = cfgZfs.forceImportRoot; @@ -670,18 +667,18 @@ in systemd.shutdownRamfs.storePaths = ["${cfgZfs.package}/bin/zpool"]; # TODO FIXME See https://github.com/NixOS/nixpkgs/pull/99386#issuecomment-798813567. To not break people's bootloader and as probably not everybody would read release notes that thoroughly add inSystem. - boot.loader.grub = mkIf (inInitrd || inSystem) { + boot.loader.grub = lib.mkIf (inInitrd || inSystem) { zfsSupport = true; zfsPackage = cfgZfs.package; }; services.zfs.zed.settings = { - ZED_EMAIL_PROG = mkIf cfgZED.enableMail (mkDefault ( + ZED_EMAIL_PROG = lib.mkIf cfgZED.enableMail (lib.mkDefault ( config.security.wrapperDir + "/" + config.services.mail.sendmailSetuidWrapper.program )); # subject in header for sendmail - ZED_EMAIL_OPTS = mkIf cfgZED.enableMail (mkDefault "@ADDRESS@"); + ZED_EMAIL_OPTS = lib.mkIf cfgZED.enableMail (lib.mkDefault "@ADDRESS@"); PATH = lib.makeBinPath [ cfgZfs.package @@ -700,7 +697,7 @@ in ACTION=="add|change", KERNEL=="sd[a-z]*[0-9]*|mmcblk[0-9]*p[0-9]*|nvme[0-9]*n[0-9]*p[0-9]*", ENV{ID_FS_TYPE}=="zfs_member", ATTR{../queue/scheduler}="none" ''; - environment.etc = genAttrs + environment.etc = lib.genAttrs (map (file: "zfs/zed.d/${file}") [ @@ -725,7 +722,7 @@ in system.fsPackages = [ cfgZfs.package ]; # XXX: needed? zfs doesn't have (need) a fsck environment.systemPackages = [ cfgZfs.package ] - ++ optional cfgSnapshots.enable autosnapPkg; # so the user can run the command to see flags + ++ lib.optional cfgSnapshots.enable autosnapPkg; # so the user can run the command to see flags services.udev.packages = [ cfgZfs.package ]; # to hook zvol naming, etc. systemd.packages = [ cfgZfs.package ]; @@ -740,7 +737,7 @@ in # This forces a sync of any ZFS pools prior to poweroff, even if they're set # to sync=disabled. createSyncService = pool: - nameValuePair "zfs-sync-${pool}" { + lib.nameValuePair "zfs-sync-${pool}" { description = "Sync ZFS pool \"${pool}\""; wantedBy = [ "shutdown.target" ]; unitConfig = { @@ -756,12 +753,12 @@ in }; createZfsService = serv: - nameValuePair serv { + lib.nameValuePair serv { after = [ "systemd-modules-load.service" ]; wantedBy = [ "zfs.target" ]; }; - in listToAttrs (map createImportService' dataPools ++ + in lib.listToAttrs (map createImportService' dataPools ++ map createSyncService allPools ++ map createZfsService [ "zfs-mount" "zfs-share" "zfs-zed" ]); @@ -770,7 +767,7 @@ in systemd.targets.zfs.wantedBy = [ "multi-user.target" ]; }) - (mkIf (cfgZfs.enabled && cfgExpandOnBoot != "disabled") { + (lib.mkIf (cfgZfs.enabled && cfgExpandOnBoot != "disabled") { systemd.services."zpool-expand@" = { description = "Expand ZFS pools"; after = [ "zfs.target" ]; @@ -823,7 +820,7 @@ in }; }) - (mkIf (cfgZfs.enabled && cfgSnapshots.enable) { + (lib.mkIf (cfgZfs.enabled && cfgSnapshots.enable) { systemd.services = let descr = name: if name == "frequent" then "15 mins" else if name == "hourly" then "hour" @@ -861,7 +858,7 @@ in }) snapshotNames); }) - (mkIf (cfgZfs.enabled && cfgScrub.enable) { + (lib.mkIf (cfgZfs.enabled && cfgScrub.enable) { systemd.services.zfs-scrub = { description = "ZFS pools scrubbing"; after = [ "zfs-import.target" ]; @@ -871,7 +868,7 @@ in script = '' ${cfgZfs.package}/bin/zpool scrub -w ${ if cfgScrub.pools != [] then - (concatStringsSep " " cfgScrub.pools) + (lib.concatStringsSep " " cfgScrub.pools) else "$(${cfgZfs.package}/bin/zpool list -H -o name)" } @@ -889,7 +886,7 @@ in }; }) - (mkIf (cfgZfs.enabled && cfgTrim.enable) { + (lib.mkIf (cfgZfs.enabled && cfgTrim.enable) { systemd.services.zpool-trim = { description = "ZFS pools trim"; after = [ "zfs-import.target" ]; From eef4c18ae8a5be20c11289822d14dcc0f191c056 Mon Sep 17 00:00:00 2001 From: Andrew Marshall Date: Tue, 24 Sep 2024 14:26:08 -0400 Subject: [PATCH 3/4] nixos/zfs: remove unused vars --- nixos/modules/tasks/filesystems/zfs.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nixos/modules/tasks/filesystems/zfs.nix b/nixos/modules/tasks/filesystems/zfs.nix index 1145338fba83..182c539cb7b9 100644 --- a/nixos/modules/tasks/filesystems/zfs.nix +++ b/nixos/modules/tasks/filesystems/zfs.nix @@ -1,11 +1,10 @@ -{ config, lib, options, pkgs, utils, ... }: +{ config, lib, pkgs, utils, ... }: # # TODO: zfs tunables let cfgZfs = config.boot.zfs; - optZfs = options.boot.zfs; cfgExpandOnBoot = config.services.zfs.expandOnBoot; cfgSnapshots = config.services.zfs.autoSnapshot; cfgSnapFlags = cfgSnapshots.flags; From bec753ddfc291508995595be32b0804c8d68bd69 Mon Sep 17 00:00:00 2001 From: Andrew Marshall Date: Tue, 24 Sep 2024 14:39:25 -0400 Subject: [PATCH 4/4] nixos/zfs: replace `with` with `let` The fully-qualified name would certainly be a lot here, but `with` can still be unclear even with narrow scope. A short `let` adds clarity without significantly increasing verbosity. --- nixos/modules/tasks/filesystems/zfs.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/modules/tasks/filesystems/zfs.nix b/nixos/modules/tasks/filesystems/zfs.nix index 182c539cb7b9..03b67438d7b6 100644 --- a/nixos/modules/tasks/filesystems/zfs.nix +++ b/nixos/modules/tasks/filesystems/zfs.nix @@ -516,7 +516,7 @@ in }; settings = lib.mkOption { - type = with lib.types; attrsOf (oneOf [ str int bool (listOf str) ]); + type = let t = lib.types; in t.attrsOf (t.oneOf [ t.str t.int t.bool (t.listOf t.str) ]); example = lib.literalExpression '' { ZED_DEBUG_LOG = "/tmp/zed.debug.log";