repart: Let systemd-repart size the generated root partition - #2532
LorbusChris wants to merge 3 commits into
Conversation
github-actions
Bot
added
area/install
4af0967 to
a6966f8
Compare
|
Thank you for testing bootc's development stream! Obviously #2314 just landed and this issue perhaps argues for disabling it by default for at least a cycle...
Thanks. I thought we covered this, but now that I re-read #2314 we didn't add tests for this. |
| const REPART_FILTERED_DEFINITIONS_DIR: &str = "/tmp/repart.d"; | ||
|
|
||
| /// The repart.d directory for definitions generated at runtime. | ||
| const REPART_RUNTIME_DIR: &str = "/run/repart.d"; |
There was a problem hiding this comment.
We should actually be careful here, I think in to-disk we may end up using the host's /run and we really absolutely should not touch any global state in /run (what if the host OS is using /run/repart.d)
I think the simplest thing to do here is for us to create a tempdir and overmount that dir if it exists - we're always in a mountns.
It's possible we could use --definitions to override, but that feels like it would involve a good bit more changes here.
| #[serde(default)] | ||
| raw_size: u64, | ||
| #[serde(default)] | ||
| raw_padding: u64, |
There was a problem hiding this comment.
Didn't we add this for something like c9s compatibility? Why are you dropping it?
| match root.raw_size / MIB { | ||
| 0 => anyhow::bail!("systemd-repart planned less than 1 MiB for the root partition"), | ||
| size_mib => Ok(size_mib), | ||
| } |
There was a problem hiding this comment.
More elegant to use NonZeroU64
| let name = Path::new(&p.file) | ||
| .file_name() | ||
| .and_then(OsStr::to_str) | ||
| .ok_or_else(|| anyhow::anyhow!("Invalid repart.d definition path {}", p.file))?; |
There was a problem hiding this comment.
pre-existing but if we require utf8 we should probably make p.file a Utf8PathBuf or so.
Here you could try converting to Utf8Path (borrowed) then get the anme from that
When the image's repart.d definitions have no root partition, bootc wrote one to /run/repart.d/50-root.conf for systemd-repart. In `bootc install to-disk`, /run may be the host's, where bootc must not change any state. The file was also left behind, so a later install from the same environment took it for one of the image's definitions and reused the previous root size. Write it to a temporary directory instead, which systemd-repart reads through --definitions= along with the standard directories, with the priority of /run/repart.d. Prep for the next commits, which generate more definitions. Generated-by: AI Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
bootc only generates its root definition, 50-root.conf, when the image's repart.d definitions have no root partition, so a definition of the image with that name is for some other partition. systemd-repart reads only one of two same-named definitions: the generated one, read with the priority of /run/repart.d, masks one in /run/repart.d, /usr/local/lib/repart.d or /usr/lib/repart.d, whose partition then silently goes missing from the layout, and one in /etc/repart.d masks it. Fail with an error that names the definition instead. Generated-by: AI Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
With systemd 262, `bootc install to-disk` fails for images whose repart.d definitions have no root partition, unless --root-size is given: systemd-repart now reports the free space as padding of the last partition (systemd 959c3286d00f), and the root size bootc derives from its dry run underflows. Plan root with a second dry run of all definitions plus an unsized root definition, and pin it to the size systemd-repart plans for it. Root then shares free space by Weight= with any other definition that can grow. A generic image's install creates only root, the ESP and BIOS boot, so pin the latter two to their planned sizes as well, with drop-ins, keeping the space planned for the partitions created on first boot. Generated-by: AI Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
a6966f8 to
5f0864c
Compare
This branch has not been deployed
2 participants
With systemd 262,
bootc install to-diskfails for images whose repart.d definitions have no root partition, unless--root-sizeis given: systemd-repart now reports the free space as padding of the last partition (systemd 959c3286d00f), and the root size bootc derives from its dry run underflows.This plans root with a second dry run of all definitions plus an unsized root definition, and pins it to the size systemd-repart plans for it. Root then shares free space by
Weight=with any other definition that can grow. A generic image's install creates only root, the ESP and BIOS boot, so the latter two are pinned to their planned sizes as well, with drop-ins, keeping the space planned for the partitions created on first boot.Two prep commits come first. Generated definitions now go to a temporary directory that systemd-repart reads through
--definitions=, with the priority of/run/repart.d, instead of into/run/repart.ditself, which may be the host's into-disk. An image definition named50-root.conf, which would mask the generated one or be masked by it, is refused.Testing
/run/repart.duntouched.make validatepasses, and each commit builds and passes the lib unit tests.plan-52-install-repartfails on the Fedora 45 and rawhide legs of composefs: Install systemd-boot via bootupd to keep shim in boot path #2512 without this.Generated-by: AI
I am familiar with the bootc concepts and have prior experience building atomic OS images. I am however not familiar with this codebase and used AI to generate these code changes.