From 89501b71b68d69d2e65e6efe27ccd99da400a55b Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Mon, 14 Apr 2025 14:38:52 +0800 Subject: [PATCH 1/2] Fix: unable to perform sync on directory when no_open=false, no_opendir=true, we can't execute the sync command on the directory, because fsyncdir uses get_data to get the handle of the directory, but no_open=false, so it will go to the handle map to find it, but in fact this is a directory, and the no_opendir is true, so it is not in the handle map, It needs to be reopened. we should use get_dirdata replace get_data Signed-off-by: tianqian.zyf --- src/passthrough/sync_io.rs | 32 +++++++++++++++----------------- src/passthrough/util.rs | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/passthrough/sync_io.rs b/src/passthrough/sync_io.rs index a740a3536..d795f308f 100644 --- a/src/passthrough/sync_io.rs +++ b/src/passthrough/sync_io.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use std::time::Duration; use super::os_compat::LinuxDirent64; -use super::util::stat_fd; +use super::util::{stat_fd, sync_fd}; use super::*; use crate::abi::fuse_abi::{CreateIn, Opcode, FOPEN_IN_KILL_SUIDGID, WRITE_KILL_PRIV}; #[cfg(any(feature = "vhost-user-fs", feature = "virtiofs"))] @@ -1074,30 +1074,19 @@ impl FileSystem for PassthroughFs { ) -> io::Result<()> { let data = self.get_data(handle, inode, libc::O_RDONLY)?; let fd = data.borrow_fd(); - - // Safe because this doesn't modify any memory and we check the return value. - let res = unsafe { - if datasync { - libc::fdatasync(fd.as_raw_fd()) - } else { - libc::fsync(fd.as_raw_fd()) - } - }; - if res == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } + sync_fd(&fd, datasync) } fn fsyncdir( &self, - ctx: &Context, + _ctx: &Context, inode: Inode, datasync: bool, handle: Handle, ) -> io::Result<()> { - self.fsync(ctx, inode, datasync, handle) + let data = self.get_dirdata(handle, inode, libc::O_RDONLY)?; + let fd = data.borrow_fd(); + sync_fd(&fd, datasync) } fn access(&self, ctx: &Context, inode: Inode, mask: u32) -> io::Result<()> { @@ -1630,4 +1619,13 @@ mod tests { let statfs = fs.statfs(&ctx, ROOT_ID).unwrap(); assert_eq!(statfs.f_namemax, 255); } + + #[test] + fn test_fsync_dir() { + let (fs, _source) = prepare_fs_tmpdir(); + let ctx = prepare_context(); + fs.no_opendir.store(true, Ordering::Relaxed); + + assert!(fs.fsyncdir(&ctx, ROOT_ID, false, 0).is_ok()); + } } diff --git a/src/passthrough/util.rs b/src/passthrough/util.rs index 08a31031f..95d7fd824 100644 --- a/src/passthrough/util.rs +++ b/src/passthrough/util.rs @@ -225,6 +225,22 @@ pub fn eperm() -> io::Error { io::Error::from_raw_os_error(libc::EPERM) } +pub fn sync_fd(fd: &impl AsRawFd, datasync: bool) -> io::Result<()> { + // Safe because this doesn't modify any memory and we check the return value. + let res = unsafe { + if datasync { + libc::fdatasync(fd.as_raw_fd()) + } else { + libc::fsync(fd.as_raw_fd()) + } + }; + if res == 0 { + Ok(()) + } else { + Err(io::Error::last_os_error()) + } +} + #[cfg(test)] mod tests { use super::*; From 974110376bf534d8b45908ad8dc354c4fb06938e Mon Sep 17 00:00:00 2001 From: "tianqian.zyf" Date: Mon, 14 Apr 2025 14:49:53 +0800 Subject: [PATCH 2/2] fix cargo clippy warnning Signed-off-by: tianqian.zyf --- src/api/filesystem/overlay.rs | 4 ++-- src/api/vfs/sync_io.rs | 8 ++------ src/transport/fusedev/mod.rs | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/api/filesystem/overlay.rs b/src/api/filesystem/overlay.rs index 1d8948d7a..445033a89 100644 --- a/src/api/filesystem/overlay.rs +++ b/src/api/filesystem/overlay.rs @@ -195,8 +195,8 @@ pub(crate) fn is_chardev(st: stat64) -> bool { pub(crate) fn is_whiteout(st: stat64) -> bool { // A whiteout is created as a character device with 0/0 device number. // See ref: https://docs.kernel.org/filesystems/overlayfs.html#whiteouts-and-opaque-directories - let major = unsafe { libc::major(st.st_rdev) }; - let minor = unsafe { libc::minor(st.st_rdev) }; + let major = libc::major(st.st_rdev); + let minor = libc::minor(st.st_rdev); is_chardev(st) && major == 0 && minor == 0 } diff --git a/src/api/vfs/sync_io.rs b/src/api/vfs/sync_io.rs index 3418fbe4a..a2aef26cb 100644 --- a/src/api/vfs/sync_io.rs +++ b/src/api/vfs/sync_io.rs @@ -306,9 +306,7 @@ impl FileSystem for Vfs { } match self.get_real_rootfs(inode)? { (Left(fs), idata) => fs.open(ctx, idata.ino(), flags, fuse_flags), - (Right(fs), idata) => fs - .open(ctx, idata.ino(), flags, fuse_flags) - .map(|(h, opt, passthrough)| (h.map(Into::into), opt, passthrough)), + (Right(fs), idata) => fs.open(ctx, idata.ino(), flags, fuse_flags), } } @@ -522,9 +520,7 @@ impl FileSystem for Vfs { } match self.get_real_rootfs(inode)? { (Left(fs), idata) => fs.opendir(ctx, idata.ino(), flags), - (Right(fs), idata) => fs - .opendir(ctx, idata.ino(), flags) - .map(|(h, opt)| (h.map(Into::into), opt)), + (Right(fs), idata) => fs.opendir(ctx, idata.ino(), flags), } } diff --git a/src/transport/fusedev/mod.rs b/src/transport/fusedev/mod.rs index 0f9bff6dd..0d8cd0e39 100644 --- a/src/transport/fusedev/mod.rs +++ b/src/transport/fusedev/mod.rs @@ -265,7 +265,7 @@ impl<'a, S: BitmapSlice> FuseDevWriter<'a, S> { } fn check_available_space(&self, sz: usize) -> io::Result<()> { - assert!(self.buffered || self.buf.len() == 0); + assert!(self.buffered || self.buf.is_empty()); if sz > self.available_bytes() { Err(io::Error::new( io::ErrorKind::InvalidData,