Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow invoking fsync #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow invoking fsync #20

wants to merge 1 commit into from

Conversation

phi-gamma
Copy link
Contributor

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 [email protected]

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 <[email protected]>
@phi-gamma
Copy link
Contributor Author

phi-gamma commented Sep 24, 2019

So this implements just the bare minimum that I need
for reliably persisting files in directories that I use openat::Dir
handles for. As stated in the commit message, the goal is to
be able to call fsync(2) on such directories.

The sync_all() method is analogous to std::fs::File.
open_full() is just a suggestion. It might be preferable
to have something like open_with_flags() instead for the
user to customize the call to open() further e. g. by passing
O_DIRECTORY. Ideally we could use
std::os::unix::fs::OpenOptionsExt instead of raw bit
flags but that type is opaque and would have to be
duplicated.

@tailhook
Copy link
Owner

One quick question: why the method is called sync_all?

open_full is good, but we should also consider #18: i.e. we need open_flags for fuchsia, so we might as well provide it now.

Also we need either sub_dir with flags or propagate the flags (or both) as discussed in #18.

@phi-gamma
Copy link
Contributor Author

One quick question: why the method is called sync_all?

Because that’s what the equivalent is called on std::fs::File:
https://doc.rust-lang.org/std/fs/struct.File.html#method.sync_all
Since this method exists I think it’s best to stick with that
nomenclature.

Also we need either sub_dir with flags or propagate the flags (or both) as discussed in #18

That would be reasonable, true. I’ll give it some thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants