From 9b56c1afdb56337b79769edcefbd32218c0181ba Mon Sep 17 00:00:00 2001 From: Michon van Dooren Date: Sun, 17 Nov 2024 15:17:59 +0100 Subject: [PATCH 1/5] Add support for ZFS spare, log & dedup vdevs --- example/zfs-with-vdevs.nix | 73 ++++++++++++++++++++++++++++++++++---- lib/tests.nix | 2 +- lib/types/zpool.nix | 47 ++++++++++++++++++++---- 3 files changed, 109 insertions(+), 13 deletions(-) diff --git a/example/zfs-with-vdevs.nix b/example/zfs-with-vdevs.nix index 2161f5cb..5ee4016a 100644 --- a/example/zfs-with-vdevs.nix +++ b/example/zfs-with-vdevs.nix @@ -43,9 +43,57 @@ }; }; }; - z = { + spare = { type = "disk"; - device = "/dev/sdz"; + device = "/dev/vdc"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + log = { + type = "disk"; + device = "/dev/vdd"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + dedup = { + type = "disk"; + device = "/dev/vde"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + special = { + type = "disk"; + device = "/dev/vdf"; content = { type = "gpt"; partitions = { @@ -61,7 +109,7 @@ }; cache = { type = "disk"; - device = "/dev/vdc"; + device = "/dev/vdg"; content = { type = "gpt"; partitions = { @@ -91,9 +139,22 @@ ]; } ]; - special = { - members = [ "z" ]; - }; + spare = [ "spare" ]; + log = [ + { + members = [ "log" ]; + } + ]; + dedup = [ + { + members = [ "dedup" ]; + } + ]; + special = [ + { + members = [ "special" ]; + } + ]; cache = [ "cache" ]; }; }; diff --git a/lib/tests.nix b/lib/tests.nix index 25b6a593..f8a17514 100644 --- a/lib/tests.nix +++ b/lib/tests.nix @@ -38,7 +38,7 @@ let }; # list of devices generated inside qemu - devices = [ "/dev/vda" "/dev/vdb" "/dev/vdc" "/dev/vdd" "/dev/vde" "/dev/vdf" ]; + devices = [ "/dev/vda" "/dev/vdb" "/dev/vdc" "/dev/vdd" "/dev/vde" "/dev/vdf" "/dev/vdg" "/dev/vdh" ]; # This is the test generator for a disko test makeDiskoTest = diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index 46bab779..453bff12 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -68,11 +68,38 @@ in members = [ "x" "y" "/dev/sda1" ]; }]; }; + spare = lib.mkOption { + type = lib.types.listOf lib.types.str; + default = [ ]; + description = '' + A list of devices to use as hot spares. See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Hot_Spares + for details. + ''; + }; + log = lib.mkOption { + type = lib.types.listOf vdev; + default = [ ]; + description = '' + A list of vdevs used for the zfs intent log (ZIL). See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Intent_Log + for details. + ''; + }; + dedup = lib.mkOption { + type = lib.types.listOf vdev; + default = [ ]; + description = '' + A list of vdevs used for the deduplication table. See + https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#dedup + for details. + ''; + }; special = lib.mkOption { - type = lib.types.nullOr vdev; - default = null; + type = lib.types.either (lib.types.listOf vdev) (lib.types.nullOr vdev); + default = [ ]; description = '' - A vdev definition for a special device. See + A list of vdevs used as special devices. See https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#special for details. ''; @@ -86,7 +113,6 @@ in for details. ''; }; - # TODO: Consider supporting log, spare, and dedup options. }; }); }; @@ -182,8 +208,17 @@ in entries=() ${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.spare != []) + (formatOutput "spare" topology.spare)} + ${lib.optionalString (hasTopology && topology.log != []) + (lib.concatMapStrings (vdev: formatOutput "log ${vdev.mode}" vdev.members) topology.log)} + ${lib.optionalString (hasTopology && topology.dedup != []) + (lib.concatMapStrings (vdev: formatOutput "dedup ${vdev.mode}" vdev.members) topology.dedup)} + ${lib.optionalString (hasTopology && topology.special != null && topology.special != []) + (lib.concatMapStrings + (vdev: formatOutput "special ${vdev.mode}" vdev.members) + (lib.lists.toList topology.special) + )} ${lib.optionalString (hasTopology && topology.cache != []) (formatOutput "cache" topology.cache)} all_devices=() From 6a472cc24872d43524fc248e04c1d63803c342f1 Mon Sep 17 00:00:00 2001 From: Michon van Dooren Date: Sun, 17 Nov 2024 22:11:53 +0100 Subject: [PATCH 2/5] Fix zpool create with > 1 log/dedup/special vdev The type keyword was included before every vdev: ```shell zpool create -f /dev/sda log mirror /dev/sdb /dev/sdc log mirror /dev/sdd /dev/sde ``` but this is incorrect and should instead be: ```shell zpool create -f /dev/sda log mirror /dev/sdb /dev/sdc mirror /dev/sdd /dev/sde ``` --- lib/types/zpool.nix | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index 453bff12..ddad990b 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -160,13 +160,14 @@ in inherit config options; default = let - formatOutput = mode: members: '' - entries+=("${mode}=${ + formatOutput = type: mode: members: '' + entries+=("${type} ${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 = type: vdev: formatOutput type vdev.mode vdev.members; + formatVdevList = type: vdevs: lib.concatMapStrings (formatVdev type) vdevs; hasTopology = !(builtins.isString config.mode); mode = if hasTopology then "prescribed" else config.mode; topology = lib.optionalAttrs hasTopology config.mode.topology; @@ -207,27 +208,31 @@ in else entries=() ${lib.optionalString (hasTopology && topology.vdev != null) - (lib.concatMapStrings formatVdev topology.vdev)} + (formatVdevList "" topology.vdev)} ${lib.optionalString (hasTopology && topology.spare != []) - (formatOutput "spare" topology.spare)} + (formatOutput "spare" "" topology.spare)} ${lib.optionalString (hasTopology && topology.log != []) - (lib.concatMapStrings (vdev: formatOutput "log ${vdev.mode}" vdev.members) topology.log)} + (formatVdevList "log" topology.log)} ${lib.optionalString (hasTopology && topology.dedup != []) - (lib.concatMapStrings (vdev: formatOutput "dedup ${vdev.mode}" vdev.members) topology.dedup)} + (formatVdevList "dedup" topology.dedup)} ${lib.optionalString (hasTopology && topology.special != null && topology.special != []) - (lib.concatMapStrings - (vdev: formatOutput "special ${vdev.mode}" vdev.members) - (lib.lists.toList topology.special) - )} + (formatVdevList "special" (lib.lists.toList topology.special))} ${lib.optionalString (hasTopology && topology.cache != []) - (formatOutput "cache" topology.cache)} + (formatOutput "cache" "" topology.cache)} all_devices=() + last_type= for line in "''${entries[@]}"; do - # lineformat is mode=device1 device2 device3 - mode=''${line%%=*} - devs=''${line#*=} + # lineformat is type mode=device1 device2 device3 + mode="''${line%%=*}" + type="''${mode%% *}" + mode="''${mode#"$type "}" + devs="''${line#*=}" IFS=' ' read -r -a devices <<< "$devs" all_devices+=("''${devices[@]}") + if ! [ "$type" = "$last_type" ]; then + topology+=" $type" + last_type="$type" + fi topology+=" ''${mode} ''${devices[*]}" done # all_devices sorted should equal zfs_devices sorted From d192ffd9449cab0f31a86c3205384c20f45b288e Mon Sep 17 00:00:00 2001 From: Michon van Dooren Date: Sun, 17 Nov 2024 22:17:30 +0100 Subject: [PATCH 3/5] Fix zpool create for disk vdev after mirror vdev A config like ```nix { vdev = [ { mode = "mirror"; members = [ "data1" "data2" ]; } { members = [ "data3" ]; } ]; } ``` would result in the following command: ```shell zpool create -f mirror /dev/data1 /dev/data2 /dev/data3 ``` which would result in a single vdev with a 3-way mirror, rather than a vdev with a 2-way mirror and a second vdev with a single disk. By reordering the vdevs to handle those with an empty mode first we transform this into: ```shell zpool create -f /dev/data3 mirror /dev/data1 /dev/data2 ``` which does have the desired outcome. --- example/zfs-with-vdevs.nix | 156 +++++++++++++++++++++++++++++++++---- lib/tests.nix | 18 ++++- lib/types/zpool.nix | 6 +- tests/zfs-with-vdevs.nix | 35 +++++++++ 4 files changed, 196 insertions(+), 19 deletions(-) diff --git a/example/zfs-with-vdevs.nix b/example/zfs-with-vdevs.nix index 5ee4016a..985e082c 100644 --- a/example/zfs-with-vdevs.nix +++ b/example/zfs-with-vdevs.nix @@ -1,9 +1,9 @@ { disko.devices = { disk = { - x = { + data1 = { type = "disk"; - device = "/dev/sdx"; + device = "/dev/vda"; content = { type = "gpt"; partitions = { @@ -27,9 +27,9 @@ }; }; }; - y = { + data2 = { type = "disk"; - device = "/dev/sdy"; + device = "/dev/vdb"; content = { type = "gpt"; partitions = { @@ -43,7 +43,7 @@ }; }; }; - spare = { + data3 = { type = "disk"; device = "/dev/vdc"; content = { @@ -59,7 +59,7 @@ }; }; }; - log = { + spare = { type = "disk"; device = "/dev/vdd"; content = { @@ -75,7 +75,7 @@ }; }; }; - dedup = { + log1 = { type = "disk"; device = "/dev/vde"; content = { @@ -91,7 +91,7 @@ }; }; }; - special = { + log2 = { type = "disk"; device = "/dev/vdf"; content = { @@ -107,7 +107,7 @@ }; }; }; - cache = { + log3 = { type = "disk"; device = "/dev/vdg"; content = { @@ -123,6 +123,118 @@ }; }; }; + dedup1 = { + type = "disk"; + device = "/dev/vdh"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + dedup2 = { + type = "disk"; + device = "/dev/vdi"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + dedup3 = { + type = "disk"; + device = "/dev/vdj"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + special1 = { + type = "disk"; + device = "/dev/vdk"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + special2 = { + type = "disk"; + device = "/dev/vdl"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + special3 = { + type = "disk"; + device = "/dev/vdm"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; + cache = { + type = "disk"; + device = "/dev/vdn"; + content = { + type = "gpt"; + partitions = { + zfs = { + size = "100%"; + content = { + type = "zfs"; + pool = "zroot"; + }; + }; + }; + }; + }; }; zpool = { zroot = { @@ -133,26 +245,38 @@ vdev = [ { mode = "mirror"; - members = [ - "x" - "y" - ]; + members = [ "data1" "data2" ]; + } + { + members = [ "data3" ]; } ]; spare = [ "spare" ]; log = [ { - members = [ "log" ]; + mode = "mirror"; + members = [ "log1" "log2" ]; + } + { + members = [ "log3" ]; } ]; dedup = [ { - members = [ "dedup" ]; + mode = "mirror"; + members = [ "dedup1" "dedup2" ]; + } + { + members = [ "dedup3" ]; } ]; special = [ { - members = [ "special" ]; + mode = "mirror"; + members = [ "special1" "special2" ]; + } + { + members = [ "special3" ]; } ]; cache = [ "cache" ]; diff --git a/lib/tests.nix b/lib/tests.nix index f8a17514..2d7fcd3b 100644 --- a/lib/tests.nix +++ b/lib/tests.nix @@ -38,7 +38,23 @@ let }; # list of devices generated inside qemu - devices = [ "/dev/vda" "/dev/vdb" "/dev/vdc" "/dev/vdd" "/dev/vde" "/dev/vdf" "/dev/vdg" "/dev/vdh" ]; + devices = [ + "/dev/vda" + "/dev/vdb" + "/dev/vdc" + "/dev/vdd" + "/dev/vde" + "/dev/vdf" + "/dev/vdg" + "/dev/vdh" + "/dev/vdi" + "/dev/vdj" + "/dev/vdk" + "/dev/vdl" + "/dev/vdm" + "/dev/vdn" + "/dev/vdo" + ]; # This is the test generator for a disko test makeDiskoTest = diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index ddad990b..5aed2039 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -1,6 +1,6 @@ { config, options, lib, diskoLib, rootMountPoint, ... }: let - # TODO: Consider expanding to handle `disk` `file` and `draid` mode options. + # TODO: Consider expanding to handle `file` and `draid` mode options. modeOptions = [ "" "mirror" @@ -167,7 +167,9 @@ in }") ''; formatVdev = type: vdev: formatOutput type vdev.mode vdev.members; - formatVdevList = type: vdevs: lib.concatMapStrings (formatVdev type) vdevs; + formatVdevList = type: vdevs: lib.concatMapStrings + (formatVdev type) + (builtins.sort (a: b: a.mode < b.mode) vdevs); hasTopology = !(builtins.isString config.mode); mode = if hasTopology then "prescribed" else config.mode; topology = lib.optionalAttrs hasTopology config.mode.topology; diff --git a/tests/zfs-with-vdevs.nix b/tests/zfs-with-vdevs.nix index da301a01..704a1733 100644 --- a/tests/zfs-with-vdevs.nix +++ b/tests/zfs-with-vdevs.nix @@ -38,5 +38,40 @@ diskoLib.testLib.makeDiskoTest { assert_property("zroot/zfs_fs", "com.sun:auto-snapshot", "true") assert_property("zroot/zfs_fs", "compression", "zstd") machine.succeed("mountpoint /zfs_fs"); + + # Take the status output and flatten it so that each device is on a single line prefixed with with the group (either + # the pool name or a designation like log/cache/spare/dedup/special) and first portion of the vdev name (empty for a + # disk from a single vdev, mirror for devices in a mirror. This makes it easy to verify that the layout is as + # expected. + group = "" + vdev = "" + actual = [] + for line in machine.succeed("zpool status -P zroot").split("\n"): + first_word = line.strip().split(" ", 1)[0] + if line.startswith("\t ") and first_word.startswith("/"): + actual.append(f"{group}{vdev}{first_word}") + elif line.startswith("\t "): + vdev = f"{first_word.split('-', 1)[0]} " + elif line.startswith("\t"): + group = f"{first_word} " + vdev = "" + actual.sort() + expected=sorted([ + 'zroot /dev/disk/by-partlabel/disk-data3-zfs', + 'zroot mirror /dev/disk/by-partlabel/disk-data1-zfs', + 'zroot mirror /dev/disk/by-partlabel/disk-data2-zfs', + 'dedup /dev/disk/by-partlabel/disk-dedup3-zfs', + 'dedup mirror /dev/disk/by-partlabel/disk-dedup1-zfs', + 'dedup mirror /dev/disk/by-partlabel/disk-dedup2-zfs', + 'special /dev/disk/by-partlabel/disk-special3-zfs', + 'special mirror /dev/disk/by-partlabel/disk-special1-zfs', + 'special mirror /dev/disk/by-partlabel/disk-special2-zfs', + 'logs /dev/disk/by-partlabel/disk-log3-zfs', + 'logs mirror /dev/disk/by-partlabel/disk-log1-zfs', + 'logs mirror /dev/disk/by-partlabel/disk-log2-zfs', + 'cache /dev/disk/by-partlabel/disk-cache-zfs', + 'spares /dev/disk/by-partlabel/disk-spare-zfs', + ]) + assert actual == expected, f"Incorrect pool layout. Expected:\n\t{'\n\t'.join(expected)}\nActual:\n\t{'\n\t'.join(actual)}" ''; } From caad548415edb0cfe20d29a363aa98137fd7cc2b Mon Sep 17 00:00:00 2001 From: Michon van Dooren Date: Mon, 18 Nov 2024 22:49:51 +0100 Subject: [PATCH 4/5] Make vdev sort only move empty modes to the start --- 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 5aed2039..12b9ccac 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -169,7 +169,7 @@ in formatVdev = type: vdev: formatOutput type vdev.mode vdev.members; formatVdevList = type: vdevs: lib.concatMapStrings (formatVdev type) - (builtins.sort (a: b: a.mode < b.mode) vdevs); + (builtins.sort (a: _: a.mode == "") vdevs); hasTopology = !(builtins.isString config.mode); mode = if hasTopology then "prescribed" else config.mode; topology = lib.optionalAttrs hasTopology config.mode.topology; From 92ad678418917a6826f9eabb34995b2b8e58229e Mon Sep 17 00:00:00 2001 From: Michon van Dooren Date: Mon, 18 Nov 2024 22:50:10 +0100 Subject: [PATCH 5/5] Add examples for all special zpool vdev types --- lib/types/zpool.nix | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/lib/types/zpool.nix b/lib/types/zpool.nix index 12b9ccac..c36acbfa 100644 --- a/lib/types/zpool.nix +++ b/lib/types/zpool.nix @@ -63,10 +63,15 @@ in 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" ]; - }]; + example = [ + { + mode = "mirror"; + members = [ "x" "y" ]; + } + { + members = [ "z" ]; + } + ]; }; spare = lib.mkOption { type = lib.types.listOf lib.types.str; @@ -76,6 +81,7 @@ in https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Hot_Spares for details. ''; + example = [ "x" "y" ]; }; log = lib.mkOption { type = lib.types.listOf vdev; @@ -85,6 +91,15 @@ in https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Intent_Log for details. ''; + example = [ + { + mode = "mirror"; + members = [ "x" "y" ]; + } + { + members = [ "z" ]; + } + ]; }; dedup = lib.mkOption { type = lib.types.listOf vdev; @@ -94,6 +109,15 @@ in https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#dedup for details. ''; + example = [ + { + mode = "mirror"; + members = [ "x" "y" ]; + } + { + members = [ "z" ]; + } + ]; }; special = lib.mkOption { type = lib.types.either (lib.types.listOf vdev) (lib.types.nullOr vdev); @@ -103,6 +127,15 @@ in https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#special for details. ''; + example = [ + { + mode = "mirror"; + members = [ "x" "y" ]; + } + { + members = [ "z" ]; + } + ]; }; cache = lib.mkOption { type = lib.types.listOf lib.types.str; @@ -112,6 +145,7 @@ in https://openzfs.github.io/openzfs-docs/man/master/7/zpoolconcepts.7.html#Cache_Devices for details. ''; + example = [ "x" "y" ]; }; }; });