Skip to content

Commit 40c5216

Browse files
ckyrouaccgwalters
authored andcommitted
install: Fix bug in mount point check
This fixes a regression from #1727 by removing the unnecessary mount point check prior to the recursive function call. Also adds some tracing statements and updates the integration test to validate the mount check works for this scenario: /boot/efi mounted with contents in /boot/efi/EFI/firmware/foo Signed-off-by: ckyrouac <ckyrouac@redhat.com>
1 parent 8734dcc commit 40c5216

2 files changed

Lines changed: 21 additions & 8 deletions

File tree

crates/lib/src/install.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1874,8 +1874,10 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
18741874
/// /var/lib/containers (a mount point).
18751875
#[context("Requiring directory contains only mount points")]
18761876
fn require_dir_contains_only_mounts(parent_fd: &Dir, dir_name: &str) -> Result<()> {
1877+
tracing::trace!("Checking directory {dir_name} for non-mount entries");
18771878
let Some(dir_fd) = parent_fd.open_dir_noxdev(dir_name)? else {
18781879
// The directory itself is a mount point
1880+
tracing::trace!("{dir_name} is a mount point");
18791881
return Ok(());
18801882
};
18811883

@@ -1884,6 +1886,7 @@ fn require_dir_contains_only_mounts(parent_fd: &Dir, dir_name: &str) -> Result<(
18841886
}
18851887

18861888
for entry in dir_fd.entries()? {
1889+
tracing::trace!("Checking entry in {dir_name}");
18871890
let entry = DirEntryUtf8::from_cap_std(entry?);
18881891
let entry_name = entry.file_name()?;
18891892

@@ -1892,7 +1895,7 @@ fn require_dir_contains_only_mounts(parent_fd: &Dir, dir_name: &str) -> Result<(
18921895
}
18931896

18941897
let etype = entry.file_type()?;
1895-
if etype == FileType::dir() && dir_fd.open_dir_noxdev(&entry_name)?.is_some() {
1898+
if etype == FileType::dir() {
18961899
require_dir_contains_only_mounts(&dir_fd, &entry_name)?;
18971900
} else {
18981901
anyhow::bail!("Found entry in {dir_name}: {entry_name}");

crates/tests-integration/src/install.rs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
9999
let sh = &xshell::Shell::new()?;
100100
reset_root(sh, image)?;
101101

102+
// Clean up any leftover LVM state from previous failed runs
103+
let _ = cmd!(sh, "sudo vgchange -an BL").ignore_status().run();
104+
let _ = cmd!(sh, "sudo vgremove -f BL").ignore_status().run();
105+
102106
// Create work directory for the test
103107
let tmpd = sh.create_temp_dir()?;
104108
let work_dir = tmpd.path();
@@ -153,13 +157,13 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
153157
cmd!(sh, "sudo partprobe {loop_dev}").run()?;
154158
std::thread::sleep(std::time::Duration::from_secs(2));
155159

156-
let loop_part2 = format!("{}p2", loop_dev); // EFI
157-
let loop_part3 = format!("{}p3", loop_dev); // Boot
160+
let efi_part2 = format!("{}p2", loop_dev); // EFI
161+
let root_part3 = format!("{}p3", loop_dev); // Boot
158162
let loop_part4 = format!("{}p4", loop_dev); // LVM
159163

160164
// 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()?;
165+
cmd!(sh, "sudo mkfs.vfat -F32 {efi_part2}").run()?;
166+
cmd!(sh, "sudo mkfs.ext4 -F {root_part3}").run()?;
163167

164168
// Setup LVM
165169
cmd!(sh, "sudo pvcreate {loop_part4}").run()?;
@@ -178,7 +182,7 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
178182
.read()?
179183
.trim()
180184
.to_string();
181-
let boot_uuid = cmd!(sh, "sudo blkid -s UUID -o value {loop_part2}")
185+
let boot_uuid = cmd!(sh, "sudo blkid -s UUID -o value {efi_part2}")
182186
.read()?
183187
.trim()
184188
.to_string();
@@ -190,9 +194,15 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments)
190194

191195
cmd!(sh, "sudo mount /dev/BL/root02 {target}").run()?;
192196
cmd!(sh, "sudo mkdir -p {target}/boot").run()?;
193-
cmd!(sh, "sudo mount {loop_part3} {target}/boot").run()?;
197+
cmd!(sh, "sudo mount {root_part3} {target}/boot").run()?;
194198
cmd!(sh, "sudo mkdir -p {target}/boot/efi").run()?;
195-
cmd!(sh, "sudo mount {loop_part2} {target}/boot/efi").run()?;
199+
cmd!(sh, "sudo mount {efi_part2} {target}/boot/efi").run()?;
200+
201+
// Create EFI directory structure with some files (simulating existing EFI content)
202+
// This tests that bootc correctly handles /boot/efi as a mount point
203+
cmd!(sh, "sudo mkdir -p {target}/boot/efi/EFI/fedora").run()?;
204+
cmd!(sh, "sudo touch {target}/boot/efi/EFI/fedora/shimx64.efi").run()?;
205+
cmd!(sh, "sudo touch {target}/boot/efi/EFI/fedora/grubx64.efi").run()?;
196206

197207
// Critical: Mount /var as a separate partition
198208
cmd!(sh, "sudo mkdir -p {target}/var").run()?;

0 commit comments

Comments
 (0)