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

Update O_SYNC/O_DSYNC open flag check to occur ahead of lookup #1042

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions mountpoint-s3/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,12 @@ where
#[cfg(target_os = "linux")]
let direct_io = flags & libc::O_DIRECT != 0;

// We can't support O_SYNC writes because they require the data to go to stable storage
// at `write` time, but we only commit a PUT at `close` time.
if flags & (libc::O_SYNC | libc::O_DSYNC) != 0 {
return Err(err!(libc::EINVAL, "O_SYNC and O_DSYNC are not supported"));
}

let force_revalidate = !self.config.cache_config.serve_lookup_from_cache || direct_io;
let lookup = self.superblock.getattr(&self.client, ino, force_revalidate).await?;

Expand All @@ -785,12 +791,6 @@ where
return Err(err!(libc::EINVAL, "O_APPEND is not supported on existing files"));
}

// We can't support O_SYNC writes because they require the data to go to stable storage
// at `write` time, but we only commit a PUT at `close` time.
if flags & (libc::O_SYNC | libc::O_DSYNC) != 0 {
return Err(err!(libc::EINVAL, "O_SYNC and O_DSYNC are not supported"));
}

let state = if flags & libc::O_RDWR != 0 {
let is_truncate = flags & libc::O_TRUNC != 0;
if !remote_file || (self.config.allow_overwrite && is_truncate) {
Expand Down
23 changes: 23 additions & 0 deletions mountpoint-s3/tests/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,29 @@ async fn test_unordered_write_fails(offset: i64) {
assert_eq!(err, libc::EINVAL);
}

#[test_case(libc::O_SYNC; "O_SYNC")]
#[test_case(libc::O_DSYNC; "O_DSYNC")]
#[tokio::test]
async fn test_sync_flags_fail(flag: libc::c_int) {
const BUCKET_NAME: &str = "test_sync_flags_fail";

let (_client, fs) = make_test_filesystem(BUCKET_NAME, &Default::default(), Default::default());

let mode = libc::S_IFREG | libc::S_IRWXU; // regular file + 0700 permissions
let dentry = fs
.mknod(FUSE_ROOT_INODE, "file2.bin".as_ref(), mode, 0, 0)
.await
.unwrap();
let file_ino = dentry.attr.ino;

let err = fs
.open(file_ino, libc::S_IFREG as i32 | flag, 0)
.await
.expect_err("open should fail due to use of O_SYNC/O_DSYNC flag")
.to_errno();
assert_eq!(err, libc::EINVAL);
}

#[tokio::test]
async fn test_duplicate_write_fails() {
const BUCKET_NAME: &str = "test_duplicate_write_fails";
Expand Down
Loading