Repository navigation
repart: Let systemd-repart size the generated root partition - #2532
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. |
| #[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?
There was a problem hiding this comment.
With this change it becomes dead code, which CI won't allow. As far as I can see it came in with the root size calculation in 16d9af2, not for c9s. The c9s workaround is the default on partno, and that's unchanged.
a6966f8 to
5f0864c
Compare
we have test for this exact case here https://github.com/bootc-dev/bootc/blob/main/tmt/tests/booted/test-install-repart.nu#L193 This wasn't caught since we don't run integration tests for F46 |
|
@Johan-Liebert1 I am thinking we may need to make repart.d opt-in like |
|
Yup, that sounds good to me |
|
Done here #2551 |
5f0864c to
807f470
Compare
18d8ba9 to
7927c6f
Compare
| }; | ||
| write_repart_file(generated.path(), REPART_GENERATED_ROOT_NAME, &root_conf)?; | ||
| for (name, size) in &boot_partition_sizes { | ||
| let dropin = format!("{name}.d/{REPART_PLANNED_SIZE_DROPIN}"); |
There was a problem hiding this comment.
Why pin the sizes? This won't be visible to repart on the first boot anyway as we're only doing this temporarily
There was a problem hiding this comment.
They're for the install run itself, not for first boot. With a generic image that run only creates root, the ESP and BIOS boot (--include-partitions), so a growable ESP or BIOS boot definition would take the space the plan leaves for the partitions created on first boot, and first boot then can't create them. The drop-ins make the install create them at their planned size; on first boot they already exist and keep it. The growable-ESP case in the tmt test fails without them.
`bootc install to-disk` only partitions with systemd-repart when given --run-repart, but bootc-installation(7) still describes that as automatic whenever the image has repart.d definitions. Generated-by: AI Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
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= after the standard directories, and name it 50-root-<UUID>.conf. The random part keeps it from masking a definition of the image named 50-root.conf, or from being masked by one, such as a mask of a base image's root definition; the prefix keeps root where it was among the partitions, which systemd-repart lays out in the order of their definitions' names. On systemd < 253, which lacks --include-partitions=, a generic image's install reads only copies of the definitions it needs; put those in such a directory too, instead of the fixed /tmp/repart.d. With every caller passing a fresh directory, collect_repart_definitions no longer clears its target. Prep for the next commit, which generates more definitions. 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. Unless --root-size is given, leave the generated root without size limits, so that systemd-repart sizes it like an unsized root of the image, sharing free space by Weight=. A generic image's install creates only root, the ESP and BIOS boot, and leaves the other partitions to first boot, so in that case plan the layout with all definitions in a second dry run and pin those three to their planned sizes, root through its definition and the others with drop-ins, keeping the space planned for the rest. With --root-size, nothing is planned or pinned: the plan would need root and all other partitions to fit the install disk, which breaks generic images meant to be grown when deployed. Generated-by: AI Signed-off-by: Christian Glombek <c.glombek@cosa.systems>
7927c6f to
59e316b
Compare
Johan-Liebert1
left a comment
Johan-Liebert1
left a comment
There was a problem hiding this comment.
Looks good on surface. Haven't tested locally though
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.Unless
--root-sizeis given, the generated root now has no size limits, so systemd-repart sizes it like an unsized root of the image, sharing free space byWeight=. A generic image's install creates only root, the ESP and BIOS boot, and leaves the other partitions to first boot, so in that case bootc plans the layout with all definitions in a second dry run and pins those three to their planned sizes, root through its definition and the others with drop-ins, keeping the space planned for the rest. With--root-size, nothing is planned or pinned: the plan would need root and all other partitions to fit the install disk, which breaks generic images meant to be grown when deployed.Two smaller commits come first:
--run-repart.--definitions=after the standard directories, instead of into/run/repart.d, which may be the host's into-disk. The generated root is named50-root-<UUID>.conf, so it can neither mask a definition of the image nor be masked by one, and still lands where50-root.confdid among the partitions. The copies a generic image's install reads on systemd < 253 go to such a directory too, instead of the fixed/tmp/repart.d.Testing
run-repartoption #2551, it installs with--run-repart.--root-size, and a simulated first boot after generic ones. Without--root-size, every set installs, and first boot creates every deferred partition and leaves root unchanged. With--root-size, a generic install behaves as before. Non-generic installs are only covered by these replays; every tmt install uses--via-loopback, which implies--generic-image.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.