Skip to content

Commit e06163d

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 e06163d

File tree

2 files changed

+306
-13
lines changed

2 files changed

+306
-13
lines changed

crates/lib/src/install.rs

Lines changed: 169 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1803,6 +1803,70 @@ 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, which is acceptable
1820+
return Ok(());
1821+
};
1822+
1823+
let mut found_any_entry = false;
1824+
1825+
for entry in dir_fd.entries()? {
1826+
let entry = DirEntryUtf8::from_cap_std(entry?);
1827+
let entry_name = entry.file_name()?;
1828+
1829+
// Skip lost+found but count it as finding an entry
1830+
if entry_name == LOST_AND_FOUND {
1831+
found_any_entry = true;
1832+
continue;
1833+
}
1834+
1835+
// Allow the EFI directory in /boot but count it as finding an entry
1836+
if dir_name == BOOT && entry_name == crate::bootloader::EFI_DIR {
1837+
found_any_entry = true;
1838+
continue;
1839+
}
1840+
1841+
// Mark that we found a real entry
1842+
found_any_entry = true;
1843+
1844+
let etype = entry.file_type()?;
1845+
if etype == FileType::dir() {
1846+
// If open_dir_noxdev returns None, this is a mount point on a different filesystem
1847+
if dir_fd.open_dir_noxdev(&entry_name)?.is_none() {
1848+
tracing::debug!("Found mount point: {dir_name}/{entry_name}");
1849+
continue;
1850+
}
1851+
1852+
// Not a mount point itself, but check recursively if it contains only mounts
1853+
require_dir_contains_only_mounts(&dir_fd, &entry_name)?;
1854+
tracing::debug!("Directory {dir_name}/{entry_name} contains only mount points");
1855+
continue;
1856+
}
1857+
1858+
// Found a non-mount, non-directory-of-mounts entry
1859+
anyhow::bail!("Found non-mount entry in {dir_name}: {entry_name}");
1860+
}
1861+
1862+
// Empty directories (that are not mounts themselves) are not allowed
1863+
if !found_any_entry {
1864+
anyhow::bail!("Empty directory (not a mount): {dir_name}");
1865+
}
1866+
1867+
Ok(())
1868+
}
1869+
18061870
#[context("Verifying empty rootfs")]
18071871
fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> {
18081872
for e in rootfs_fd.entries()? {
@@ -1811,20 +1875,15 @@ fn require_empty_rootdir(rootfs_fd: &Dir) -> Result<()> {
18111875
if name == LOST_AND_FOUND {
18121876
continue;
18131877
}
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-
}
1825-
} else {
1826-
anyhow::bail!("Non-empty root filesystem; found {name:?}");
1878+
1879+
// Check if this entry is a directory
1880+
let etype = e.file_type()?;
1881+
if etype == FileType::dir() {
1882+
require_dir_contains_only_mounts(rootfs_fd, &name)?;
18271883
}
1884+
1885+
// If we reach here, found an entry that shouldn't exist
1886+
anyhow::bail!("Non-empty root filesystem; found {name:?}");
18281887
}
18291888
Ok(())
18301889
}
@@ -2573,4 +2632,101 @@ UUID=boot-uuid /boot ext4 defaults 0 0
25732632

25742633
Ok(())
25752634
}
2635+
2636+
#[test]
2637+
fn test_require_dir_contains_only_mounts() -> Result<()> {
2638+
// Test 1: Empty directory should fail (not a mount point)
2639+
{
2640+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2641+
td.create_dir("empty")?;
2642+
assert!(require_dir_contains_only_mounts(&td, "empty").is_err());
2643+
}
2644+
2645+
// Test 2: Directory with only lost+found should succeed (lost+found is ignored)
2646+
{
2647+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2648+
td.create_dir_all("var/lost+found")?;
2649+
assert!(require_dir_contains_only_mounts(&td, "var").is_ok());
2650+
}
2651+
2652+
// Test 3: Directory with a regular file should fail
2653+
{
2654+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2655+
td.create_dir("var")?;
2656+
td.write("var/test.txt", b"content")?;
2657+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2658+
}
2659+
2660+
// Test 4: Nested directory structure with a file should fail
2661+
{
2662+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2663+
td.create_dir_all("var/lib/containers")?;
2664+
td.write("var/lib/containers/storage.db", b"data")?;
2665+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2666+
}
2667+
2668+
// Test 5: boot directory with efi subdirectory should succeed (efi is allowed in boot)
2669+
{
2670+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2671+
td.create_dir_all("boot/efi")?;
2672+
assert!(require_dir_contains_only_mounts(&td, "boot").is_ok());
2673+
}
2674+
2675+
// Test 6: boot directory with both efi and lost+found should succeed (both are allowed)
2676+
{
2677+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2678+
td.create_dir_all("boot/efi")?;
2679+
td.create_dir_all("boot/lost+found")?;
2680+
assert!(require_dir_contains_only_mounts(&td, "boot").is_ok());
2681+
}
2682+
2683+
// Test 7: boot directory with grub should fail (grub2 is not a mount and contains files)
2684+
{
2685+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2686+
td.create_dir_all("boot/grub2")?;
2687+
td.write("boot/grub2/grub.cfg", b"config")?;
2688+
assert!(require_dir_contains_only_mounts(&td, "boot").is_err());
2689+
}
2690+
2691+
// Test 8: Nested empty directories should fail (empty directories are not mount points)
2692+
{
2693+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2694+
td.create_dir_all("var/lib/containers")?;
2695+
td.create_dir_all("var/log/journal")?;
2696+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2697+
}
2698+
2699+
// Test 9: Directory with lost+found and a file should fail (lost+found is ignored, but file is not allowed)
2700+
{
2701+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2702+
td.create_dir_all("var/lost+found")?;
2703+
td.write("var/data.txt", b"content")?;
2704+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2705+
}
2706+
2707+
// Test 10: Directory with a symlink should fail
2708+
{
2709+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2710+
td.create_dir("var")?;
2711+
td.symlink_contents("../usr/lib", "var/lib")?;
2712+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2713+
}
2714+
2715+
// Test 11: Deeply nested directory with a file should fail
2716+
{
2717+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2718+
td.create_dir_all("var/lib/containers/storage/overlay")?;
2719+
td.write("var/lib/containers/storage/overlay/file.txt", b"data")?;
2720+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2721+
}
2722+
2723+
// Test 12: Non-boot directory with efi should fail (special handling only applies to boot/efi)
2724+
{
2725+
let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
2726+
td.create_dir_all("var/efi")?;
2727+
assert!(require_dir_contains_only_mounts(&td, "var").is_err());
2728+
}
2729+
2730+
Ok(())
2731+
}
25762732
}

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)