tailhook / openat

A wrapper around openat, symlinkat, and similar system calls
Apache License 2.0
21 stars 19 forks source link

The fd in a Dir object cannot be used with fsync() #47

Open zackw opened 10 months ago

zackw commented 10 months ago

If you want a guarantee that a rename() operation has been committed to disk, you need to call fsync() on all the directories that were involved, which of course means you need file descriptors referring to those directories. I was hoping I could use openat::Dir to abstract over the process of acquiring those fds, and just write this in my code:

use std::io;
use std::os::fd::AsRawFd;
use openat::Dir;
fn sync_dir(d: &Dir) -> io::Result<()> {
    // SAFETY: trusts you to supply a valid fd, manual error checking required
    let status = unsafe { libc::fsync(d.as_raw_fd()) };
    if status != 0 {
        return Err(io::Error::last_os_error());
    }
    Ok(())
}

Unfortunately, this doesn't work; the fsync() call fails with code EBADF. The problem appears to be that openat::Dir wraps a fd that was opened with O_PATH, and to use fsync you instead need a fd that was opened with O_RDONLY|O_DIRECTORY. A C program that demonstrates this requirement is attached.

As far as I know, the only other differences between O_PATH and O_RDONLY|O_DIRECTORY, when applied to a directory, are that in the latter case you need read permission on the directory, and, if it succeeds, you are then allowed to read the contents of the directory (readdir in C, getdents at the syscall level). Thus, the Right Thing™, in my opinion, would be, on Linux, to attempt an open call first with O_RDONLY|O_DIRECTORY and then fall back to O_PATH|O_DIRECTORY if that fails with EPERM. This change would fix my problem and should also fix #34.

fsync_dir_requires_o_directory.c

cehteh commented 10 months ago

In my fork (openat_ct) I added a DirFlags builder which allows to specify the flags for creating Dir's. I just noticed that the documentation is wrong there in a earlier implementation i forced O_PATH flags, that's not there anymore. Eventually i should go over it and fix the doc and bring that crate up to date (remove the build.rs). A sync() -> Result<(), CanNotSync> method on a Dir could be nice, PR welcome.