Allow swap size and partition UUIDs to be specified in make-disk-image

[numinit]

Motivation for this change

Specifying swap space in image builds is potentially useful, especially if we support ZFS in make-disk-image.nix someday, since ZVOLs cause system deadlocks if they are used as swap devices:

https://github.com/zfsonlinux/pkg-zfs/wiki/HOWTO-use-a-zvol-as-a-swap-device

Along the way, add hardcoded UUIDs for the MBR and boot/swap/root partitions to match nixos/modules/installer/sd-card/sd-image.nix. This probably isn't perfect, but makes the behavior a little closer, which is good for reproducibility.

Add tests in image-contents.nix.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[numinit]

@ofborg eval

[samueldr]

❗ ❌ This is a soft "think about the consequences" type of thing. make-disk-image is, in my opinion, a terrible implementation of the desired results. And we probably shouldn't expand on it anymore. It already is doing too much, in brittle ways.

Furthermore, it's one of the (IIRC) four different implementations of disk image and filesystem image building in Nixpkgs. And I think it's probably the worst of the bunch.

I don't think adding even more complexity here is a good thing. This is a public interface, which is being used elsewhere, and the more we add to it, the more we need to continue maintaining.

Note that I want us to have a proper filesystem/disk image building tooling!! And I'm not alone! See #23052 for someone else thinking about the issue.

I might have to have a go at finishing up my proposal for the interface / implementation of what I believe to be the proper way to go. A sneak peek can be seen here. The main thing missing is to have the option to make either the steps be their distinct derivations (current state) or create a "big builder script". The latter being required when building disk images on e.g. hydra, where distinct derivations are limited in size.

So uh... I really don't know what to make of this PR :/. The only reason I fixed-up the correctness issue with auto-sizing was because the feature had been merged.

[numinit]

Yeah that's fair. I wasn't sure how much we wanted to add or if duplicating from sd-image.nix was even desirable ¯\_(ツ)_/¯. It was more an "at least that's useful to me" sort of thing that I figured I'd try and PR in case it's useful to others...

Some of the general approaches may be useful at least?

[numinit]

I guess this ended up being another case of "following the nixpkgs status quo is actually a bad idea here" too 😅. Seen that in some other areas too (e.g. modules), the RFC process has helped me crib off of some better examples.

[samueldr]

Yeah, sorry, I didn't mean to dump this on you, but relate how we might want to approach this with caution. Mainly for other contributors, reviewers, maintainers and commiters, than for you.

To be fair I haven't looked deeply into how things are done here. Mainly at how it was changing the public-facing interface a lot.

[numinit]

No problem, totally get it. I've always found the community pretty professional in handling "well, the current way has issues" sort of problems. And part of it is probably that I should actually sit in IRC too since bouncing ideas will help making the most effective dent in improving things anyway... :-)

One of the weird things I see though is that the old way actually gets used to hack on new features - I noticed that #106574 used something derived from make-disk-image. I think we're caught in a weird spot with having awesome tooling that lots of people want to improve but also needing to deal with cutting down tech debt and crufty interfaces like this one. But, yeah, maybe this is just a sign, as you may have implied, that we just need to RFC a "unified" way to approach making things like make-sd-image and make-disk-image cooperate.

[samueldr]

Hahaha, yes. @grahamc is aware of the situation. And IIRC we discussed it some when he first introduced the PR and started working on it.

Though maybe not something for an RFC. Let me quote from today's high level discussion:

• https://logs.nix.samueldr.com/nixos-dev/2021-05-02#1619989820-1619991132;

[numinit]

Oh yeah, CFPs seem like a pretty good idea. If only as a grievance usecase airing board about alternatives to the old way. It's funny though, just based on how many people use make-disk-image (all of nixos-generators, for one...) - the whole situation reminds me of this. Like, do we keep propping up the stack of cards? Everyone uses it...

One thing I considered was just... what if we turned make-disk-image into a shim for the better way once it's out? Not great for the API cruft, but it would at least provide a deprecation path.

I'll get on the IRC eventually, need to find the right box to run my client on 😛

[samueldr]

Part of what I had in mind, before the idea of a CFP, was to keep the existing interfaces working at least for a while with warnings. And for the CFP, that would be a requirement: implementing the existing interface for backwards compatibility. This requirement should help ensure the "one true" implementation is flexible enough for at least all of Nixpkgs' needs.

[numinit]

Yeah I like that. Seems like a decent way to bridge the gap between making forward progress requiring touching some of the more "scary" parts of nixpkgs.

[Lassulus]

The code looks nice. I'm mostly with samueldr here and are more general image builder or something along those line would maybe be preferable. I have no idea how to continue from here, more options means more stuff we will have to support once we deprecate/rewrite this, but I also want progress. Would it be better to start from scratch and port stuff to the new image builders? it was quite hard to get any reviews for my changes for this file. I really dislike it being as old and weird as it is, but maybe changes should be made iterative (like in this PR).

[samueldr]

but maybe changes should be made iterative (like in this PR).

If the abstractions given were okay, it would be the approach I would prefer.

Though the abstractions already were wrong, and only partially fixed in the recent auto-sizing. The main issue being that there is a lack of distinction between the partition (filesystem image) and the disk image. There is also a lack of separation of issues with the boot scheme and the partition scheme.

An example of how it matters:

• Using diskSize determines the overall disk size.
• With a scheme where there is only the main partition, this generally means that the disk size is the filesystem size.
• With a scheme where there is a boot partition ("gpt" or "hybrid" [or "legacy+gpt"!!]) the fileSystem size is approximately diskSize - bootSize.

This means that changing the boot scheme changes the filesystem size, which could result in surprises in hand-crafted "tight" images.

Then when comes automatic size, this is flipped on its head, and the filesystem change is static! This means that if the total disk image size matters, it may fail in unexpected manners.

This is my main issue with make-disk-image, the implementation details are as it is, not great, but details. The interface is wrong at a fundamental level.

[numinit]

The main issue being that there is a lack of distinction between the partition (filesystem image) and the disk image. There is also a lack of separation of issues with the boot scheme and the partition scheme.

Agreed, but... in practice, I think it's okay to be a little opinionated here by optimizing for the common usecase. Which, by my perception, is making an image that's burnt to a flash drive or SD card or cloud disk, and letting the automatic expansion of the last partition handle the discrepancy. If someone just wants to burn a custom built NixOS image to a disk with dd rather than going through the trouble of using the ISO installer, it should mainly just work. Having a helper that lets you set the boot/swap partition sizes and just letting the root expand to fill the medium accomplishes this goal pretty well.

I think there's definite value in the distinction between the disk and individual partitions, but how this interface ends up being used pretty often is someone asking to build a disk image that's going to be smaller than the disk. And, while the interface is crufty and inflexible and pretty incorrect, the end goal ends up being accomplished pretty nicely for, e.g., nixos-generators.

And yeah, the basic idea was an iterative but also backwards compatible change as @Lassulus suggests. Even though I wasn't sure about the interface to begin with.

[stale]

I marked this as stale due to inactivity. → More info