install: Enable installing to multi device parents #1911
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully enables installing to multi-device parent filesystems, such as LVM spanning multiple disks. It correctly discovers all parent devices and, for bootupd/GRUB, installs the bootloader to all devices with an ESP partition. For bootloaders that only support single-device configurations like systemd-boot and zipl, the implementation correctly defaults to using the first available device. The changes are well-architected, adapting data structures and logic to handle multiple devices. A new, thorough integration test validates both single and dual ESP scenarios. Overall, this is a solid enhancement with good error handling and logging. I have one suggestion to further improve the robustness of ESP detection.
| let devpath = dev.path(); | ||
| println!("Installing bootloader via bootupd to {devpath}"); | ||
| Command::new("bootupctl") | ||
| .args(["backend", "install", "--write-uuid"]) |
There was a problem hiding this comment.
I think this will lead to a kind of last-one wins behavior for bootupd.json - but in the end they should be identical I guess?
cc @HuijingHei
We probably want to document the right way to do multi-device installs there. (and have man pages in general)
Alternatively it might be nicer to explicitly support this in bootupd by just passing each device?
There was a problem hiding this comment.
I think this will lead to a kind of last-one wins behavior for
bootupd.json- but in the end they should be identical I guess?
Agree, but we need this like RAID.
Alternatively it might be nicer to explicitly support this in bootupd by just passing each device?
That will be cleaner, and we could do this only if we make bootupd not fail if the passed device does not have the esp device.
ckyrouac
force-pushed
the
multiple-backing-devices
branch
from
f2a175a to
f7b1892
Compare
|
waiting to merge until the patch release goes out |
ckyrouac
force-pushed
the
multiple-backing-devices
branch
3 times, most recently
from
77b65cb to
d03c6fa
Compare
cgwalters
force-pushed
the
multiple-backing-devices
branch
from
d03c6fa to
9b1c313
Compare
ckyrouac
force-pushed
the
multiple-backing-devices
branch
2 times, most recently
from
6802697 to
081f3b2
Compare
When the root filesystem spans multiple backing devices (e.g., LVM across multiple disks), discover all parent devices and find ESP partitions on each. For bootupd/GRUB, install the bootloader to all devices with an ESP partition, enabling boot from any disk in a multi-disk setup. systemd-boot and zipl only support single-device configurations. This adds a new integration test validating both single-ESP and dual-ESP multi-device scenarios. Fixes: bootc-dev#481 Assisted-by: Claude Code (Opus 4.5) Signed-off-by: ckyrouac <[email protected]>
ckyrouac
force-pushed
the
multiple-backing-devices
branch
from
081f3b2 to
9d0e284
Compare
| let esp_part = match esp_parts.len() { | ||
| 0 => { | ||
| anyhow::bail!("Cannot locate ESP: no ESP partition found on any backing device") | ||
| } | ||
| 1 => esp_parts.into_iter().next().unwrap(), | ||
| n => anyhow::bail!( | ||
| "Found {n} ESP partitions across backing devices; only a single ESP is supported for composefs boot" | ||
| ), | ||
| }; |
There was a problem hiding this comment.
We can do this without an unwrap() more like this
let Some(esp_part) = esp_parts.pop() else { bail!("empty") };
if esp_parts.len() > 0 { bail!("multiple"); }
or so
| tracing::warn!( | ||
| "No backing device with ESP found; UEFI boot may fail if ESP cannot be auto-detected" | ||
| ); |
There was a problem hiding this comment.
The more I look at this the more it feels like the right fix here is for us to push multi-device installation down into bootupd.
It's really bootupd that understands e.g. "update payload only has BIOS so that's OK and that's all we should install".
It will need to be bootupd that handles non-EFI platforms too (like android boot).
| /// Contains all devices that have an ESP partition when the root filesystem | ||
| /// spans multiple backing devices (e.g., LVM across multiple disks). | ||
| pub(crate) device_info: Vec<bootc_blockdev::PartitionTable>, |
There was a problem hiding this comment.
Related to above: We can't define this as being filtered to the ESP for e.g. s390x and other non-EFI platforms.
I think this just needs to be all backing partitioned blockdevs, and then we filter it later.
3 participants
When the root filesystem spans multiple backing devices (e.g., LVM across multiple disks), discover all parent devices and find ESP partitions on each. For bootupd/GRUB, install the bootloader to all devices with an ESP partition, enabling boot from any disk in a multi-disk setup. systemd-boot and zipl only support single-device configurations.
This adds a new integration test validating both single-ESP and dual-ESP multi-device scenarios.
Fixes: #481
Assisted-by: Claude Code (Opus 4.5)