Skip to content

Commit 1ad3542

Browse files
committed
install: Allow mounted directories during install to-filesystem
When performing a to-filesystem installation, the target directory may contain pre-existing mount points for directories like /var, /var/lib/containers, etc. These are legitimate in hybrid/existing filesystem scenarios where certain directories are on separate partitions. This change enhances the empty rootdir check to: - Recursively detect directories that contain only mount points - Skip directories that are themselves mount points - Allow installation to proceed when mount hierarchies exist (e.g., /var containing /var/lib which contains mounted /var/lib/containers) Also adds integration test coverage for separate /var mount scenarios using LVM. Assisted-by: Claude Code (Sonnet 4.5) Signed-off-by: ckyrouac <[email protected]>
1 parent 535f1e7 commit 1ad3542

File tree

2 files changed

+283
-11
lines changed

2 files changed

+283
-11
lines changed

crates/lib/src/install.rs

Lines changed: 146 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,6 +1803,50 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
18031803
Ok(())
18041804
}
18051805

1806+
/// Require that a directory contains only mount points (or is empty), recursively.
1807+
/// Returns Ok(()) if all entries in the directory tree are either:
1808+
/// - Mount points (on different filesystems)
1809+
/// - Directories that themselves contain only mount points (recursively)
1810+
/// - The lost+found directory
1811+
///
1812+
/// Returns an error if any non-mount entry is found.
1813+
///
1814+
/// This handles cases like /var containing /var/lib (not a mount) which contains
1815+
/// /var/lib/containers (a mount point).
1816+
#[context("Requiring directory contains only mount points")]
1817+
fn require_dir_contains_only_mounts(parent_fd: &Dir, dir_name: &str) -> Result<()> {
1818+
let Some(dir_fd) = parent_fd.open_dir_noxdev(dir_name)? else {
1819+
// The directory itself is a mount point
1820+
return Ok(());
1821+
};
1822+
1823+
if dir_fd.entries()?.next().is_none() {
1824+
anyhow::bail!("Found empty directory: {dir_name}");
1825+
}
1826+
1827+
for entry in dir_fd.entries()? {
1828+
let entry = DirEntryUtf8::from_cap_std(entry?);
1829+
let entry_name = entry.file_name()?;
1830+
1831+
if entry_name == LOST_AND_FOUND {
1832+
continue;
1833+
}
1834+
1835+
if dir_name == BOOT && entry_name == crate::bootloader::EFI_DIR {
1836+
continue;
1837+
}
1838+
1839+
let etype = entry.file_type()?;
1840+
if etype == FileType::dir() && dir_fd.open_dir_noxdev(&entry_name)?.is_some() {
1841+
require_dir_contains_only_mounts(&dir_fd, &entry_name)?;
1842+
} else {
1843+
anyhow::bail!("Found entry in {dir_name}: {entry_name}");
1844+
}
1845+
}
1846+
1847+
Ok(())
1848+
}
1849+
18061850
#[context("Verifying empty rootfs")]
18071851
fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> {
18081852
for e in rootfs_fd.entries()? {
@@ -1811,17 +1855,11 @@ fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> {
18111855
if name == LOST_AND_FOUND {
18121856
continue;
18131857
}
1814-
// There must be a boot directory (that is empty)
1815-
if name == BOOT {
1816-
let mut entries = rootfs_fd.read_dir(BOOT)?;
1817-
if let Some(e) = entries.next() {
1818-
let e = DirEntryUtf8::from_cap_std(e?);
1819-
let name = e.file_name()?;
1820-
if matches!(name.as_str(), LOST_AND_FOUND | crate::bootloader::EFI_DIR) {
1821-
continue;
1822-
}
1823-
anyhow::bail!("Non-empty boot directory, found {name}");
1824-
}
1858+
1859+
// Check if this entry is a directory
1860+
let etype = e.file_type()?;
1861+
if etype == FileType::dir() {
1862+
require_dir_contains_only_mounts(rootfs_fd, &name)?;
18251863
} else {
18261864
anyhow::bail!("Non-empty root filesystem; found {name:?}");
18271865
}
@@ -2573,4 +2611,101 @@ UUID=boot-uuid /boot ext4 defaults 0 0
25732611

25742612
Ok(())
25752613
}
2614+
2615+
#[test]
2616+
fn test_require_dir_contains_only_mounts() -> Result<()> {
2617+
// Test 1: Empty directory should fail (not a mount point)
2618+
{
2619+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2620+
td.create_dir("empty")?;
2621+
assert!(require_dir_contains_only_mounts(&td, "empty").is_err());
2622+
}
2623+
2624+
// Test 2: Directory with only lost+found should succeed (lost+found is ignored)
2625+
{
2626+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2627+
td.create_dir_all("var/lost+found")?;
2628+
assert!(require_dir_contains_only_mounts(&td, "var").is_ok());
2629+
}
2630+
2631+
// Test 3: Directory with a regular file should fail
2632+
{
2633+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2634+
td.create_dir("var")?;
2635+
td.write("var/test.txt", b"content")?;
2636+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2637+
}
2638+
2639+
// Test 4: Nested directory structure with a file should fail
2640+
{
2641+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2642+
td.create_dir_all("var/lib/containers")?;
2643+
td.write("var/lib/containers/storage.db", b"data")?;
2644+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2645+
}
2646+
2647+
// Test 5: boot directory with efi subdirectory should succeed (efi is allowed in boot)
2648+
{
2649+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2650+
td.create_dir_all("boot/efi")?;
2651+
assert!(require_dir_contains_only_mounts(&td, "boot").is_ok());
2652+
}
2653+
2654+
// Test 6: boot directory with both efi and lost+found should succeed (both are allowed)
2655+
{
2656+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2657+
td.create_dir_all("boot/efi")?;
2658+
td.create_dir_all("boot/lost+found")?;
2659+
assert!(require_dir_contains_only_mounts(&td, "boot").is_ok());
2660+
}
2661+
2662+
// Test 7: boot directory with grub should fail (grub2 is not a mount and contains files)
2663+
{
2664+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2665+
td.create_dir_all("boot/grub2")?;
2666+
td.write("boot/grub2/grub.cfg", b"config")?;
2667+
assert!(require_dir_contains_only_mounts(&td, "boot").is_err());
2668+
}
2669+
2670+
// Test 8: Nested empty directories should fail (empty directories are not mount points)
2671+
{
2672+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2673+
td.create_dir_all("var/lib/containers")?;
2674+
td.create_dir_all("var/log/journal")?;
2675+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2676+
}
2677+
2678+
// Test 9: Directory with lost+found and a file should fail (lost+found is ignored, but file is not allowed)
2679+
{
2680+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2681+
td.create_dir_all("var/lost+found")?;
2682+
td.write("var/data.txt", b"content")?;
2683+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2684+
}
2685+
2686+
// Test 10: Directory with a symlink should fail
2687+
{
2688+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2689+
td.create_dir("var")?;
2690+
td.symlink_contents("../usr/lib", "var/lib")?;
2691+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2692+
}
2693+
2694+
// Test 11: Deeply nested directory with a file should fail
2695+
{
2696+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2697+
td.create_dir_all("var/lib/containers/storage/overlay")?;
2698+
td.write("var/lib/containers/storage/overlay/file.txt", b"data")?;
2699+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2700+
}
2701+
2702+
// Test 12: Non-boot directory with efi should fail (special handling only applies to boot/efi)
2703+
{
2704+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2705+
td.create_dir_all("var/efi")?;
2706+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2707+
}
2708+
2709+
Ok(())
2710+
}
25762711
}

crates/tests-integration/src/install.rs

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,143 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
9393
cmd!(sh, "sudo {BASE_ARGS...} -v {tmpdisk}:/disk {image} bootc install to-disk --via-loopback /disk").run()?;
9494
Ok(())
9595
}),
96+
Trial::test(
97+
"install to-filesystem with separate /var mount",
98+
move || {
99+
let sh = &xshell::Shell::new()?;
100+
reset_root(sh, image)?;
101+
102+
// Create work directory for the test
103+
let tmpd = sh.create_temp_dir()?;
104+
let work_dir = tmpd.path();
105+
106+
// Create a disk image with partitions for root and var
107+
let disk_img = work_dir.join("disk.img");
108+
let size = 12 * 1024 * 1024 * 1024;
109+
let disk_file = std::fs::File::create(&disk_img)?;
110+
disk_file.set_len(size)?;
111+
drop(disk_file);
112+
113+
// Setup loop device
114+
let loop_dev = cmd!(sh, "sudo losetup -f --show {disk_img}")
115+
.read()?
116+
.trim()
117+
.to_string();
118+
119+
// Helper closure for cleanup
120+
let cleanup = |sh: &Shell, loop_dev: &str, target: &str| {
121+
// Unmount filesystems
122+
let _ = cmd!(sh, "sudo umount -R {target}").ignore_status().run();
123+
// Deactivate LVM
124+
let _ = cmd!(sh, "sudo vgchange -an BL").ignore_status().run();
125+
let _ = cmd!(sh, "sudo vgremove -f BL").ignore_status().run();
126+
// Detach loop device
127+
let _ = cmd!(sh, "sudo losetup -d {loop_dev}").ignore_status().run();
128+
};
129+
130+
// Create partition table
131+
if let Err(e) = (|| -> Result<()> {
132+
cmd!(sh, "sudo parted -s {loop_dev} mklabel gpt").run()?;
133+
// Create BIOS boot partition (for GRUB on GPT)
134+
cmd!(sh, "sudo parted -s {loop_dev} mkpart primary 1MiB 2MiB").run()?;
135+
cmd!(sh, "sudo parted -s {loop_dev} set 1 bios_grub on").run()?;
136+
// Create EFI partition
137+
cmd!(
138+
sh,
139+
"sudo parted -s {loop_dev} mkpart primary fat32 2MiB 202MiB"
140+
)
141+
.run()?;
142+
cmd!(sh, "sudo parted -s {loop_dev} set 2 esp on").run()?;
143+
// Create boot partition
144+
cmd!(
145+
sh,
146+
"sudo parted -s {loop_dev} mkpart primary ext4 202MiB 1226MiB"
147+
)
148+
.run()?;
149+
// Create LVM partition
150+
cmd!(sh, "sudo parted -s {loop_dev} mkpart primary 1226MiB 100%").run()?;
151+
152+
// Reload partition table
153+
cmd!(sh, "sudo partprobe {loop_dev}").run()?;
154+
std::thread::sleep(std::time::Duration::from_secs(2));
155+
156+
let loop_part2 = format!("{}p2", loop_dev); // EFI
157+
let loop_part3 = format!("{}p3", loop_dev); // Boot
158+
let loop_part4 = format!("{}p4", loop_dev); // LVM
159+
160+
// Create filesystems on boot partitions
161+
cmd!(sh, "sudo mkfs.vfat -F32 {loop_part2}").run()?;
162+
cmd!(sh, "sudo mkfs.ext4 -F {loop_part3}").run()?;
163+
164+
// Setup LVM
165+
cmd!(sh, "sudo pvcreate {loop_part4}").run()?;
166+
cmd!(sh, "sudo vgcreate BL {loop_part4}").run()?;
167+
168+
// Create logical volumes
169+
cmd!(sh, "sudo lvcreate -L 4G -n var02 BL").run()?;
170+
cmd!(sh, "sudo lvcreate -L 5G -n root02 BL").run()?;
171+
172+
// Create filesystems on logical volumes
173+
cmd!(sh, "sudo mkfs.ext4 -F /dev/BL/var02").run()?;
174+
cmd!(sh, "sudo mkfs.ext4 -F /dev/BL/root02").run()?;
175+
176+
// Get UUIDs
177+
let root_uuid = cmd!(sh, "sudo blkid -s UUID -o value /dev/BL/root02")
178+
.read()?
179+
.trim()
180+
.to_string();
181+
let boot_uuid = cmd!(sh, "sudo blkid -s UUID -o value {loop_part2}")
182+
.read()?
183+
.trim()
184+
.to_string();
185+
186+
// Mount the partitions
187+
let target_dir = work_dir.join("target");
188+
std::fs::create_dir_all(&target_dir)?;
189+
let target = target_dir.to_str().unwrap();
190+
191+
cmd!(sh, "sudo mount /dev/BL/root02 {target}").run()?;
192+
cmd!(sh, "sudo mkdir -p {target}/boot").run()?;
193+
cmd!(sh, "sudo mount {loop_part3} {target}/boot").run()?;
194+
cmd!(sh, "sudo mkdir -p {target}/boot/efi").run()?;
195+
cmd!(sh, "sudo mount {loop_part2} {target}/boot/efi").run()?;
196+
197+
// Critical: Mount /var as a separate partition
198+
cmd!(sh, "sudo mkdir -p {target}/var").run()?;
199+
cmd!(sh, "sudo mount /dev/BL/var02 {target}/var").run()?;
200+
201+
// Run bootc install to-filesystem
202+
// This should succeed and handle the separate /var mount correctly
203+
// Mount the target at /target inside the container for simplicity
204+
cmd!(
205+
sh,
206+
"sudo {BASE_ARGS...} -v {target}:/target -v /dev:/dev {image} bootc install to-filesystem --karg=root=UUID={root_uuid} --root-mount-spec=UUID={root_uuid} --boot-mount-spec=UUID={boot_uuid} /target"
207+
)
208+
.run()?;
209+
210+
// Verify the installation succeeded
211+
// Check that bootc created the necessary files
212+
cmd!(sh, "sudo test -d {target}/ostree").run()?;
213+
cmd!(sh, "sudo test -d {target}/ostree/repo").run()?;
214+
// Verify bootloader was installed
215+
cmd!(sh, "sudo test -d {target}/boot/grub2").run()?;
216+
217+
Ok(())
218+
})() {
219+
let target = work_dir.join("target");
220+
let target_str = target.to_str().unwrap();
221+
cleanup(sh, &loop_dev, target_str);
222+
return Err(e.into());
223+
}
224+
225+
// Clean up on success
226+
let target = work_dir.join("target");
227+
let target_str = target.to_str().unwrap();
228+
cleanup(sh, &loop_dev, target_str);
229+
230+
Ok(())
231+
},
232+
),
96233
Trial::test(
97234
"replace=alongside with ssh keys and a karg, and SELinux disabled",
98235
move || {

0 commit comments

Comments
 (0)