Skip to content

Commit

Permalink
Merge pull request #4702 from wasmerio/redirect-when-renaming
Browse files Browse the repository at this point in the history 
Rename when redirect is to the same fs
  • Loading branch information
@Arshia001
Arshia001 authored May 15, 2024
2 parents f410690 + b90536f commit 930abdf
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 86 deletions.
214 changes: 129 additions & 85 deletions lib/virtual-fs/src/mem_fs/filesystem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use self::offloaded_file::OffloadBackingStore;

use super::*;
use crate::{DirEntry, FileSystem as _, FileType, FsError, Metadata, OpenOptions, ReadDir, Result};
use futures::future::BoxFuture;
use futures::future::{BoxFuture, Either};
use slab::Slab;
use std::collections::VecDeque;
use std::convert::identity;
Expand Down Expand Up @@ -438,12 +438,8 @@ impl crate::FileSystem for FileSystem {
Box::pin(async {
let name_of_to;

let (
(position_of_from, inode, inode_of_from_parent),
(inode_of_to_parent, name_of_to),
inode_dest,
) = {
// Read lock.
// Read lock.
let (name_of_from, inode_of_from_parent, name_of_to, inode_of_to_parent) = {
let fs = self.inner.read().map_err(|_| FsError::Lock)?;

let from = fs.canonicalize_without_inode(from)?;
Expand All @@ -462,85 +458,119 @@ impl crate::FileSystem for FileSystem {

// Find the parent inodes.
let inode_of_from_parent = match fs.inode_of_parent(parent_of_from)? {
InodeResolution::Found(a) => a,
InodeResolution::Redirect(..) => {
return Err(FsError::InvalidInput);
InodeResolution::Found(a) => Either::Left(a),
InodeResolution::Redirect(fs, mut path) => {
path.push(&name_of_from);
Either::Right((fs, path))
}
};
let inode_of_to_parent = match fs.inode_of_parent(parent_of_to)? {
InodeResolution::Found(a) => a,
InodeResolution::Redirect(..) => {
return Err(FsError::InvalidInput);
InodeResolution::Found(a) => Either::Left(a),
InodeResolution::Redirect(fs, mut path) => {
path.push(&name_of_to);
Either::Right((fs, path))
}
};

// Find the inode of the dest file if it exists
let maybe_position_and_inode_of_file =
fs.as_parent_get_position_and_inode_of_file(inode_of_to_parent, &name_of_to)?;

// Get the child indexes to update in the parent nodes, in
// addition to the inode of the directory to update.
let (position_of_from, inode) = fs
.as_parent_get_position_and_inode(inode_of_from_parent, &name_of_from)?
.ok_or(FsError::EntryNotFound)?;

(
(position_of_from, inode, inode_of_from_parent),
(inode_of_to_parent, name_of_to),
maybe_position_and_inode_of_file,
name_of_from,
inode_of_from_parent,
name_of_to,
inode_of_to_parent,
)
};

let inode = match inode {
InodeResolution::Found(a) => a,
InodeResolution::Redirect(..) => {
return Err(FsError::InvalidInput);
}
};

{
// Write lock.
let mut fs = self.inner.write().map_err(|_| FsError::Lock)?;

if let Some((position, inode_of_file)) = inode_dest {
// Remove the file from the storage.
match inode_of_file {
InodeResolution::Found(inode_of_file) => {
fs.storage.remove(inode_of_file);
}
match (inode_of_from_parent, inode_of_to_parent) {
(Either::Left(inode_of_from_parent), Either::Left(inode_of_to_parent)) => {
let fs = self.inner.read().map_err(|_| FsError::Lock)?;

// Find the inode of the dest file if it exists
let maybe_position_and_inode_of_file = fs
.as_parent_get_position_and_inode_of_file(
inode_of_to_parent,
&name_of_to,
)?;

// Get the child indexes to update in the parent nodes, in
// addition to the inode of the directory to update.
let (position_of_from, inode) = fs
.as_parent_get_position_and_inode(inode_of_from_parent, &name_of_from)?
.ok_or(FsError::EntryNotFound)?;

let (
(position_of_from, inode, inode_of_from_parent),
(inode_of_to_parent, name_of_to),
inode_dest,
) = (
(position_of_from, inode, inode_of_from_parent),
(inode_of_to_parent, name_of_to),
maybe_position_and_inode_of_file,
);

let inode = match inode {
InodeResolution::Found(a) => a,
InodeResolution::Redirect(..) => {
return Err(FsError::InvalidInput);
}
}
};

fs.remove_child_from_node(inode_of_to_parent, position)?;
}
drop(fs);

{
// Write lock.
let mut fs = self.inner.write().map_err(|_| FsError::Lock)?;

if let Some((position, inode_of_file)) = inode_dest {
// Remove the file from the storage.
match inode_of_file {
InodeResolution::Found(inode_of_file) => {
fs.storage.remove(inode_of_file);
}
InodeResolution::Redirect(..) => {
return Err(FsError::InvalidInput);
}
}

fs.remove_child_from_node(inode_of_to_parent, position)?;
}

// Update the file name, and update the modified time.
fs.update_node_name(inode, name_of_to)?;
// Update the file name, and update the modified time.
fs.update_node_name(inode, name_of_to)?;

// The parents are different. Let's update them.
if inode_of_from_parent != inode_of_to_parent {
// Remove the file from its parent, and update the
// modified time.
fs.remove_child_from_node(inode_of_from_parent, position_of_from)?;
// The parents are different. Let's update them.
if inode_of_from_parent != inode_of_to_parent {
// Remove the file from its parent, and update the
// modified time.
fs.remove_child_from_node(inode_of_from_parent, position_of_from)?;

// Add the file to its new parent, and update the modified
// time.
fs.add_child_to_node(inode_of_to_parent, inode)?;
// Add the file to its new parent, and update the modified
// time.
fs.add_child_to_node(inode_of_to_parent, inode)?;
}
// Otherwise, we need to at least update the modified time of the parent.
else {
let mut inode = fs.storage.get_mut(inode_of_from_parent);
match inode.as_mut() {
Some(Node::Directory(node)) => node.metadata.modified = time(),
Some(Node::ArcDirectory(node)) => node.metadata.modified = time(),
_ => return Err(FsError::UnknownError),
}
}
}

Ok(())
}
// Otherwise, we need to at least update the modified time of the parent.
else {
let mut inode = fs.storage.get_mut(inode_of_from_parent);
match inode.as_mut() {
Some(Node::Directory(node)) => node.metadata.modified = time(),
Some(Node::ArcDirectory(node)) => node.metadata.modified = time(),
_ => return Err(FsError::UnknownError),
(Either::Right((from_fs, from_path)), Either::Right((to_fs, to_path))) => {
if Arc::ptr_eq(&from_fs, &to_fs) {
let same_fs = from_fs;

same_fs.rename(&from_path, &to_path).await
} else {
Err(FsError::InvalidInput)
}
}
_ => Err(FsError::InvalidInput),
}

Ok(())
})
}

Expand Down Expand Up @@ -1257,6 +1287,10 @@ mod test_filesystem {
async fn test_rename() {
let fs = FileSystem::default();

let fs2 = FileSystem::default();
fs2.create_dir(path!("/boz")).unwrap();
let arc_fs2: Arc<dyn crate::FileSystem + Send + Sync> = Arc::new(fs2);

assert_eq!(
fs.rename(path!("/"), path!("/bar")).await,
Err(FsError::BaseNotDirectory),
Expand All @@ -1280,30 +1314,29 @@ mod test_filesystem {
assert_eq!(fs.create_dir(path!("/bar")), Ok(()));

assert!(
matches!(
fs.new_open_options()
.write(true)
.create_new(true)
.open(path!("/bar/hello1.txt")),
Ok(_),
),
fs.new_open_options()
.write(true)
.create_new(true)
.open(path!("/bar/hello1.txt"))
.is_ok(),
"creating a new file (`hello1.txt`)",
);
assert!(
matches!(
fs.new_open_options()
.write(true)
.create_new(true)
.open(path!("/bar/hello2.txt")),
Ok(_),
),
fs.new_open_options()
.write(true)
.create_new(true)
.open(path!("/bar/hello2.txt"))
.is_ok(),
"creating a new file (`hello2.txt`)",
);

fs.mount(path!(buf "/mnt"), &arc_fs2, path!(buf "/"))
.unwrap();

{
let fs_inner = fs.inner.read().unwrap();

assert_eq!(fs_inner.storage.len(), 6, "storage has all files");
assert_eq!(fs_inner.storage.len(), 7, "storage has all files");
assert!(
matches!(
fs_inner.storage.get(ROOT_INODE),
Expand All @@ -1312,9 +1345,9 @@ mod test_filesystem {
name,
children,
..
})) if name == "/" && children == &[1, 3]
})) if name == "/" && children == &[1, 3, 6]
),
"`/` contains `foo` and `bar`",
"`/` contains `foo` and `bar` and `mnt`",
);
assert!(
matches!(
Expand Down Expand Up @@ -1389,6 +1422,17 @@ mod test_filesystem {
"renaming a directory",
);

assert_eq!(
fs.rename(path!("/mnt/boz"), path!("/mnt/cat")).await,
Ok(()),
"renaming a directory within a mounted FS"
);

assert!(
arc_fs2.read_dir(path!("/cat")).is_ok(),
"The directory was renamed in the inner FS"
);

assert_eq!(
fs.rename(path!("/bar/hello1.txt"), path!("/bar/world1.txt"))
.await,
Expand All @@ -1403,7 +1447,7 @@ mod test_filesystem {

assert_eq!(
fs_inner.storage.len(),
6,
7,
"storage has still all directories"
);
assert!(
Expand All @@ -1414,9 +1458,9 @@ mod test_filesystem {
name,
children,
..
})) if name == "/" && children == &[3]
})) if name == "/" && children == &[3, 6]
),
"`/` contains `bar`",
"`/` contains `bar` and `mnt`",
);
assert!(
matches!(
Expand Down
6 changes: 6 additions & 0 deletions tests/wasi-fyi/fs_rename-file.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,10 @@ fn main() {
let new_path = "/fyi/fs_rename-file.dir/new_file";

assert!(fs::rename(old_path, new_path).is_ok());

let error = fs::metadata(old_path).unwrap_err();
assert_eq!(error.kind(), std::io::ErrorKind::NotFound);

let metadata = fs::metadata(new_path).unwrap();
assert!(metadata.is_file());
}
2 changes: 1 addition & 1 deletion tests/wasi-fyi/test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ bash build.sh
status=0

# Define skip list as an array
SKIP_LIST=("ported_readlink.wasm" "fs_create_dir-existing-directory.wasm" "fs_rename-directory.wasm" "fs_rename-file.wasm")
SKIP_LIST=("ported_readlink.wasm" "fs_create_dir-existing-directory.wasm")

# List and process .foo files
for file in *.wasm; do
Expand Down

0 comments on commit 930abdf

Please sign in to comment.