Should virtualisation.disksize be nullable?

[Bert-Proesmans]

Describe the bug

Setting nixos option virtualisation.disksize to null, which is legal according to typechecks https://github.com/NixOS/nixpkgs/blob/9e2274928b2467b604cdee4d8831232f097ea7ba/nixos/modules/virtualisation/qemu-vm.nix#L355-L357

results in an invalid disk size value passed to qemu-img create

https://github.com/NixOS/nixpkgs/blob/9e2274928b2467b604cdee4d8831232f097ea7ba/nixos/modules/virtualisation/qemu-vm.nix#L124

Launching the script, aka system build attribute config.system.build.vm, will throw an error at the disk creation stage mentioning an invalid argument for the size parameter.

Disk image do not exist, creating the virtualisation disk image...
qemu-img: Invalid image size specified. You may use k, M, G, T, P or E suffixes for
qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes.

Steps To Reproduce

Steps to reproduce the behavior:

1. Initialize new flake
2. Create output attribute machine with value of basic nixosSystem
3. Add option virtualisation.diskSize = null;
4. Build attribute .#machine.config.system.build.vm
5. Run ./result

Expected behavior

Launching ./result will create a qcow disk and launch qemu with the machine configuration.

Additional context

I found this bug through changes in nixos-generators. I've created an issue in their repo first, but their changes might be technically correct. REF; https://github.com/nix-community/nixos-generators/issues/306

Notify maintainers

No maintainers list within that nixos module file. Pinging @rnhmjoj as last change author on the type of virtualisation.disksize.

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 6.1.47, NixOS, 23.11 (Tapir), 23.11.20230828.48516a8`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.17.0`
 - nixpkgs: `/nix/store/9y932x095yqpr33blix323r3ybn75c27-source`

---

Add a :+1: reaction to issues you find important.

[rnhmjoj]

I don't remember why I've set this option to nullOr ints.positive. From the commit that added it (https://github.com/NixOS/nixpkgs/commit/310eefffe7a3fbd8054eb79f3a6569c4d0635112) it's clear that null is not handled, so it's probably a mistake.

[phaer]

Just stumbled upon this issue and agree with the analysis here.

PR at https://github.com/NixOS/nixpkgs/pull/338967