Skip to content
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.

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

torture: trickfs #765

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

torture: trickfs #765

wants to merge 1 commit into from

Conversation

pepyakin
Copy link
Contributor

Introduces trickfs, a fuse filesystem for made for failure injection.

Copy link
Contributor Author

pepyakin commented Jan 30, 2025

@pepyakin pepyakin changed the base branch from master to graphite-base/765 January 30, 2025 15:44
@pepyakin pepyakin changed the base branch from graphite-base/765 to pep-ci-install-fuse January 30, 2025 15:44
@pepyakin pepyakin force-pushed the pep-ci-install-fuse branch from 69f9ed1 to c1e1c91 Compare January 30, 2025 17:38
@pepyakin pepyakin marked this pull request as ready for review January 30, 2025 18:02
Copy link
Contributor

@gabriele-0201 gabriele-0201 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside some missing comments on unwraps, it looks very nice!

//
// One safety note here is that `ptr` is allocated with `MAP_NORESERVE` flag, so
// the memory is only "committed" upon the first access and if there is not actual
// physical memory to back it up, the kernel will kill the process.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that even if we are mmapping the entirety of the file, the os will allocate only a portion of the physical memory only at the time it is being written, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, essentially.

But I think the documentation sucks here, it is a lazy explaination: memory mapped regions start their life unbacked by phyiscal pages either way and only get backed by one upon the first access.

However, by default the system reserves the swap space for the mapping and we are doing here 1 TiB, which is too much!

return;
};
// TODO: this is definitely wrong wrt sign ext.
let offset = offset as usize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see how it could be wrong with a negative offset, but I don't really see why the offset is an i64. Is it to support a Seek mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why would it be this way. Probably some idiosynchronies of unix. I don't think this is seek because that, I would imagine, should be handled on another level.

The comment is wrong, it seems like I assumed that the offset is i32. No sign extension should happen here.

panic!("container entry is not registered")
}
Some(inode_data) if inode_data.is_dir() => {
panic!("unexpected nested directory");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why there can't be nested directories? It doesn't seem to be not permitted on creation, but the opposite, mkdir requires the parent to be a directory.

The weird thing is that the following test passes. Does fuse handle recursive dir deletion on its own?

    #[test]
    fn rmdir() {
        init_log();
        let mountpoint = tempfile::tempdir().unwrap();
        let options = &[
            MountOption::RW,
            MountOption::AutoUnmount,
            MountOption::FSName("trick".to_string()),
        ];
        let fs = Trick::new();
        let mount_handle = fuser::spawn_mount2(fs, &mountpoint, options).unwrap();

        let dirname1 = mountpoint.path().join("dir_to_remove");
        let dirname2 = dirname1.clone().join("dir_within_dir");
        let _ = fs::create_dir(&dirname1).unwrap();
        let _ = fs::create_dir(&dirname2).unwrap();
        assert!(dirname1.clone().exists());
        assert!(dirname2.clone().exists());

        fs::remove_dir_all(&dirname1).unwrap();
        assert!(!dirname1.exists());
        assert!(!dirname2.exists());
        drop(mount_handle);
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah, it's not fuse though: I think this is the contract between VFS & FS!

@pepyakin pepyakin changed the base branch from pep-ci-install-fuse to graphite-base/765 January 31, 2025 14:36
@pepyakin pepyakin changed the base branch from graphite-base/765 to master January 31, 2025 14:37
Introduces trickfs, a fuse filesystem for made for failure injection.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants