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

Implement statfs with synthetic values #1118

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions mountpoint-s3/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

### Other changes

* Implement statfs to report non-zero synthetic values. This may unblock applications which rely on verifying there is available space before creating new files.([#1118](https://github.com/awslabs/mountpoint-s3/pull/1118)).
* Fix an issue where `fstat` would fail and return `ESTALE` when invoked on a file descriptor after a successful `fsync`. ([#1085](https://github.com/awslabs/mountpoint-s3/pull/1085))

## v1.10.0 (October 15, 2024)
Expand Down
52 changes: 52 additions & 0 deletions mountpoint-s3/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,43 @@ pub struct DirectoryEntry {
lookup: LookedUp,
}

#[derive(Debug)]
/// Reply to a 'statfs' call
pub struct StatFs {
/// Total number of blocks
pub total_blocks: u64,
/// Number of free blocks
pub free_blocks: u64,
/// Number of free blocks available to unprivileged user
pub available_blocks: u64,
/// Number of inodes in file system
pub total_inodes: u64,
/// Available inodes
pub free_inodes: u64,
/// Optimal transfer block size
pub block_size: u32,
/// Maximum name length
pub maximum_name_length: u32,
/// Fragement size
pub fragment_size: u32,
}
Comment on lines +110 to +129
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: didn't catch this last time, but we always put Rustdoc comments above macros (so its clear what macros are applied to the struct)

Suggested change
#[derive(Debug)]
/// Reply to a 'statfs' call
pub struct StatFs {
/// Total number of blocks
pub total_blocks: u64,
/// Number of free blocks
pub free_blocks: u64,
/// Number of free blocks available to unprivileged user
pub available_blocks: u64,
/// Number of inodes in file system
pub total_inodes: u64,
/// Available inodes
pub free_inodes: u64,
/// Optimal transfer block size
pub block_size: u32,
/// Maximum name length
pub maximum_name_length: u32,
/// Fragement size
pub fragment_size: u32,
}
/// Reply to a 'statfs' call
#[derive(Debug)]
pub struct StatFs {
/// Total number of blocks
pub total_blocks: u64,
/// Number of free blocks
pub free_blocks: u64,
/// Number of free blocks available to unprivileged user
pub available_blocks: u64,
/// Number of inodes in file system
pub total_inodes: u64,
/// Available inodes
pub free_inodes: u64,
/// Optimal transfer block size
pub block_size: u32,
/// Maximum name length
pub maximum_name_length: u32,
/// Fragement size
pub fragment_size: u32,
}

non-blocking


impl Default for StatFs {
fn default() -> Self {
// Default values copied from Fuser (https://github.com/cberner/fuser/blob/e18bd9bf9071ecd8be62993726e06ff11d6ec709/src/lib.rs#L695-L698)
Self {
total_blocks: 0,
free_blocks: 0,
available_blocks: 0,
total_inodes: 0,
free_inodes: 0,
block_size: 512,
maximum_name_length: 255,
fragment_size: 0,
}
}
}

impl<Client, Prefetcher> S3Filesystem<Client, Prefetcher>
where
Client: ObjectClient + Clone + Send + Sync + 'static,
Expand Down Expand Up @@ -790,6 +827,21 @@ where
}
Ok(self.superblock.unlink(&self.client, parent_ino, name).await?)
}

pub async fn statfs(&self, _ino: InodeNo) -> Result<StatFs, Error> {
const FREE_BLOCKS: u64 = u64::MAX / 1024;
const FREE_INODES: u64 = u64::MAX / 1024;

let reply = StatFs {
free_blocks: FREE_BLOCKS,
available_blocks: FREE_BLOCKS,
free_inodes: FREE_INODES,
total_blocks: FREE_BLOCKS,
total_inodes: FREE_INODES,
..Default::default()
};
Ok(reply)
}
}

#[cfg(test)]
Expand Down
19 changes: 18 additions & 1 deletion mountpoint-s3/src/fuse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::prefetch::Prefetch;
use fuser::ReplyXTimes;
use fuser::{
Filesystem, KernelConfig, ReplyAttr, ReplyBmap, ReplyCreate, ReplyData, ReplyEmpty, ReplyEntry, ReplyIoctl,
ReplyLock, ReplyLseek, ReplyOpen, ReplyWrite, ReplyXattr, Request, TimeOrNow,
ReplyLock, ReplyLseek, ReplyOpen, ReplyStatfs, ReplyWrite, ReplyXattr, Request, TimeOrNow,
};

pub mod session;
Expand Down Expand Up @@ -580,4 +580,21 @@ where
fn getxtimes(&self, _req: &Request<'_>, ino: u64, reply: ReplyXTimes) {
fuse_unsupported!("getxtimes", reply);
}

#[instrument(level="warn", skip_all, fields(req=_req.unique(), ino=ino))]
fn statfs(&self, _req: &Request<'_>, ino: u64, reply: ReplyStatfs) {
match block_on(self.fs.statfs(ino).in_current_span()) {
Ok(statfs) => reply.statfs(
statfs.total_blocks,
statfs.free_blocks,
statfs.available_blocks,
statfs.total_inodes,
statfs.free_inodes,
statfs.block_size,
statfs.maximum_name_length,
statfs.fragment_size,
),
Err(e) => fuse_error!("statfs", reply, e),
}
}
}
1 change: 1 addition & 0 deletions mountpoint-s3/tests/fuse_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ mod readdir_test;
mod rmdir_test;
mod semantics_doc_test;
mod setattr_test;
mod statfs_test;
mod unlink_test;
mod write_test;
82 changes: 82 additions & 0 deletions mountpoint-s3/tests/fuse_tests/statfs_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
use crate::common::fuse::{self, TestSessionCreator};
use test_case::test_case;

/// Tests that non-zero empty space is reported
fn statfs_test_available_nonzero(creator_fn: impl TestSessionCreator, prefix: &str) {
let test_session = creator_fn(prefix, Default::default());
let mount_dir = test_session.mount_path();
let stats = nix::sys::statvfs::statvfs(mount_dir.into()).unwrap();

Check failure on line 8 in mountpoint-s3/tests/fuse_tests/statfs_test.rs

View workflow job for this annotation

GitHub Actions / Clippy

useless conversion to the same type: `&std::path::Path`
assert_ne!(stats.blocks_free(), 0);
assert_ne!(stats.blocks_available(), 0);
assert_ne!(stats.blocks(), 0);
}

/// Tests that default values from FUSER are reported for mpst fields
Copy link
Contributor

Choose a reason for hiding this comment

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

What is mpst, is it typo for most?

fn statfs_test_fuser_defaults(creator_fn: impl TestSessionCreator, prefix: &str) {
let test_session = creator_fn(prefix, Default::default());
let mount_dir = test_session.mount_path();
let stats = nix::sys::statvfs::statvfs(mount_dir.into()).unwrap();

Check failure on line 18 in mountpoint-s3/tests/fuse_tests/statfs_test.rs

View workflow job for this annotation

GitHub Actions / Clippy

useless conversion to the same type: `&std::path::Path`
//assert_eq!(stats.name_max(), 255);
Copy link
Contributor

Choose a reason for hiding this comment

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

drop commented out code

// These five aren't default values but set by us, so maybe drop
assert_eq!(stats.blocks(), u64::MAX / 1024);
assert_eq!(stats.blocks_free(), u64::MAX / 1024);
assert_eq!(stats.blocks_available(), u64::MAX / 1024);
assert_eq!(stats.files(), u64::MAX / 1024);
assert_eq!(stats.files_available(), u64::MAX / 1024);
Comment on lines +20 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd drop these, but actually maybe we just combine with the test above. No need to separate them.

// These are default values from the Default implementation
assert_eq!(stats.block_size(), 512);
assert_eq!(stats.name_max(), 255);
println!("{}", stats.fragment_size().to_string());

Check failure on line 29 in mountpoint-s3/tests/fuse_tests/statfs_test.rs

View workflow job for this annotation

GitHub Actions / Clippy

`to_string` applied to a type that implements `Display` in `println!` args
// This may be a bit surprising, however as we set fsize to 0,
// it will be automatically set to the block_size, if it is not available
// c.f. https://stackoverflow.com/questions/54823541/what-do-f-bsize-and-f-frsize-in-struct-statvfs-stand-for
assert_eq!(stats.fragment_size(), 512);
dannycjones marked this conversation as resolved.
Show resolved Hide resolved
}

/// Test that total blocks >= blocks_free,
/// as some tools rely on calculations with these values to determine percentage of blocks available
fn statfs_test_block_arithmetic(creator_fn: impl TestSessionCreator, prefix: &str) {
let test_session = creator_fn(prefix, Default::default());
let mount_dir = test_session.mount_path();
let stats = nix::sys::statvfs::statvfs(mount_dir.into()).unwrap();

Check failure on line 41 in mountpoint-s3/tests/fuse_tests/statfs_test.rs

View workflow job for this annotation

GitHub Actions / Clippy

useless conversion to the same type: `&std::path::Path`
assert!(stats.blocks() >= stats.blocks_available());
}

#[test_case(""; "no prefix")]
#[test_case("statfs_report_nonzero_test"; "prefix")]
fn statfs_report_nonzero_test_mock(prefix: &str) {
statfs_test_available_nonzero(fuse::mock_session::new, prefix);
}

#[cfg(feature = "s3_tests")]
#[test_case(""; "no prefix")]
#[test_case("statfs_report_nonzero_test"; "prefix")]
fn statfs_report_nonzero_s3(prefix: &str) {
statfs_test_available_nonzero(fuse::s3_session::new, prefix);
}

#[test_case(""; "no prefix")]
#[test_case("statfs_report_fuser_defaults_test"; "prefix")]
fn statfs_report_fuser_defaults_mock(prefix: &str) {
statfs_test_fuser_defaults(fuse::mock_session::new, prefix);
}

#[cfg(feature = "s3_tests")]
#[test_case(""; "no prefix")]
#[test_case("statfs_report_nonzero_test"; "prefix")]
fn statfs_report_fuser_defaults_s3(prefix: &str) {
statfs_test_available_nonzero(fuse::s3_session::new, prefix);
}
Comment on lines +64 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

test case is wrong

Suggested change
#[cfg(feature = "s3_tests")]
#[test_case(""; "no prefix")]
#[test_case("statfs_report_nonzero_test"; "prefix")]
fn statfs_report_fuser_defaults_s3(prefix: &str) {
statfs_test_available_nonzero(fuse::s3_session::new, prefix);
}
#[cfg(feature = "s3_tests")]
#[test_case(""; "no prefix")]
#[test_case("statfs_report_fuser_defaults"; "prefix")]
fn statfs_report_fuser_defaults_s3(prefix: &str) {
statfs_report_fuser_defaults(fuse::s3_session::new, prefix);
}


#[test_case(""; "no prefix")]
#[test_case("statfs_block_arithmetic_test"; "prefix")]
fn statfs_block_arithmetic_mock(prefix: &str) {
statfs_test_block_arithmetic(fuse::mock_session::new, prefix);
}

#[cfg(feature = "s3_tests")]
#[test_case(""; "no prefix")]
#[test_case("statfs_block_arithmetic_test"; "prefix")]
fn statfs_block_arithmetic_s3(prefix: &str) {
statfs_test_block_arithmetic(fuse::s3_session::new, prefix);
}
Loading