Skip to content

Commit e42310a

Browse files
Champ-Goblembergwolf
authored andcommitted
fuse: Ensure fd has same flags as read/write
We found when running minio with recent versions of nydus that it was no longer able to boot properly due to Invalid Argument when writing to a temporary file. Tracing the high-level flow showed that the file was originally opened with O_DIRECT, then later on if the buffer size was not correctly aligned, fcntl with F_SETFL would be used to remove the O_DIRECT flag. In virtiofs there is no independant call for fcntl, so the underlying file descriptor opened by nydus still had the O_DIRECT flag set. Meaning that during a write to this file it would error with -EINVAL as the buffer size was not aligned. This patch aims to fix this by first recording the open flags for a file in the `HandleData` struct, then on read/write checking these flags match the flags provided by virtiofs in the read/write message. If the flags do not match, the current flags of the file descriptor are set with F_SETFL and updated on the HandleData entry. Signed-off-by: Champ-Goblem <[email protected]>
1 parent 50131dd commit e42310a

File tree

2 files changed

+39
-8
lines changed

2 files changed

+39
-8
lines changed

src/passthrough/mod.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use std::os::unix::ffi::OsStringExt;
2323
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
2424
use std::path::PathBuf;
2525
use std::str::FromStr;
26-
use std::sync::atomic::{AtomicBool, AtomicU64, AtomicU8, Ordering};
26+
use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, AtomicU8, Ordering};
2727
use std::sync::{Arc, Mutex, MutexGuard, RwLock, RwLockWriteGuard};
2828
use std::time::Duration;
2929

@@ -333,14 +333,16 @@ struct HandleData {
333333
inode: Inode,
334334
file: File,
335335
lock: Mutex<()>,
336+
open_flags: AtomicU32,
336337
}
337338

338339
impl HandleData {
339-
fn new(inode: Inode, file: File) -> Self {
340+
fn new(inode: Inode, file: File, flags: u32) -> Self {
340341
HandleData {
341342
inode,
342343
file,
343344
lock: Mutex::new(()),
345+
open_flags: AtomicU32::new(flags),
344346
}
345347
}
346348

@@ -355,6 +357,14 @@ impl HandleData {
355357
fn get_handle_raw_fd(&self) -> RawFd {
356358
self.file.as_raw_fd()
357359
}
360+
361+
fn get_flags(&self) -> u32 {
362+
self.open_flags.load(Ordering::Relaxed)
363+
}
364+
365+
fn set_flags(&self, flags: u32) {
366+
self.open_flags.store(flags, Ordering::Relaxed);
367+
}
358368
}
359369

360370
struct HandleMap {

src/passthrough/sync_io.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,22 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
3636
Self::open_proc_file(&self.proc_self_fd, file.as_raw_fd(), new_flags, data.mode)
3737
}
3838

39+
/// Check the HandleData flags against the flags from the current request
40+
/// if these do not match update the file descriptor flags and store the new
41+
/// result in the HandleData entry
42+
#[inline(always)]
43+
fn check_fd_flags(&self, data: Arc<HandleData>, fd: RawFd, flags: u32) -> io::Result<()> {
44+
let open_flags = data.get_flags();
45+
if open_flags != flags {
46+
let ret = unsafe { libc::fcntl(fd, libc::F_SETFL, flags) };
47+
if ret != 0 {
48+
return Err(io::Error::last_os_error());
49+
}
50+
data.set_flags(flags);
51+
}
52+
Ok(())
53+
}
54+
3955
fn do_readdir(
4056
&self,
4157
inode: Inode,
@@ -173,7 +189,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
173189
let file = self.open_inode(inode, flags as i32)?;
174190
drop(killpriv);
175191

176-
let data = HandleData::new(inode, file);
192+
let data = HandleData::new(inode, file, flags);
177193
let handle = self.next_handle.fetch_add(1, Ordering::Relaxed);
178194
self.handle_map.insert(handle, data);
179195

@@ -258,7 +274,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
258274
self.handle_map.get(handle, inode)
259275
} else {
260276
let file = self.open_inode(inode, flags | libc::O_DIRECTORY)?;
261-
Ok(Arc::new(HandleData::new(inode, file)))
277+
Ok(Arc::new(HandleData::new(inode, file, flags as u32)))
262278
}
263279
}
264280

@@ -273,7 +289,7 @@ impl<S: BitmapSlice + Send + Sync> PassthroughFs<S> {
273289
self.handle_map.get(handle, inode)
274290
} else {
275291
let file = self.open_inode(inode, flags)?;
276-
Ok(Arc::new(HandleData::new(inode, file)))
292+
Ok(Arc::new(HandleData::new(inode, file, flags as u32)))
277293
}
278294
}
279295
}
@@ -573,7 +589,7 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
573589

574590
let ret_handle = if !self.no_open.load(Ordering::Relaxed) {
575591
let handle = self.next_handle.fetch_add(1, Ordering::Relaxed);
576-
let data = HandleData::new(entry.inode, file);
592+
let data = HandleData::new(entry.inode, file, args.flags);
577593

578594
self.handle_map.insert(handle, data);
579595
Some(handle)
@@ -643,14 +659,17 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
643659
size: u32,
644660
offset: u64,
645661
_lock_owner: Option<u64>,
646-
_flags: u32,
662+
flags: u32,
647663
) -> io::Result<usize> {
648664
let data = self.get_data(handle, inode, libc::O_RDONLY)?;
649665

650666
// Manually implement File::try_clone() by borrowing fd of data.file instead of dup().
651667
// It's safe because the `data` variable's lifetime spans the whole function,
652668
// so data.file won't be closed.
653669
let f = unsafe { File::from_raw_fd(data.get_handle_raw_fd()) };
670+
671+
self.check_fd_flags(data, f.as_raw_fd(), flags)?;
672+
654673
let mut f = ManuallyDrop::new(f);
655674

656675
w.write_from(&mut *f, size as usize, offset)
@@ -666,7 +685,7 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
666685
offset: u64,
667686
_lock_owner: Option<u64>,
668687
_delayed_write: bool,
669-
_flags: u32,
688+
flags: u32,
670689
fuse_flags: u32,
671690
) -> io::Result<usize> {
672691
let data = self.get_data(handle, inode, libc::O_RDWR)?;
@@ -676,6 +695,8 @@ impl<S: BitmapSlice + Send + Sync> FileSystem for PassthroughFs<S> {
676695
// so data.file won't be closed.
677696
let f = unsafe { File::from_raw_fd(data.get_handle_raw_fd()) };
678697

698+
self.check_fd_flags(data, f.as_raw_fd(), flags)?;
699+
679700
if self.seal_size.load(Ordering::Relaxed) {
680701
let st = Self::stat_fd(f.as_raw_fd(), None)?;
681702
self.seal_size_check(Opcode::Write, st.st_size as u64, offset, size as u64, 0)?;

0 commit comments

Comments
 (0)