Skip to content

Commit

Permalink
Optimize error handlling
Browse files Browse the repository at this point in the history
  • Loading branch information
@diqiu50
diqiu50 committed Dec 4, 2024
1 parent c36b212 commit a8456cb
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 90 deletions.
26 changes: 14 additions & 12 deletions clients/filesystem-fuse/src/filesystem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,39 +19,41 @@
use fuse3::{Errno, FileType, Timestamp};
use std::time::SystemTime;

pub type Result<T> = std::result::Result<T, Errno>;

/// File system interface for the file system implementation. it use by FuseApiHandle
/// the `file_id` and `parent_file_id` it is the unique identifier for the file system, it is used to identify the file or directory
/// the `fh` it is the file handle, it is used to identify the opened file, it is used to read or write the file content
pub trait IFileSystem: Send + Sync {
fn get_file_path(&self, file_id: u64) -> String;

fn get_opened_file(&self, file_id: u64, fh: u64) -> Option<OpenedFile>;
fn get_opened_file(&self, file_id: u64, fh: u64) -> Result<OpenedFile>;

fn stat(&self, file_id: u64) -> Option<FileStat>;
fn stat(&self, file_id: u64) -> Result<FileStat>;

fn lookup(&self, parent_file_id: u64, name: &str) -> Option<FileStat>;
fn lookup(&self, parent_file_id: u64, name: &str) -> Result<FileStat>;

fn read_dir(&self, dir_file_id: u64) -> Vec<FileStat>;
fn read_dir(&self, dir_file_id: u64) -> Result<Vec<FileStat>>;

fn open_file(&self, file_id: u64) -> Result<OpenedFile, Errno>;
fn open_file(&self, file_id: u64) -> Result<OpenedFile>;

fn create_file(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile, Errno>;
fn create_file(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile>;

fn create_dir(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile, Errno>;
fn create_dir(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile>;

fn set_attr(&self, file_id: u64, file_stat: &FileStat) -> Result<(), Errno>;
fn set_attr(&self, file_id: u64, file_stat: &FileStat) -> Result<()>;

fn update_file_status(&self, file_id: u64, file_stat: &FileStat);
fn update_file_status(&self, file_id: u64, file_stat: &FileStat) -> Result<()>;

fn read(&self, file_id: u64, fh: u64) -> Box<dyn FileReader>;

fn write(&self, file_id: u64, fh: u64) -> Box<dyn FileWriter>;

fn remove_file(&self, parent_file_id: u64, name: &str) -> Result<(), Errno>;
fn remove_file(&self, parent_file_id: u64, name: &str) -> Result<()>;

fn remove_dir(&self, parent_file_id: u64, name: &str) -> Result<(), Errno>;
fn remove_dir(&self, parent_file_id: u64, name: &str) -> Result<()>;

fn close_file(&self, file_id: u64, fh: u64) -> Result<(), Errno>;
fn close_file(&self, file_id: u64, fh: u64) -> Result<()>;
}

pub struct FileSystemContext {
Expand Down
99 changes: 39 additions & 60 deletions clients/filesystem-fuse/src/fuse_api_handle.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,19 @@ impl FuseApiHandle {
size: Option<u64>,
atime: Option<Timestamp>,
mtime: Option<Timestamp>,
) -> Option<FileStat> {
let file_stat = self.local_fs.stat(inode);
match file_stat {
Some(f) => {
let mut nf = FileStat::clone(&f);
size.map(|size| {
nf.size = size;
});
atime.map(|atime| {
nf.atime = atime;
});
mtime.map(|mtime| {
nf.mtime = mtime;
});
Some(nf)
}
_ => None,
}
) -> Result<FileStat, Errno> {
let file_stat = self.local_fs.stat(inode)?;
let mut nf = FileStat::clone(&file_stat);
size.map(|size| {
nf.size = size;
});
atime.map(|atime| {
nf.atime = atime;
});
mtime.map(|mtime| {
nf.mtime = mtime;
});
Ok(nf)
}
}

Expand All @@ -108,16 +103,12 @@ impl Filesystem for FuseApiHandle {
parent: Inode,
name: &OsStr,
) -> fuse3::Result<ReplyEntry> {
let file_stat = self.local_fs.lookup(parent, name.to_str().unwrap());

match file_stat {
Some(f) => Ok(ReplyEntry {
ttl: self.default_ttl,
attr: fstat_to_file_attr(&f, &self.fs_context),
generation: 0,
}),
None => Err(libc::ENOENT.into()),
}
let file_stat = self.local_fs.lookup(parent, name.to_str().unwrap())?;
Ok(ReplyEntry {
ttl: self.default_ttl,
attr: fstat_to_file_attr(&file_stat, &self.fs_context),
generation: 0,
})
}

async fn getattr(
Expand All @@ -128,20 +119,18 @@ impl Filesystem for FuseApiHandle {
_flags: u32,
) -> fuse3::Result<ReplyAttr> {
// check the opened file inode is the same as the inode
if let Some(f) = fh.and_then(|fh| self.local_fs.get_opened_file(inode, fh)) {
if f.file_id != inode {
if let Some(fh) = fh {
let opened_file = self.local_fs.get_opened_file(inode, fh)?;
if opened_file.file_id != inode {
return Err(libc::EBADF.into());
}
}

let file_stat = self.local_fs.stat(inode);
match file_stat {
Some(f) => Ok(ReplyAttr {
ttl: self.default_ttl,
attr: fstat_to_file_attr(&f, &self.fs_context),
}),
None => Err(libc::ENOENT.into()),
}
let file_stat = self.local_fs.stat(inode)?;
Ok(ReplyAttr {
ttl: self.default_ttl,
attr: fstat_to_file_attr(&file_stat, &self.fs_context),
})
}

async fn setattr(
Expand All @@ -152,18 +141,13 @@ impl Filesystem for FuseApiHandle {
set_attr: SetAttr,
) -> fuse3::Result<ReplyAttr> {
let new_file_stat =
self.get_modified_file_stat(inode, set_attr.size, set_attr.atime, set_attr.mtime);
match new_file_stat {
Some(stat) => {
let attr = fstat_to_file_attr(&stat, &self.fs_context);
self.local_fs.set_attr(inode, &stat);
Ok(ReplyAttr {
ttl: self.default_ttl,
attr: attr,
})
}
None => Err(libc::ENOENT.into()),
}
self.get_modified_file_stat(inode, set_attr.size, set_attr.atime, set_attr.mtime)?;
let attr = fstat_to_file_attr(&new_file_stat, &self.fs_context);
self.local_fs.set_attr(inode, &new_file_stat)?;
Ok(ReplyAttr {
ttl: self.default_ttl,
attr: attr,
})
}

async fn mkdir(
Expand Down Expand Up @@ -300,11 +284,11 @@ impl Filesystem for FuseApiHandle {
) -> fuse3::Result<ReplyDirectory<Self::DirEntryStream<'a>>> {
let current = self.local_fs.stat(parent);
let current = match current {
Some(file) => file,
None => return Err(libc::ENOENT.into()),
Ok(file) => file,
Err(e) => return Err(e),
};

let files = self.local_fs.read_dir(parent);
let files = self.local_fs.read_dir(parent)?;
let entries_stream =
stream::iter(files.into_iter().enumerate().map(|(index, file_stat)| {
Ok(DirectoryEntry {
Expand Down Expand Up @@ -383,13 +367,8 @@ impl Filesystem for FuseApiHandle {
offset: u64,
lock_owner: u64,
) -> fuse3::Result<ReplyDirectoryPlus<Self::DirEntryPlusStream<'a>>> {
let current = self.local_fs.stat(parent);
let current = match current {
Some(file) => file,
None => return Err(libc::ENOENT.into()),
};

let files = self.local_fs.read_dir(parent);
let current = self.local_fs.stat(parent)?;
let files = self.local_fs.read_dir(parent)?;
let entries_stream =
stream::iter(files.into_iter().enumerate().map(|(index, file_stat)| {
Ok(DirectoryEntryPlus {
Expand Down
36 changes: 18 additions & 18 deletions clients/filesystem-fuse/src/memory_filesystem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@
* under the License.
*/
use crate::file_handle_manager::FileHandleManager;
use crate::filesystem::{FileReader, FileStat, FileWriter, IFileSystem, OpenedFile};
use crate::filesystem::{FileReader, FileStat, FileWriter, IFileSystem, OpenedFile, Result};
use crate::filesystem_metadata::{DefaultFileSystemMetadata, IFileSystemMetadata};
use dashmap::DashMap;
use fuse3::Errno;
use std::sync::{Arc, Mutex, RwLock};

// MemoryFileSystem is a simple in-memory filesystem implementation
Expand Down Expand Up @@ -71,27 +70,28 @@ impl IFileSystem for MemoryFileSystem {
meta.get_file_path(file_id)
}

fn get_opened_file(&self, _file_id: u64, fh: u64) -> Option<OpenedFile> {
fn get_opened_file(&self, _file_id: u64, fh: u64) -> Result<OpenedFile> {
let file_handle_map = self.file_handle_manager.read().unwrap();
file_handle_map.get_file(fh)
file_handle_map.get_file(fh).ok_or(libc::ENOENT.into())
}

fn stat(&self, file_id: u64) -> Option<FileStat> {
fn stat(&self, file_id: u64) -> Result<FileStat> {
let meta = self.meta.read().unwrap();
meta.get_file(file_id)
meta.get_file(file_id).ok_or(libc::ENOENT.into())
}

fn lookup(&self, parent_file_id: u64, name: &str) -> Option<FileStat> {
fn lookup(&self, parent_file_id: u64, name: &str) -> Result<FileStat> {
let meta = self.meta.read().unwrap();
meta.get_file_from_dir(parent_file_id, name)
.ok_or(libc::ENOENT.into())
}

fn read_dir(&self, file_id: u64) -> Vec<FileStat> {
fn read_dir(&self, file_id: u64) -> Result<Vec<FileStat>> {
let meta = self.meta.read().unwrap();
meta.get_dir_childs(file_id)
Ok(meta.get_dir_childs(file_id))
}

fn open_file(&self, file_id: u64) -> Result<OpenedFile, Errno> {
fn open_file(&self, file_id: u64) -> Result<OpenedFile> {
let meta = self.meta.read().unwrap();
let file_stat = meta.get_file(file_id);
match file_stat {
Expand All @@ -104,7 +104,7 @@ impl IFileSystem for MemoryFileSystem {
}
}

fn create_file(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile, Errno> {
fn create_file(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile> {
let mut meta = self.meta.write().unwrap();
let file_stat = meta.add_file(parent_file_id, name);

Expand All @@ -116,7 +116,7 @@ impl IFileSystem for MemoryFileSystem {
Ok(file_handle)
}

fn create_dir(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile, Errno> {
fn create_dir(&self, parent_file_id: u64, name: &str) -> Result<OpenedFile> {
let mut meta = self.meta.write().unwrap();
let file_stat = meta.add_dir(parent_file_id, name);

Expand All @@ -125,13 +125,13 @@ impl IFileSystem for MemoryFileSystem {
Ok(file_handle)
}

fn set_attr(&self, file_id: u64, file_info: &FileStat) -> Result<(), Errno> {
fn set_attr(&self, file_id: u64, file_info: &FileStat) -> Result<()> {
Ok(())
}

fn update_file_status(&self, file_id: u64, file_stat: &FileStat) {
fn update_file_status(&self, file_id: u64, file_stat: &FileStat) -> Result<()> {
let mut meta = self.meta.write().unwrap();
meta.update_file_stat(file_id, file_stat)
Ok(meta.update_file_stat(file_id, file_stat))
}

fn read(&self, file_id: u64, fh: u64) -> Box<dyn FileReader> {
Expand Down Expand Up @@ -162,18 +162,18 @@ impl IFileSystem for MemoryFileSystem {
})
}

fn remove_file(&self, parent_file_id: u64, name: &str) -> Result<(), Errno> {
fn remove_file(&self, parent_file_id: u64, name: &str) -> Result<()> {
let mut meta = self.meta.write().unwrap();
meta.remove_file(parent_file_id, name);
Ok(())
}

fn remove_dir(&self, parent_file_id: u64, name: &str) -> Result<(), Errno> {
fn remove_dir(&self, parent_file_id: u64, name: &str) -> Result<()> {
let mut meta = self.meta.write().unwrap();
meta.remove_dir(parent_file_id, name)
}

fn close_file(&self, _file_id: u64, fh: u64) -> Result<(), Errno> {
fn close_file(&self, _file_id: u64, fh: u64) -> Result<()> {
let mut file_handle_manager = self.file_handle_manager.write().unwrap();
file_handle_manager.remove_file(fh);
Ok(())
Expand Down

0 comments on commit a8456cb

Please sign in to comment.