From b695560a16ce29e73997de7f21177a9d4d30686f Mon Sep 17 00:00:00 2001 From: Philipp Gesang Date: Thu, 12 Sep 2019 11:51:37 +0200 Subject: [PATCH] Allow invoking fsync The rationale is that often it is desired that directory metadata be sync'd so changes become visible in the filesystem, in particular after creating or renaming a file [0]. This is necessary for some filesystems like ext4. To avoid races, the call to fsync(2) must be passed the same fd that was used as the target directory in the original operation (e. g. renamat(), openat()). In order for fsync to be effective, the fd must be equipped with a full set of file operations kernel-side. Opening the directory with `O_PATH` prevents this and attempts to fsync will fail with EBADF. Thus an open variant `Dir::open_full()` is added that omits the flag. [0] https://lwn.net/Articles/457667/ (section: *When should you fsync?*) Signed-off-by: Philipp Gesang --- src/dir.rs | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index e2ed930..872e9ec 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -16,6 +16,7 @@ use {Dir, AsPath}; const BASE_OPEN_FLAGS: libc::c_int = libc::O_CLOEXEC; #[cfg(not(target_os="macos"))] const BASE_OPEN_FLAGS: libc::c_int = libc::O_PATH|libc::O_CLOEXEC; +const FULL_OPEN_FLAGS: libc::c_int = libc::O_CLOEXEC; impl Dir { /// Creates a directory descriptor that resolves paths relative to current @@ -34,12 +35,18 @@ impl Dir { /// Open a directory descriptor at specified path // TODO(tailhook) maybe accept only absolute paths? pub fn open(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref()) + Dir::_open(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS) } - fn _open(path: &CStr) -> io::Result { + /// Open a directory descriptor at specified path with full + /// file operations. + pub fn open_full(path: P) -> io::Result { + Dir::_open(to_cstr(path)?.as_ref(), FULL_OPEN_FLAGS) + } + + fn _open(path: &CStr, flags: libc::c_int) -> io::Result { let fd = unsafe { - libc::open(path.as_ptr(), BASE_OPEN_FLAGS) + libc::open(path.as_ptr(), flags) }; if fd < 0 { Err(io::Error::last_os_error()) @@ -397,6 +404,16 @@ impl Dir { } } } + + /// Sync directory metadata. + pub fn sync_all(&self) -> io::Result<()> { + let res = unsafe { libc::fsync(self.0) }; + if res == -1 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } + } } /// Rename (move) a file between directories @@ -585,4 +602,21 @@ mod test { }) .is_some()); } + + #[test] + #[cfg(target_os="linux")] + #[should_panic(expected="Bad file descriptor")] + fn test_sync_base_fail() { + /* A lightweight fd obtained with `O_PATH` lacks file operations. */ + let dir = Dir::open("src").unwrap(); + dir.sync_all().unwrap(); + } + + #[test] + #[cfg(target_os="linux")] + fn test_sync_full_ok() { + /* A regular fd can be fsync(2)ed. */ + let dir = Dir::open_full("src").unwrap(); + assert!(dir.sync_all().is_ok()); + } }