From b2a106f8ed84b948cbde5031ddc0942e709b8e1e Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Thu, 1 Aug 2024 15:41:09 -0400 Subject: [PATCH 01/10] zfs: Add topology attribute to zpool --- lib/default.nix | 1 + lib/types/zpool.nix | 118 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 110 insertions(+), 9 deletions(-) diff --git a/lib/default.nix b/lib/default.nix index f4337f9f..39753d43 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -195,6 +195,7 @@ let || isAttrsOfSubmodule o # TODO don't hardcode diskoLib.subType options. || n == "content" || n == "partitions" || n == "datasets" || n == "swap" + || n == "topology" ); in lib.toShellVars diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index f690c984..055ad84d 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -1,4 +1,20 @@ { config, options, lib, diskoLib, rootMountPoint, ... }: +let + modeOptions = [ + "" + "mirror" + "raidz" + "raidz2" + "raidz3" + ]; + format_output = (mode: members: '' + entries+=("${mode}=${ + lib.concatMapStringsSep " " + (d: "/dev/disk/by-partlabel/disk-${d}-zfs") members + }") + ''); + format_vdev = (vdev: format_output vdev.mode vdev.members); +in { options = { name = lib.mkOption { @@ -13,13 +29,7 @@ description = "Type"; }; mode = lib.mkOption { - type = lib.types.enum [ - "" - "mirror" - "raidz" - "raidz2" - "raidz3" - ]; + type = lib.types.enum (modeOptions ++ [ "prescribed" ]); default = ""; description = "Mode of the ZFS pool"; }; @@ -50,6 +60,55 @@ }); description = "List of datasets to define"; }; + topology = lib.mkOption { + type = + let + vdev = lib.types.submodule ({ name, ... }: { + options = { + mode = lib.mkOption { + type = lib.types.enum modeOptions; + default = ""; + description = "mode of the zfs vdev"; + }; + members = lib.mkOption { + type = lib.types.listOf lib.types.str; + description = "Members of the vdev"; + }; + }; + }); + parent = config; + in + lib.types.nullOr + (lib.types.submodule + ({ config, name, ... }: { + options = { + type = lib.mkOption { + type = lib.types.enum [ "zfs_topology" ]; + default = "zfs_topology"; + internal = true; + description = "Type"; + }; + # zfs disk types + vdev = lib.mkOption { + type = lib.types.listOf vdev; + default = [ ]; + description = "A list of storage vdevs"; + }; + special = lib.mkOption { + type = lib.types.nullOr vdev; + default = null; + description = "A list of devices for the special vdev"; + }; + cache = lib.mkOption { + type = lib.types.nullOr lib.types.str; + default = null; + description = "The cache device"; + }; + }; + })); + default = null; + description = "Topology of the ZFS pool"; + }; _meta = lib.mkOption { internal = true; readOnly = true; @@ -84,11 +143,52 @@ fi done if [ $continue -eq 1 ]; then + topology="" + # For shell check + mode="${config.mode}" + if [ $mode != "prescribed" ]; then + ${if config.topology == null then + ''topology="${config.mode} \"''${zfs_devices}\""'' + else + '' + echo "topology cannot be set when mode != 'prescribed', skipping creating zpool ${config.name}" >&2 + continue=0 + '' + } + else + entries=() + ${lib.concatMapStrings format_vdev config.topology.vdev} + ${lib.optionalString (config.topology.special != null) + (format_output "special ${config.topology.special.mode}" config.topology.special.members)} + ${lib.optionalString (config.topology.cache != null) + (format_output "cache" [config.topology.cache])} + all_devices=() + for line in "''${entries[@]}"; do + # lineformat is mode=device1 device2 device3 + mode=''${line%%=*} + devs=''${line#*=} + IFS=' ' read -r -a devices <<< "$devs" + all_devices+=("''${devices[@]}") + # shellcheck disable=SC2089 + topology+=" ''${mode} ''${devices[*]}" + done + # all_devices sorted should equal zfs_devices sorted + all_devices_list=$(echo "''${all_devices[*]}" | tr ' ' '\n' | sort) + zfs_devices_list=$(echo "''${zfs_devices[*]}" | tr ' ' '\n' | sort) + if [[ "$all_devices_list" != "$zfs_devices_list" ]]; then + echo "not all disks accounted for, skipping creating zpool ${config.name}" >&2 + diff <(echo "$all_devices_list" ) <(echo "$zfs_devices_list") >&2 + continue=0 + fi + fi + fi + if [ $continue -eq 1 ]; then + # shellcheck disable=SC2090 zpool create -f ${config.name} \ - -R ${rootMountPoint} ${config.mode} \ + -R ${rootMountPoint} \ ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \ ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-O ${n}=${v}") config.rootFsOptions)} \ - "''${zfs_devices[@]}" + ''${topology:+ $topology} if [[ $(zfs get -H mounted ${config.name} | cut -f3) == "yes" ]]; then zfs unmount ${config.name} fi From 6bebcc728e85efb22b639a1ac308e90764201f0d Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Thu, 1 Aug 2024 15:59:33 -0400 Subject: [PATCH 02/10] tidy: move relevant variables and format script block --- lib/types/zpool.nix | 161 +++++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 78 deletions(-) diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index 055ad84d..d3fcda40 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -7,13 +7,6 @@ let "raidz2" "raidz3" ]; - format_output = (mode: members: '' - entries+=("${mode}=${ - lib.concatMapStringsSep " " - (d: "/dev/disk/by-partlabel/disk-${d}-zfs") members - }") - ''); - format_vdev = (vdev: format_output vdev.mode vdev.members); in { options = { @@ -119,83 +112,95 @@ in }; _create = diskoLib.mkCreateOption { inherit config options; - default = '' - readarray -t zfs_devices < <(cat "$disko_devices_dir"/zfs_${config.name}) - # Try importing the pool without mounting anything if it exists. - # This allows us to set mounpoints. - if zpool import -N -f '${config.name}' || zpool list '${config.name}'; then - echo "not creating zpool ${config.name} as a pool with that name already exists" >&2 - else - continue=1 - for dev in "''${zfs_devices[@]}"; do - if ! blkid "$dev" >/dev/null; then - # blkid fails, so device seems empty - : - elif (blkid "$dev" -o export | grep '^PTUUID='); then - echo "device $dev already has a partuuid, skipping creating zpool ${config.name}" >&2 - continue=0 - elif (blkid "$dev" -o export | grep '^TYPE=zfs_member'); then - # zfs_member is a zfs partition, so we try to add the device to the pool - : - elif (blkid "$dev" -o export | grep '^TYPE='); then - echo "device $dev already has a partition, skipping creating zpool ${config.name}" >&2 - continue=0 - fi - done - if [ $continue -eq 1 ]; then - topology="" - # For shell check - mode="${config.mode}" - if [ $mode != "prescribed" ]; then - ${if config.topology == null then - ''topology="${config.mode} \"''${zfs_devices}\""'' - else - '' - echo "topology cannot be set when mode != 'prescribed', skipping creating zpool ${config.name}" >&2 + default = + let + format_output = (mode: members: '' + entries+=("${mode}=${ + lib.concatMapStringsSep " " + (d: "/dev/disk/by-partlabel/disk-${d}-zfs") members + }") + ''); + format_vdev = (vdev: format_output vdev.mode vdev.members); + hasTopology = config.topology != null; + in + '' + readarray -t zfs_devices < <(cat "$disko_devices_dir"/zfs_${config.name}) + # Try importing the pool without mounting anything if it exists. + # This allows us to set mounpoints. + if zpool import -N -f '${config.name}' || zpool list '${config.name}'; then + echo "not creating zpool ${config.name} as a pool with that name already exists" >&2 + else + continue=1 + for dev in "''${zfs_devices[@]}"; do + if ! blkid "$dev" >/dev/null; then + # blkid fails, so device seems empty + : + elif (blkid "$dev" -o export | grep '^PTUUID='); then + echo "device $dev already has a partuuid, skipping creating zpool ${config.name}" >&2 continue=0 - '' - } - else - entries=() - ${lib.concatMapStrings format_vdev config.topology.vdev} - ${lib.optionalString (config.topology.special != null) - (format_output "special ${config.topology.special.mode}" config.topology.special.members)} - ${lib.optionalString (config.topology.cache != null) - (format_output "cache" [config.topology.cache])} - all_devices=() - for line in "''${entries[@]}"; do - # lineformat is mode=device1 device2 device3 - mode=''${line%%=*} - devs=''${line#*=} - IFS=' ' read -r -a devices <<< "$devs" - all_devices+=("''${devices[@]}") - # shellcheck disable=SC2089 - topology+=" ''${mode} ''${devices[*]}" - done - # all_devices sorted should equal zfs_devices sorted - all_devices_list=$(echo "''${all_devices[*]}" | tr ' ' '\n' | sort) - zfs_devices_list=$(echo "''${zfs_devices[*]}" | tr ' ' '\n' | sort) - if [[ "$all_devices_list" != "$zfs_devices_list" ]]; then - echo "not all disks accounted for, skipping creating zpool ${config.name}" >&2 - diff <(echo "$all_devices_list" ) <(echo "$zfs_devices_list") >&2 + elif (blkid "$dev" -o export | grep '^TYPE=zfs_member'); then + # zfs_member is a zfs partition, so we try to add the device to the pool + : + elif (blkid "$dev" -o export | grep '^TYPE='); then + echo "device $dev already has a partition, skipping creating zpool ${config.name}" >&2 continue=0 fi + done + if [ $continue -eq 1 ]; then + topology="" + # For shell check + mode="${config.mode}" + if [ $mode != "prescribed" ]; then + ${if hasTopology then + ''topology="${config.mode} \"''${zfs_devices}\""'' + else + '' + echo "topology cannot be set when mode != 'prescribed', skipping creating zpool ${config.name}" >&2 + continue=0 + '' + } + else + entries=() + ${lib.optionalString (hasTopology && config.topology.vdev != null) + (lib.concatMapStrings format_vdev config.topology.vdev)} + ${lib.optionalString (hasTopology && config.topology.special != null) + (format_output "special ${config.topology.special.mode}" config.topology.special.members)} + ${lib.optionalString (hasTopology && config.topology.cache != null) + (format_output "cache" [config.topology.cache])} + all_devices=() + for line in "''${entries[@]}"; do + # lineformat is mode=device1 device2 device3 + mode=''${line%%=*} + devs=''${line#*=} + IFS=' ' read -r -a devices <<< "$devs" + all_devices+=("''${devices[@]}") + # shellcheck disable=SC2089 + topology+=" ''${mode} ''${devices[*]}" + done + # all_devices sorted should equal zfs_devices sorted + all_devices_list=$(echo "''${all_devices[*]}" | tr ' ' '\n' | sort) + zfs_devices_list=$(echo "''${zfs_devices[*]}" | tr ' ' '\n' | sort) + if [[ "$all_devices_list" != "$zfs_devices_list" ]]; then + echo "not all disks accounted for, skipping creating zpool ${config.name}" >&2 + diff <(echo "$all_devices_list" ) <(echo "$zfs_devices_list") >&2 + continue=0 + fi + fi fi - fi - if [ $continue -eq 1 ]; then - # shellcheck disable=SC2090 - zpool create -f ${config.name} \ - -R ${rootMountPoint} \ - ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \ - ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-O ${n}=${v}") config.rootFsOptions)} \ - ''${topology:+ $topology} - if [[ $(zfs get -H mounted ${config.name} | cut -f3) == "yes" ]]; then - zfs unmount ${config.name} + if [ $continue -eq 1 ]; then + # shellcheck disable=SC2090 + zpool create -f ${config.name} \ + -R ${rootMountPoint} \ + ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \ + ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-O ${n}=${v}") config.rootFsOptions)} \ + ''${topology:+ $topology} + if [[ $(zfs get -H mounted ${config.name} | cut -f3) == "yes" ]]; then + zfs unmount ${config.name} + fi fi fi - fi - ${lib.concatMapStrings (dataset: dataset._create) (lib.attrValues config.datasets)} - ''; + ${lib.concatMapStrings (dataset: dataset._create) (lib.attrValues config.datasets)} + ''; }; _mount = diskoLib.mkMountOption { inherit config options; From f484389085ea1ebd697516510d944dec55515288 Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Thu, 1 Aug 2024 16:00:11 -0400 Subject: [PATCH 03/10] example: barebones example of zfs-with-vdevs --- example/zfs-with-vdevs.nix | 114 +++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 example/zfs-with-vdevs.nix diff --git a/example/zfs-with-vdevs.nix b/example/zfs-with-vdevs.nix new file mode 100644 index 00000000..cd85addb --- /dev/null +++ b/example/zfs-with-vdevs.nix @@ -0,0 +1,114 @@ +{ + disko.devices = { + disk = { + x = { + type = "disk"; + device = "/dev/sdx"; + content = { + type = "gpt"; + partitions = { + ESP = { + size = "64M"; + type = "EF00"; + content = { + type = "filesystem"; + format = "vfat"; + mountpoint = "/boot"; + }; + }; + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + y = { + type = "disk"; + device = "/dev/sdy"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + z = { + type = "disk"; + device = "/dev/sdz"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + cache = { + type = "disk"; + device = "/dev/sdc"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + }; + zpool = { + zroot = { + type = "zpool"; + mode = "prescribed"; + rootFsOptions = { + compression = "zstd"; + "com.sun:auto-snapshot" = "false"; + }; + mountpoint = "/"; + + topology = { + type = "zfs_topology"; + vdev = [ + { + mode = "mirror"; + members = [ "x" "y" ]; + } + ]; + special = { + members = [ "z" ]; + }; + cache = "cache"; + }; + + datasets = { + # See examples/zfs.nix for more comprehensive usage. + zfs_fs = { + type = "zfs_fs"; + mountpoint = "/zfs_fs"; + options."com.sun:auto-snapshot" = "true"; + }; + }; + }; + }; + }; +} + From 8d071db09b1cc5f4a31940a53bd287f63e915052 Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Mon, 5 Aug 2024 12:11:35 -0400 Subject: [PATCH 04/10] test: fix and add stub test for zfs-with-vdevs --- example/zfs-with-vdevs.nix | 15 +++++++-------- example/zfs.nix | 1 - lib/types/zpool.nix | 8 +++----- tests/zfs-with-vdevs.nix | 24 ++++++++++++++++++++++++ 4 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 tests/zfs-with-vdevs.nix diff --git a/example/zfs-with-vdevs.nix b/example/zfs-with-vdevs.nix index cd85addb..0058ac5f 100644 --- a/example/zfs-with-vdevs.nix +++ b/example/zfs-with-vdevs.nix @@ -60,7 +60,7 @@ }; cache = { type = "disk"; - device = "/dev/sdc"; + device = "/dev/vdc"; content = { type = "gpt"; partitions = { @@ -100,15 +100,14 @@ }; datasets = { - # See examples/zfs.nix for more comprehensive usage. - zfs_fs = { - type = "zfs_fs"; - mountpoint = "/zfs_fs"; - options."com.sun:auto-snapshot" = "true"; - }; + # See examples/zfs.nix for more comprehensive usage. + zfs_fs = { + type = "zfs_fs"; + mountpoint = "/zfs_fs"; + options."com.sun:auto-snapshot" = "true"; + }; }; }; }; }; } - diff --git a/example/zfs.nix b/example/zfs.nix index 933386a6..4d09061b 100644 --- a/example/zfs.nix +++ b/example/zfs.nix @@ -100,4 +100,3 @@ }; }; } - diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index d3fcda40..b7e35d37 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -150,9 +150,9 @@ in topology="" # For shell check mode="${config.mode}" - if [ $mode != "prescribed" ]; then - ${if hasTopology then - ''topology="${config.mode} \"''${zfs_devices}\""'' + if [ "$mode" != "prescribed" ]; then + ${if !hasTopology then + ''topology="${config.mode} ''${zfs_devices[*]}"'' else '' echo "topology cannot be set when mode != 'prescribed', skipping creating zpool ${config.name}" >&2 @@ -174,7 +174,6 @@ in devs=''${line#*=} IFS=' ' read -r -a devices <<< "$devs" all_devices+=("''${devices[@]}") - # shellcheck disable=SC2089 topology+=" ''${mode} ''${devices[*]}" done # all_devices sorted should equal zfs_devices sorted @@ -188,7 +187,6 @@ in fi fi if [ $continue -eq 1 ]; then - # shellcheck disable=SC2090 zpool create -f ${config.name} \ -R ${rootMountPoint} \ ${lib.concatStringsSep " " (lib.mapAttrsToList (n: v: "-o ${n}=${v}") config.options)} \ diff --git a/tests/zfs-with-vdevs.nix b/tests/zfs-with-vdevs.nix new file mode 100644 index 00000000..5f8c67d3 --- /dev/null +++ b/tests/zfs-with-vdevs.nix @@ -0,0 +1,24 @@ +{ pkgs ? import { } +, diskoLib ? pkgs.callPackage ../lib { } +}: +diskoLib.testLib.makeDiskoTest { + inherit pkgs; + name = "zfs-with-vdevs"; + disko-config = ../example/zfs-with-vdevs.nix; + extraInstallerConfig.networking.hostId = "8425e349"; + extraSystemConfig = { + networking.hostId = "8425e349"; + }; + extraTestScript = '' + def assert_property(ds, property, expected_value): + out = machine.succeed(f"zfs get -H {property} {ds} -o value").rstrip() + assert ( + out == expected_value + ), f"Expected {property}={expected_value} on {ds}, got: {out}" + + assert_property("zroot", "compression", "zstd") + assert_property("zroot/zfs_fs", "com.sun:auto-snapshot", "true") + assert_property("zroot/zfs_fs", "compression", "zstd") + machine.succeed("mountpoint /zfs_fs"); + ''; +} From 540cd416f689b2eaf482752bec1f0c0417b37537 Mon Sep 17 00:00:00 2001 From: zerox Date: Mon, 26 Aug 2024 09:48:53 +0500 Subject: [PATCH 05/10] zfs: add ability to specify full path of to the disk. --- lib/types/zpool.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index b7e35d37..f6167be1 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -117,7 +117,7 @@ in format_output = (mode: members: '' entries+=("${mode}=${ lib.concatMapStringsSep " " - (d: "/dev/disk/by-partlabel/disk-${d}-zfs") members + (d: if lib.strings.hasPrefix "/" d then d else "/dev/disk/by-partlabel/disk-${d}-zfs") members }") ''); format_vdev = (vdev: format_output vdev.mode vdev.members); From cc2e247193513a378fe0457374d4de7823aede1f Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Mon, 26 Aug 2024 12:24:41 -0400 Subject: [PATCH 06/10] zfs: make topology a mode type --- example/zfs-with-vdevs.nix | 31 +++++---- lib/default.nix | 2 +- lib/types/zpool.nix | 135 ++++++++++++++++++------------------- 3 files changed, 83 insertions(+), 85 deletions(-) diff --git a/example/zfs-with-vdevs.nix b/example/zfs-with-vdevs.nix index 0058ac5f..864cbbd3 100644 --- a/example/zfs-with-vdevs.nix +++ b/example/zfs-with-vdevs.nix @@ -78,27 +78,26 @@ zpool = { zroot = { type = "zpool"; - mode = "prescribed"; + mode = { + topology = { + vdev = [ + { + mode = "mirror"; + members = [ "x" "y" ]; + } + ]; + special = { + members = [ "z" ]; + }; + cache = "cache"; + }; + }; + rootFsOptions = { compression = "zstd"; "com.sun:auto-snapshot" = "false"; }; mountpoint = "/"; - - topology = { - type = "zfs_topology"; - vdev = [ - { - mode = "mirror"; - members = [ "x" "y" ]; - } - ]; - special = { - members = [ "z" ]; - }; - cache = "cache"; - }; - datasets = { # See examples/zfs.nix for more comprehensive usage. zfs_fs = { diff --git a/lib/default.nix b/lib/default.nix index 39753d43..31acc9cb 100644 --- a/lib/default.nix +++ b/lib/default.nix @@ -195,7 +195,7 @@ let || isAttrsOfSubmodule o # TODO don't hardcode diskoLib.subType options. || n == "content" || n == "partitions" || n == "datasets" || n == "swap" - || n == "topology" + || n == "mode" ); in lib.toShellVars diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index f6167be1..415f221c 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -22,8 +22,61 @@ in description = "Type"; }; mode = lib.mkOption { - type = lib.types.enum (modeOptions ++ [ "prescribed" ]); default = ""; + type = (lib.types.enum modeOptions) // (lib.types.attrsOf (diskoLib.subType { + types = { + topology = + let + vdev = lib.types.submodule ({ name, ... }: { + options = { + mode = lib.mkOption { + type = lib.types.enum modeOptions; + default = ""; + description = "mode of the zfs vdev"; + }; + members = lib.mkOption { + type = lib.types.listOf lib.types.str; + description = "Members of the vdev"; + }; + }; + }); + parent = config; + in + lib.types.submodule + ({ config, name, ... }: { + options = { + type = lib.mkOption { + type = lib.types.enum [ "topology" ]; + default = "topology"; + internal = true; + description = "Type"; + }; + # zfs disk types + vdev = lib.mkOption { + type = lib.types.listOf vdev; + default = [ ]; + description = "A list of storage vdevs"; + example = [{ + mode = "mirror"; + members = [ "x" "y" ]; + }]; + }; + special = lib.mkOption { + type = lib.types.nullOr vdev; + default = null; + description = "A list of devices for the special vdev"; + }; + cache = lib.mkOption { + type = lib.types.nullOr lib.types.str; + default = null; + description = "The cache device"; + }; + # TODO: Consider + }; + }); + }; + extraArgs.parent = config; + })); description = "Mode of the ZFS pool"; }; options = lib.mkOption { @@ -53,55 +106,6 @@ in }); description = "List of datasets to define"; }; - topology = lib.mkOption { - type = - let - vdev = lib.types.submodule ({ name, ... }: { - options = { - mode = lib.mkOption { - type = lib.types.enum modeOptions; - default = ""; - description = "mode of the zfs vdev"; - }; - members = lib.mkOption { - type = lib.types.listOf lib.types.str; - description = "Members of the vdev"; - }; - }; - }); - parent = config; - in - lib.types.nullOr - (lib.types.submodule - ({ config, name, ... }: { - options = { - type = lib.mkOption { - type = lib.types.enum [ "zfs_topology" ]; - default = "zfs_topology"; - internal = true; - description = "Type"; - }; - # zfs disk types - vdev = lib.mkOption { - type = lib.types.listOf vdev; - default = [ ]; - description = "A list of storage vdevs"; - }; - special = lib.mkOption { - type = lib.types.nullOr vdev; - default = null; - description = "A list of devices for the special vdev"; - }; - cache = lib.mkOption { - type = lib.types.nullOr lib.types.str; - default = null; - description = "The cache device"; - }; - }; - })); - default = null; - description = "Topology of the ZFS pool"; - }; _meta = lib.mkOption { internal = true; readOnly = true; @@ -114,14 +118,16 @@ in inherit config options; default = let - format_output = (mode: members: '' + formatOutput = (mode: members: '' entries+=("${mode}=${ lib.concatMapStringsSep " " (d: if lib.strings.hasPrefix "/" d then d else "/dev/disk/by-partlabel/disk-${d}-zfs") members }") ''); - format_vdev = (vdev: format_output vdev.mode vdev.members); - hasTopology = config.topology != null; + formatVdev = (vdev: formatOutput vdev.mode vdev.members); + hasTopology = builtins.isString config.mode; + mode = if hasTopology then config.mode else "prescribed"; + topology = if hasTopology then config.mode.topology else { }; in '' readarray -t zfs_devices < <(cat "$disko_devices_dir"/zfs_${config.name}) @@ -149,24 +155,17 @@ in if [ $continue -eq 1 ]; then topology="" # For shell check - mode="${config.mode}" + mode="${mode}" if [ "$mode" != "prescribed" ]; then - ${if !hasTopology then - ''topology="${config.mode} ''${zfs_devices[*]}"'' - else - '' - echo "topology cannot be set when mode != 'prescribed', skipping creating zpool ${config.name}" >&2 - continue=0 - '' - } + topology="${mode} ''${zfs_devices[*]}" else entries=() - ${lib.optionalString (hasTopology && config.topology.vdev != null) - (lib.concatMapStrings format_vdev config.topology.vdev)} - ${lib.optionalString (hasTopology && config.topology.special != null) - (format_output "special ${config.topology.special.mode}" config.topology.special.members)} - ${lib.optionalString (hasTopology && config.topology.cache != null) - (format_output "cache" [config.topology.cache])} + ${lib.optionalString (hasTopology && topology.vdev != null) + (lib.concatMapStrings formatVdev topology.vdev)} + ${lib.optionalString (hasTopology && topology.special != null) + (formatOutput "special ${topology.special.mode}" topology.special.members)} + ${lib.optionalString (hasTopology && topology.cache != null) + (formatOutput "cache" [topology.cache])} all_devices=() for line in "''${entries[@]}"; do # lineformat is mode=device1 device2 device3 From ea3ce722ea9bd569cee40d797f115f1e7489cc8b Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Mon, 26 Aug 2024 13:03:25 -0400 Subject: [PATCH 07/10] zfs: fix test and add documentation --- example/zfs-with-vdevs.nix | 1 + lib/types/zfs.nix | 1 + lib/types/zpool.nix | 38 ++++++++++++++++++++++++++------------ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/example/zfs-with-vdevs.nix b/example/zfs-with-vdevs.nix index 864cbbd3..caaa4a0d 100644 --- a/example/zfs-with-vdevs.nix +++ b/example/zfs-with-vdevs.nix @@ -80,6 +80,7 @@ type = "zpool"; mode = { topology = { + type = "topology"; vdev = [ { mode = "mirror"; diff --git a/lib/types/zfs.nix b/lib/types/zfs.nix index 9aa8577c..2b43623a 100644 --- a/lib/types/zfs.nix +++ b/lib/types/zfs.nix @@ -53,3 +53,4 @@ }; }; } + diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index 415f221c..858f456e 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -1,9 +1,11 @@ { config, options, lib, diskoLib, rootMountPoint, ... }: let + # TODO: Consider expanding to handle `disk` `file` and `draid` mode options. modeOptions = [ "" "mirror" "raidz" + "raidz1" "raidz2" "raidz3" ]; @@ -32,7 +34,7 @@ in mode = lib.mkOption { type = lib.types.enum modeOptions; default = ""; - description = "mode of the zfs vdev"; + description = "Mode of the zfs vdev"; }; members = lib.mkOption { type = lib.types.listOf lib.types.str; @@ -51,27 +53,39 @@ in internal = true; description = "Type"; }; - # zfs disk types + # zfs device types vdev = lib.mkOption { type = lib.types.listOf vdev; default = [ ]; - description = "A list of storage vdevs"; + description = '' + A list of storage vdevs. See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Virtual_Devices_(vdevs) + for details. + ''; example = [{ mode = "mirror"; - members = [ "x" "y" ]; + members = [ "x" "y" "/dev/sda1" ]; }]; }; special = lib.mkOption { type = lib.types.nullOr vdev; default = null; - description = "A list of devices for the special vdev"; + description = '' + A vdev definition for a special device. See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#special + for details. + ''; }; cache = lib.mkOption { - type = lib.types.nullOr lib.types.str; + type = lib.types.listOf lib.types.str; default = null; - description = "The cache device"; + description = '' + A dedicated zfs cache device (L2ARC). See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Cache_Devices + for details. + ''; }; - # TODO: Consider + # TODO: Consider supporting log, spare, and dedup options. }; }); }; @@ -125,8 +139,8 @@ in }") ''); formatVdev = (vdev: formatOutput vdev.mode vdev.members); - hasTopology = builtins.isString config.mode; - mode = if hasTopology then config.mode else "prescribed"; + hasTopology = !(builtins.isString config.mode); + mode = if !hasTopology then config.mode else "prescribed"; topology = if hasTopology then config.mode.topology else { }; in '' @@ -164,8 +178,8 @@ in (lib.concatMapStrings formatVdev topology.vdev)} ${lib.optionalString (hasTopology && topology.special != null) (formatOutput "special ${topology.special.mode}" topology.special.members)} - ${lib.optionalString (hasTopology && topology.cache != null) - (formatOutput "cache" [topology.cache])} + ${lib.optionalString (hasTopology && topology.cache != []) + (formatOutput "cache" topology.cache)} all_devices=() for line in "''${entries[@]}"; do # lineformat is mode=device1 device2 device3 From c9d3bc37553140444ad51df29a231aece29f97ea Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Mon, 26 Aug 2024 14:00:44 -0400 Subject: [PATCH 08/10] fix: properly apply oneOf --- example/zfs-with-vdevs.nix | 2 +- example/zfs.nix | 1 + lib/types/zfs.nix | 1 - lib/types/zpool.nix | 131 +++++++++++++++++++------------------ 4 files changed, 69 insertions(+), 66 deletions(-) diff --git a/example/zfs-with-vdevs.nix b/example/zfs-with-vdevs.nix index caaa4a0d..a61680a6 100644 --- a/example/zfs-with-vdevs.nix +++ b/example/zfs-with-vdevs.nix @@ -90,7 +90,7 @@ special = { members = [ "z" ]; }; - cache = "cache"; + cache = [ "cache" ]; }; }; diff --git a/example/zfs.nix b/example/zfs.nix index 4d09061b..933386a6 100644 --- a/example/zfs.nix +++ b/example/zfs.nix @@ -100,3 +100,4 @@ }; }; } + diff --git a/lib/types/zfs.nix b/lib/types/zfs.nix index 2b43623a..9aa8577c 100644 --- a/lib/types/zfs.nix +++ b/lib/types/zfs.nix @@ -53,4 +53,3 @@ }; }; } - diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index 858f456e..2ddbb0e4 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -25,72 +25,75 @@ in }; mode = lib.mkOption { default = ""; - type = (lib.types.enum modeOptions) // (lib.types.attrsOf (diskoLib.subType { - types = { - topology = - let - vdev = lib.types.submodule ({ name, ... }: { - options = { - mode = lib.mkOption { - type = lib.types.enum modeOptions; - default = ""; - description = "Mode of the zfs vdev"; + type = (lib.types.oneOf [ + (lib.types.enum modeOptions) + (lib.types.attrsOf (diskoLib.subType { + types = { + topology = + let + vdev = lib.types.submodule ({ name, ... }: { + options = { + mode = lib.mkOption { + type = lib.types.enum modeOptions; + default = ""; + description = "Mode of the zfs vdev"; + }; + members = lib.mkOption { + type = lib.types.listOf lib.types.str; + description = "Members of the vdev"; + }; }; - members = lib.mkOption { - type = lib.types.listOf lib.types.str; - description = "Members of the vdev"; + }); + parent = config; + in + lib.types.submodule + ({ config, name, ... }: { + options = { + type = lib.mkOption { + type = lib.types.enum [ "topology" ]; + default = "topology"; + internal = true; + description = "Type"; + }; + # zfs device types + vdev = lib.mkOption { + type = lib.types.listOf vdev; + default = [ ]; + description = '' + A list of storage vdevs. See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Virtual_Devices_(vdevs) + for details. + ''; + example = [{ + mode = "mirror"; + members = [ "x" "y" "/dev/sda1" ]; + }]; + }; + special = lib.mkOption { + type = lib.types.nullOr vdev; + default = null; + description = '' + A vdev definition for a special device. See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#special + for details. + ''; + }; + cache = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = null; + description = '' + A dedicated zfs cache device (L2ARC). See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Cache_Devices + for details. + ''; + }; + # TODO: Consider supporting log, spare, and dedup options. }; - }; - }); - parent = config; - in - lib.types.submodule - ({ config, name, ... }: { - options = { - type = lib.mkOption { - type = lib.types.enum [ "topology" ]; - default = "topology"; - internal = true; - description = "Type"; - }; - # zfs device types - vdev = lib.mkOption { - type = lib.types.listOf vdev; - default = [ ]; - description = '' - A list of storage vdevs. See - https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Virtual_Devices_(vdevs) - for details. - ''; - example = [{ - mode = "mirror"; - members = [ "x" "y" "/dev/sda1" ]; - }]; - }; - special = lib.mkOption { - type = lib.types.nullOr vdev; - default = null; - description = '' - A vdev definition for a special device. See - https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#special - for details. - ''; - }; - cache = lib.mkOption { - type = lib.types.listOf lib.types.str; - default = null; - description = '' - A dedicated zfs cache device (L2ARC). See - https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Cache_Devices - for details. - ''; - }; - # TODO: Consider supporting log, spare, and dedup options. - }; - }); - }; - extraArgs.parent = config; - })); + }); + }; + extraArgs.parent = config; + })) + ]); description = "Mode of the ZFS pool"; }; options = lib.mkOption { From 071306e76a7d1a05d0ffa4251a466c3d300e268d Mon Sep 17 00:00:00 2001 From: Dylan Madisetti Date: Mon, 26 Aug 2024 14:13:07 -0400 Subject: [PATCH 09/10] tidy: apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörg Thalheim --- lib/types/zpool.nix | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index 2ddbb0e4..0510b940 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -135,16 +135,16 @@ in inherit config options; default = let - formatOutput = (mode: members: '' + formatOutput = mode: members: '' entries+=("${mode}=${ lib.concatMapStringsSep " " (d: if lib.strings.hasPrefix "/" d then d else "/dev/disk/by-partlabel/disk-${d}-zfs") members }") - ''); - formatVdev = (vdev: formatOutput vdev.mode vdev.members); + ''; + formatVdev = vdev: formatOutput vdev.mode vdev.members; hasTopology = !(builtins.isString config.mode); - mode = if !hasTopology then config.mode else "prescribed"; - topology = if hasTopology then config.mode.topology else { }; + mode = if hasTopology then "prescribed" else config.mode; + topology = lib.optionalAttrs hasTopology config.mode.topology; in '' readarray -t zfs_devices < <(cat "$disko_devices_dir"/zfs_${config.name}) From cf5d451adc87720753f5a85caac2c81e83550110 Mon Sep 17 00:00:00 2001 From: dylan madisetti Date: Tue, 27 Aug 2024 08:29:31 -0400 Subject: [PATCH 10/10] test: Add cache specific test --- tests/zfs-with-vdevs.nix | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/zfs-with-vdevs.nix b/tests/zfs-with-vdevs.nix index 5f8c67d3..dbca4fc9 100644 --- a/tests/zfs-with-vdevs.nix +++ b/tests/zfs-with-vdevs.nix @@ -16,6 +16,15 @@ diskoLib.testLib.makeDiskoTest { out == expected_value ), f"Expected {property}={expected_value} on {ds}, got: {out}" + # These fields are 0 if l2arc is disabled + assert ( + machine.succeed( + "cat /proc/spl/kstat/zfs/arcstats" + " | grep '^l2_' | tr -s ' '" + " | cut -s -d ' ' -f3 | uniq" + ).strip() != "0" + ), "Excepted cache to be utilized." + assert_property("zroot", "compression", "zstd") assert_property("zroot/zfs_fs", "com.sun:auto-snapshot", "true") assert_property("zroot/zfs_fs", "compression", "zstd")