diff --git a/tests/test_cases/src/test_virtiofs_misc.rs b/tests/test_cases/src/test_virtiofs_misc.rs index 5bd6c860b..085d4dab8 100644 --- a/tests/test_cases/src/test_virtiofs_misc.rs +++ b/tests/test_cases/src/test_virtiofs_misc.rs @@ -73,12 +73,34 @@ mod guest { use super::*; use crate::Test; + use std::collections::HashSet; + use std::ffi::{CStr, CString}; use std::fs; use std::io::{Read, Seek, SeekFrom, Write}; use std::os::unix::fs::MetadataExt; use std::os::unix::io::AsRawFd; + use std::panic::catch_unwind; use nix::fcntl::{fallocate, FallocateFlags}; + use nix::libc; + + fn run_subtests(tests: &[(&str, fn())]) { + let mut failed = Vec::new(); + for (name, f) in tests { + if catch_unwind(f).is_err() { + eprintln!("FAIL: {name}"); + failed.push(*name); + } else { + eprintln!("PASS: {name}"); + } + } + + if failed.is_empty() { + println!("OK"); + } else { + println!("FAILED: {}", failed.join(", ")); + } + } fn test_fallocate_basic() { let path = "/test_fallocate_basic"; @@ -196,14 +218,383 @@ mod guest { assert!(ret.is_err(), "PUNCH_HOLE without KEEP_SIZE should fail"); } + // -- DirStream cache coherence tests (PR #693) -- + // + // The macOS virtiofs passthrough caches directory entries in DirStream once + // per open handle. Without the fix, mutations (create, unlink, mkdir, etc.) + // are invisible to subsequent readdir calls on the same handle. + + /// Read all entry names from `dir` (excluding "." and ".."). + unsafe fn read_entries(dir: *mut libc::DIR) -> HashSet { + let mut names = HashSet::new(); + loop { + let ent = libc::readdir(dir); + if ent.is_null() { + break; + } + let name = CStr::from_ptr((*ent).d_name.as_ptr()) + .to_string_lossy() + .into_owned(); + if name != "." && name != ".." { + names.insert(name); + } + } + names + } + + /// Test that files created after an initial readdir are visible after rewinddir. + fn test_dirstream_create() { + let dir = "/test_dirstream_create"; + fs::create_dir(dir).expect("mkdir"); + fs::write(format!("{dir}/before"), b"").expect("write before"); + + let c_dir = CString::new(dir).unwrap(); + unsafe { + let dp = libc::opendir(c_dir.as_ptr()); + assert!(!dp.is_null(), "opendir failed"); + + let entries1 = read_entries(dp); + assert!(entries1.contains("before"), "should see 'before' initially"); + assert!(!entries1.contains("after1"), "should not see 'after1' yet"); + + fs::write(format!("{dir}/after1"), b"").expect("write after1"); + fs::write(format!("{dir}/after2"), b"").expect("write after2"); + + libc::rewinddir(dp); + let entries2 = read_entries(dp); + assert!(entries2.contains("before"), "should still see 'before'"); + assert!( + entries2.contains("after1"), + "should see 'after1' after rewinddir" + ); + assert!( + entries2.contains("after2"), + "should see 'after2' after rewinddir" + ); + + libc::closedir(dp); + } + } + + /// Test that unlinked files disappear from readdir after rewinddir. + fn test_dirstream_unlink() { + let dir = "/test_dirstream_unlink"; + fs::create_dir(dir).expect("mkdir"); + fs::write(format!("{dir}/keep"), b"").expect("write keep"); + fs::write(format!("{dir}/remove_me"), b"").expect("write remove_me"); + + let c_dir = CString::new(dir).unwrap(); + unsafe { + let dp = libc::opendir(c_dir.as_ptr()); + assert!(!dp.is_null(), "opendir failed"); + + let entries1 = read_entries(dp); + assert!( + entries1.contains("remove_me"), + "should see 'remove_me' initially" + ); + + fs::remove_file(format!("{dir}/remove_me")).expect("unlink"); + + libc::rewinddir(dp); + let entries2 = read_entries(dp); + assert!(entries2.contains("keep"), "should still see 'keep'"); + assert!( + !entries2.contains("remove_me"), + "should not see 'remove_me' after unlink" + ); + + libc::closedir(dp); + } + } + + /// Test that mkdir/rmdir are reflected in readdir after rewinddir. + fn test_dirstream_mkdir_rmdir() { + let dir = "/test_dirstream_mkdir"; + fs::create_dir(dir).expect("mkdir"); + + let c_dir = CString::new(dir).unwrap(); + unsafe { + let dp = libc::opendir(c_dir.as_ptr()); + assert!(!dp.is_null(), "opendir failed"); + + let entries1 = read_entries(dp); + assert!(entries1.is_empty(), "dir should start empty"); + + fs::create_dir(format!("{dir}/subdir")).expect("mkdir subdir"); + + libc::rewinddir(dp); + let entries2 = read_entries(dp); + assert!( + entries2.contains("subdir"), + "should see 'subdir' after mkdir" + ); + + fs::remove_dir(format!("{dir}/subdir")).expect("rmdir subdir"); + + libc::rewinddir(dp); + let entries3 = read_entries(dp); + assert!( + !entries3.contains("subdir"), + "should not see 'subdir' after rmdir" + ); + + libc::closedir(dp); + } + } + + /// Test that symlink creation is reflected in readdir after rewinddir. + fn test_dirstream_symlink() { + let dir = "/test_dirstream_symlink"; + fs::create_dir(dir).expect("mkdir"); + fs::write("/symlink_target", b"target").expect("write target"); + + let c_dir = CString::new(dir).unwrap(); + unsafe { + let dp = libc::opendir(c_dir.as_ptr()); + assert!(!dp.is_null(), "opendir failed"); + + let entries1 = read_entries(dp); + assert!(entries1.is_empty(), "dir should start empty"); + + std::os::unix::fs::symlink("/symlink_target", format!("{dir}/link")).expect("symlink"); + + libc::rewinddir(dp); + let entries2 = read_entries(dp); + assert!(entries2.contains("link"), "should see symlink 'link'"); + + libc::closedir(dp); + } + } + + /// Test that hard link creation is reflected in readdir after rewinddir. + fn test_dirstream_link() { + let dir = "/test_dirstream_link"; + fs::create_dir(dir).expect("mkdir"); + fs::write(format!("{dir}/original"), b"data").expect("write original"); + + let c_dir = CString::new(dir).unwrap(); + unsafe { + let dp = libc::opendir(c_dir.as_ptr()); + assert!(!dp.is_null(), "opendir failed"); + + let entries1 = read_entries(dp); + assert_eq!(entries1.len(), 1, "should have 1 entry initially"); + + fs::hard_link(format!("{dir}/original"), format!("{dir}/hardlink")).expect("hard_link"); + + libc::rewinddir(dp); + let entries2 = read_entries(dp); + assert!(entries2.contains("hardlink"), "should see 'hardlink'"); + + libc::closedir(dp); + } + } + + /// Test that rename is reflected in readdir after rewinddir. + fn test_dirstream_rename() { + let dir = "/test_dirstream_rename"; + fs::create_dir(dir).expect("mkdir"); + fs::write(format!("{dir}/old_name"), b"data").expect("write old_name"); + + let c_dir = CString::new(dir).unwrap(); + unsafe { + let dp = libc::opendir(c_dir.as_ptr()); + assert!(!dp.is_null(), "opendir failed"); + + let entries1 = read_entries(dp); + assert!( + entries1.contains("old_name"), + "should see 'old_name' initially" + ); + + fs::rename(format!("{dir}/old_name"), format!("{dir}/new_name")).expect("rename"); + + libc::rewinddir(dp); + let entries2 = read_entries(dp); + assert!( + !entries2.contains("old_name"), + "should not see 'old_name' after rename" + ); + assert!( + entries2.contains("new_name"), + "should see 'new_name' after rename" + ); + + libc::closedir(dp); + } + } + + /// Test rename across directories: both source and dest handles should reflect it. + fn test_dirstream_rename_cross_dir() { + let src_dir = "/test_dirstream_rename_src"; + let dst_dir = "/test_dirstream_rename_dst"; + fs::create_dir(src_dir).expect("mkdir src"); + fs::create_dir(dst_dir).expect("mkdir dst"); + fs::write(format!("{src_dir}/moved_file"), b"data").expect("write"); + + let c_src = CString::new(src_dir).unwrap(); + let c_dst = CString::new(dst_dir).unwrap(); + unsafe { + let dp_src = libc::opendir(c_src.as_ptr()); + let dp_dst = libc::opendir(c_dst.as_ptr()); + assert!(!dp_src.is_null() && !dp_dst.is_null(), "opendir failed"); + + // Populate caches. + let _ = read_entries(dp_src); + let _ = read_entries(dp_dst); + + fs::rename( + format!("{src_dir}/moved_file"), + format!("{dst_dir}/moved_file"), + ) + .expect("rename cross-dir"); + + libc::rewinddir(dp_src); + libc::rewinddir(dp_dst); + let src_entries = read_entries(dp_src); + let dst_entries = read_entries(dp_dst); + + assert!( + !src_entries.contains("moved_file"), + "source should not contain 'moved_file'" + ); + assert!( + dst_entries.contains("moved_file"), + "dest should contain 'moved_file'" + ); + + libc::closedir(dp_src); + libc::closedir(dp_dst); + } + } + + /// Test that creating files mid-iteration does not cause duplicates. + /// + /// POSIX says readdir behavior is unspecified when the directory is modified + /// during iteration. In practice (verified on ext4, btrfs, and tmpfs), + /// entries already returned are not repeated. + /// + /// Creates files named `m_*`, reads a few, then inserts `aaa_*` which are + /// likely to land before the already-returned entries in the directory + /// ordering — maximizing the chance of exposing duplicate entries. + fn test_dirstream_no_duplicates_on_mid_iteration_create() { + let dir = "/test_dirstream_mid_iter"; + fs::create_dir(dir).expect("mkdir"); + + // Pre-populate with files that sort late so the sneaky insert lands before them. + for i in 0..10 { + fs::write(format!("{dir}/m_{i:02}"), b"").expect("write"); + } + + let c_dir = CString::new(dir).unwrap(); + unsafe { + let dp = libc::opendir(c_dir.as_ptr()); + assert!(!dp.is_null(), "opendir failed"); + + // Read a few entries (not all). We ask for 7 readdir calls to + // get past "." and ".." and read ~5 real entries. + let mut seen = Vec::new(); + for _ in 0..7 { + let ent = libc::readdir(dp); + if ent.is_null() { + break; + } + let name = CStr::from_ptr((*ent).d_name.as_ptr()) + .to_string_lossy() + .into_owned(); + if name != "." && name != ".." { + seen.push(name); + } + } + + let before_mutation = seen.clone(); + + // Insert files that sort BEFORE everything we already read. + // If the host returns entries sorted by name, these shift all + // existing entries to the right in the rebuilt cache. + for i in 0..3 { + fs::write(format!("{dir}/aaa_{i:02}"), b"").expect("write aaa"); + } + + // Continue reading the rest. + let mark = seen.len(); + loop { + let ent = libc::readdir(dp); + if ent.is_null() { + break; + } + let name = CStr::from_ptr((*ent).d_name.as_ptr()) + .to_string_lossy() + .into_owned(); + if name != "." && name != ".." { + seen.push(name); + } + } + + let after_mutation: Vec<_> = seen[mark..].to_vec(); + + // Find duplicates. + let mut dups = Vec::new(); + for (i, a) in seen.iter().enumerate() { + for b in &seen[..i] { + if a == b { + dups.push(a.clone()); + break; + } + } + } + + let unique: HashSet<&str> = seen.iter().map(|s| s.as_str()).collect(); + assert!( + dups.is_empty(), + "readdir returned {} duplicates: {dups:?}\n\ + before mutation: {before_mutation:?}\n\ + after mutation: {after_mutation:?}\n\ + all seen ({} total): {seen:?}", + dups.len(), + seen.len(), + ); + + // All original files should be present (they existed before iteration + // started and were never removed). + for i in 0..10 { + let name = format!("m_{i:02}"); + assert!( + unique.contains(name.as_str()), + "original file {name} missing from readdir results: {seen:?}" + ); + } + + libc::closedir(dp); + } + } + impl Test for TestVirtioFsMisc { fn in_guest(self: Box) { - test_fallocate_basic(); - test_fallocate_keep_size(); - test_fallocate_punch_hole(); - test_fallocate_punch_hole_requires_keep_size(); - - println!("OK"); + run_subtests(&[ + ("fallocate_basic", test_fallocate_basic), + ("fallocate_keep_size", test_fallocate_keep_size), + ("fallocate_punch_hole", test_fallocate_punch_hole), + ( + "fallocate_punch_hole_requires_keep_size", + test_fallocate_punch_hole_requires_keep_size, + ), + ("dirstream_create", test_dirstream_create), + ("dirstream_unlink", test_dirstream_unlink), + ("dirstream_mkdir_rmdir", test_dirstream_mkdir_rmdir), + ("dirstream_symlink", test_dirstream_symlink), + ("dirstream_link", test_dirstream_link), + ("dirstream_rename", test_dirstream_rename), + ( + "dirstream_rename_cross_dir", + test_dirstream_rename_cross_dir, + ), + ( + "dirstream_no_duplicates_mid_iter", + test_dirstream_no_duplicates_on_mid_iteration_create, + ), + ]); } } }