(Draft) Gvfs-fuse implementation and code structure layout #5738
Conversation
| futures-util = "0.3.30" | ||
| fuse3 = { version = "0.8.1", "features" = ["tokio-runtime", "unprivileged"] } |
There was a problem hiding this comment.
Yes, the thread mode of fuse3 is better than fuser. It uses multiple threads with asynchronous I/O.
There was a problem hiding this comment.
does fuse3 have some disadvantages?
There was a problem hiding this comment.
FUSE3 is more modern, with fewer legacy burdens, but it has a smaller user base. On the other hand, fuser has been around for a long time, its implementation and technologies are older, but it has a larger user base, making it more stable in practice.
I'm not sure which one would be better to use right now. What do you think?
There was a problem hiding this comment.
My concern about FUSE3 is the stability and smaller user community. It may hard to resolve if encountering some underlying problems.
There was a problem hiding this comment.
Do you think we should use fuser?
diqiu50
force-pushed
the
gvfs-2
branch
2 times, most recently
from
a8456cb to
cd11e47
Compare
| /// 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; |
There was a problem hiding this comment.
add async to all interfaces?
There was a problem hiding this comment.
add Result to wrap errors for all interfaces?
There was a problem hiding this comment.
We'll consider adding async later.
There was a problem hiding this comment.
Yes， all the interface wrap by Resut<T, Errno>
|
How about could split this PR into sevral parts:
|
| } | ||
|
|
||
| fn add_dir_with_name(&mut self, parent: &str, name: &str) -> FileStat { | ||
| let parent_inode = self.dir_name_map.get(parent).unwrap().clone(); |
There was a problem hiding this comment.
it's not recommended to unwrap directly for all codes
There was a problem hiding this comment.
Ok, I understand. I will only use unwrap when it is guaranteed to succeed.
Let's wait until the interfaces stabilize before deciding whether to split the PR. Currently, this is a minimal implementation that I need to use to verify if the structure is reasonable. |
| - name: Build and test Gravitino | ||
| run: | | ||
| ./gradlew :clients:filesystem-fuse:build -PenableFuse=true | ||
|
|
||
| - name: Package Gravitino | ||
| run: | | ||
| ./gradlew compileDistribution -x test -PjdkVersion=${{ matrix.java-version }} -PenableFuse=true |
There was a problem hiding this comment.
why the compileDistribution after Build and test Gravitino ?
There was a problem hiding this comment.
compileDistribution is use to running the integration test, build and test only run uts
There was a problem hiding this comment.
do you mean you mix fuseIT in compileDistribution?
There was a problem hiding this comment.
No, fuseITs depend on the package of compileDistribution
There was a problem hiding this comment.
what's the function of -PenableFuse=true here
There was a problem hiding this comment.
plz remove this and the below steps since it's unnecessary now
| where | ||
| T: 'a; | ||
|
|
||
| async fn readdirplus<'a>( |
There was a problem hiding this comment.
What‘s the difference between readdirplus and readdir?
I could have a better name for readdirplus function.
There was a problem hiding this comment.
readdirplus is a new api of fuse. it has better performance
| Ok(()) | ||
| } | ||
|
|
||
| async fn rmdir(&self, req: Request, parent: Inode, name: &OsStr) -> fuse3::Result<()> { |
There was a problem hiding this comment.
Please change the function name to rm_dir
There was a problem hiding this comment.
The function names are the trait of fues3::Filesystem. we can't change them
diqiu50
changed the title
diqiu50
changed the title
What changes were proposed in this pull request?
Why are the changes needed?
Fix: #5734
Does this PR introduce any user-facing change?
No
How was this patch tested?
IT and UT