Would any of you have any opinion on which of these two alternatives are better/safer/preferable?

{
  _file = ./auto-create.nix;
  flake.modules.nixos."nas-2024" =
    { config, lib, ... }:
    let
      # https://discourse.nixos.org/t/configure-zfs-filesystems-after-install/48633/2
      zfsDatasets =
        config.disko.devices.zpool
        |> lib.attrsToList
        |> lib.foldl' (
          acc: zpool:
          acc
          ++ (
            zpool.value.datasets
            |> lib.attrValues
            |> lib.filter (dataset: dataset.name != "__root")
            |> lib.map (dataset: {
              zpool = zpool.name;
              inherit (dataset) name mountpoint;
              creationScript = dataset._create;
            })
          )
        ) [ ];
      diskoFormatScript = lib.getExe config.system.build.formatScript;
    in
    {
      # Perform the "create new datasets/zvols" operation
      systemd.services = {
        "zfs-import-tank".preStart = diskoFormatScript;
        # "zfs-import-nvmepool".preStart = diskoFormatScript; # Don't need this yet
      }
      // (
        zfsDatasets
        |> lib.filter (dataset: dataset.name == "doesn't exist") # Disable for now
        |> lib.map (
          dataset:
          lib.nameValuePair "zfs-create-${dataset.zpool}-${lib.replaceString "/" "_" dataset.name}" {
            unitConfig.DefaultDependencies = false;
            requiredBy = [ "local-fs.target" ];
            before = [ "local-fs.target" ];
            after = [
              "zfs-import-${dataset.zpool}.service"
              "zfs-mount.service"
            ];
            unitConfig.ConditionPathIsMountPoint = lib.mkIf (
              dataset.mountpoint != null
            ) "!${dataset.mountpoint}";
            script = dataset.creationScript;
          }
        )
        |> lib.listToAttrs
      );
    };
}

In my opinion Disko would benefit from natively supporting this kind of thing---I have run into this too, and when tweaking my dataset properties it is a pain to try to keep them in sync with my Disko config.

One idea is to have a boolean dataset attribute datasets.<name>.followConfig or similar. If this is set, then on activation (and boot) Disko will try to set the dataset properties to match the config.

Balancing idempotency with the desire to not lose data accidentally is tricky---if you delete a dataset from your config, should Disko delete the dataset from your pool? One idea is to use a custom ZFS user property to identify datasets which are being managed by Disko; this would allow deleting dataset which no longer appear in yoru config, while keeping the ability for people to manually create unmanaged datasets

In my opinion Disko would benefit from natively supporting this kind of thing---I have run into this too, and when tweaking my dataset properties it is a pain to try to keep them in sync with my Disko config.

One idea is to have a boolean dataset attribute datasets.<name>.followConfig or similar. If this is set, then on activation (and boot) Disko will try to set the dataset properties to match the config.

Balancing idempotency with the desire to not lose data accidentally is tricky---if you delete a dataset from your config, should Disko delete the dataset from your pool? One idea is to use a custom ZFS user property to identify datasets which are being managed by Disko; this would allow deleting datasets which no longer appear in your config, while keeping the ability for people to manually create unmanaged datasets

In my opinion Disko would benefit from natively supporting this kind of thing---I have run into this too, and when tweaking my dataset properties it is a pain to try to keep them in sync with my Disko config.

One idea is to have a boolean dataset attribute datasets.<name>.managed or similar. If this is set, then on activation (and boot) Disko will try to set the dataset properties to match the config.

Balancing idempotency with the desire to not lose data accidentally is tricky---if you delete a dataset from your config, should Disko delete the dataset from your pool? One idea is to use a custom ZFS user property to identify datasets which are being managed by Disko; this would allow deleting datasets which no longer appear in your config, while keeping the ability for people to manually create unmanaged datasets