Skip to content

fix(fs): remove tombstones when re-creating deleted files #37

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.

Sign up for GitHub

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

Open
chaserhkj wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(fs): remove tombstones when re-creating deleted files #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion src/fs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,7 @@ impl Filesystem for BranchFs {
use std::os::unix::fs::PermissionsExt;
let perm = std::fs::Permissions::from_mode(mode & !umask);
let _ = std::fs::set_permissions(&delta, perm);
self.revive_path(&branch, &rel_path);
let inode_path = format!("/@{}{}", branch, rel_path);
let ino = self.inodes.get_or_create(&inode_path, false);
if let Some(attr) = self.make_attr(ino, &delta) {
Expand Down Expand Up @@ -1016,6 +1017,7 @@ impl Filesystem for BranchFs {
use std::os::unix::fs::PermissionsExt;
let perm = std::fs::Permissions::from_mode(mode & !umask);
let _ = std::fs::set_permissions(&delta, perm);
self.revive_path(&self.get_branch_name(), &path);
if self.is_stale() {
let _ = std::fs::remove_file(&delta);
reply.error(libc::ESTALE);
Expand Down Expand Up @@ -1282,13 +1284,13 @@ impl Filesystem for BranchFs {
if dst_existed {
b.add_tombstone(&dst_rel)?;
}
b.remove_tombstone(&dst_rel);
Ok(())
});
if result.is_err() {
reply.error(libc::EIO);
return;
}
self.revive_path(&branch, &dst_rel);

if src_is_root && self.is_stale() {
reply.error(libc::ESTALE);
Expand Down Expand Up @@ -1682,6 +1684,7 @@ impl Filesystem for BranchFs {
use std::os::unix::fs::PermissionsExt;
let perm = std::fs::Permissions::from_mode(mode & !umask);
let _ = std::fs::set_permissions(&delta, perm);
self.revive_path(&branch, &rel_path);
let inode_path = format!("/@{}{}", branch, rel_path);
let ino = self.inodes.get_or_create(&inode_path, true);
if let Some(attr) = self.make_attr(ino, &delta) {
Expand Down Expand Up @@ -1710,6 +1713,7 @@ impl Filesystem for BranchFs {
use std::os::unix::fs::PermissionsExt;
let perm = std::fs::Permissions::from_mode(mode & !umask);
let _ = std::fs::set_permissions(&delta, perm);
self.revive_path(&self.get_branch_name(), &path);
if self.is_stale() {
let _ = std::fs::remove_dir_all(&delta);
reply.error(libc::ESTALE);
Expand Down Expand Up @@ -1817,6 +1821,7 @@ impl Filesystem for BranchFs {
}
match std::os::unix::fs::symlink(target, &delta) {
Ok(()) => {
self.revive_path(&branch, &rel_path);
let inode_path = format!("/@{}{}", branch, rel_path);
let ino = self.inodes.get_or_create(&inode_path, false);
match self.make_attr(ino, &delta) {
Expand Down Expand Up @@ -1844,6 +1849,7 @@ impl Filesystem for BranchFs {
}
match std::os::unix::fs::symlink(target, &delta) {
Ok(()) => {
self.revive_path(&self.get_branch_name(), &path);
if self.is_stale() {
let _ = std::fs::remove_file(&delta);
reply.error(libc::ESTALE);
Expand Down
9 changes: 9 additions & 0 deletions src/fs_helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ impl BranchFs {
.unwrap()
}

/// Clear (revive) a tombstone for `rel_path` in the given branch
/// Note that every creation should call this to handle re-creations correctly
pub(crate) fn revive_path(&self, branch: &str, rel_path: &str) {
let _ = self.manager.with_branch(branch, |b| {
b.remove_tombstone(rel_path);
Ok(())
});
}

pub(crate) fn ensure_cow(&self, rel_path: &str) -> std::io::Result<std::path::PathBuf> {
self.ensure_cow_for_branch(&self.get_branch_name(), rel_path)
}
Expand Down
64 changes: 64 additions & 0 deletions tests/test_file_operations.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,66 @@ test_synthetic_entry_ownership() {
}

# Run tests
test_recreate_file_after_delete() {
setup
do_mount
echo "first content" > "$TEST_MNT/recreate.txt"
assert_file_contains "$TEST_MNT/recreate.txt" "first content" "File created with first content"
rm "$TEST_MNT/recreate.txt"
assert_file_not_exists "$TEST_MNT/recreate.txt" "File deleted"
echo "second content" > "$TEST_MNT/recreate.txt"

# readdir forces a fresh resolution through resolve_path (unlike stat, which
# is masked by the kernel's positive dentry cache from create)
assert "ls '$TEST_MNT' | grep -qx 'recreate.txt'" "File recreated with same name visible in listing"
assert_file_contains "$TEST_MNT/recreate.txt" "second content" "Recreated file has second content"

local delta_dir="$TEST_STORAGE/branches/main/files"
local count
count=$(ls "$delta_dir/recreate.txt" 2>/dev/null | wc -l)
assert_eq "$count" "1" "Exactly one delta entry for recreated file (no orphan)"
do_unmount
}

test_recreate_dir_after_delete() {
setup
do_mount
mkdir "$TEST_MNT/recreate_dir"
assert "[[ -d '$TEST_MNT/recreate_dir' ]]" "Directory created"
rmdir "$TEST_MNT/recreate_dir"
assert "[[ ! -d '$TEST_MNT/recreate_dir' ]]" "Directory deleted"
mkdir "$TEST_MNT/recreate_dir"
assert "[[ -d '$TEST_MNT/recreate_dir' ]]" "Directory recreated with same name"
do_unmount
}

test_recreate_symlink_after_delete() {
setup
do_mount
echo "target" > "$TEST_MNT/recreate_target.txt"
ln -s recreate_target.txt "$TEST_MNT/recreate_link"
assert "[[ -L '$TEST_MNT/recreate_link' ]]" "Symlink created"
rm "$TEST_MNT/recreate_link"
assert "[[ ! -L '$TEST_MNT/recreate_link' ]]" "Symlink deleted"
ln -s recreate_target.txt "$TEST_MNT/recreate_link"
assert "[[ -L '$TEST_MNT/recreate_link' ]]" "Symlink recreated with same name"
do_unmount
}

test_recreate_file_after_delete_in_branch() {
setup
do_mount
do_create "recreate_b" "main"
echo "branch first" > "$TEST_MNT/@recreate_b/recreate.txt"
assert_file_contains "$TEST_MNT/@recreate_b/recreate.txt" "branch first" "Branch file created"
rm "$TEST_MNT/@recreate_b/recreate.txt"
assert_file_not_exists "$TEST_MNT/@recreate_b/recreate.txt" "Branch file deleted"
echo "branch second" > "$TEST_MNT/@recreate_b/recreate.txt"
assert_file_exists "$TEST_MNT/@recreate_b/recreate.txt" "Branch file recreated with same name"
assert_file_contains "$TEST_MNT/@recreate_b/recreate.txt" "branch second" "Branch recreated file has second content"
do_unmount
}

run_test "Read Base Files" test_read_base_files
run_test "Write New File in Branch" test_write_new_file_in_branch
run_test "Modify Existing File (COW)" test_modify_existing_file_cow
Expand All @@ -247,5 +307,9 @@ run_test "Chmod File" test_chmod_file
run_test "Chmod Directory" test_chmod_directory
run_test "Chmod Existing File (COW)" test_chmod_existing_file_cow
run_test "Synthetic Entry Ownership" test_synthetic_entry_ownership
run_test "Recreate File After Delete" test_recreate_file_after_delete
run_test "Recreate Dir After Delete" test_recreate_dir_after_delete
run_test "Recreate Symlink After Delete" test_recreate_symlink_after_delete
run_test "Recreate File After Delete in Branch" test_recreate_file_after_delete_in_branch

print_summary