Skip to content

fix(storage): reject invalid device and engine configs - #1301

Open
mjc wants to merge 2 commits into
foyer-rs:mainfrom
mjc:mjc/storage-guards
Open

fix(storage): reject invalid device and engine configs#1301
mjc wants to merge 2 commits into
foyer-rs:mainfrom
mjc:mjc/storage-guards

Conversation

@mjc

@mjc mjc commented Jun 16, 2026

Copy link
Copy Markdown

Move default-capacity discovery and storage-engine validation out of panic paths and into fallible, type-driven APIs.

What changed:

  • FileDeviceBuilder now probes block-device capacity or parent-directory free space through fallible helpers instead of unwrap/expect.
  • FsDeviceBuilder now computes default capacity lazily and returns Error if filesystem probing fails.
  • BlockEngineConfig and UringIoEngineConfig now use NonZeroUsize for counts and depths, so zero values are rejected at compile time for normal callers.
  • CPU pinning now uses nonempty CPU sets to set thread count, leaves pinning off when the CPU set is empty, and rejects mismatched with_cpus(...) / with_threads(...) combinations up front.

Tests added:

  • test_file_device_builder_rejects_path_without_parent
  • test_fs_device_builder_propagates_dir_creation_errors
  • test_uring_empty_cpu_sets_disable_pinning
  • test_uring_rejects_mismatched_cpu_count

@mjc
mjc requested review from MrCroxx, Xuanwo and tisonkun as code owners June 16, 2026 04:11
@mjc
mjc force-pushed the mjc/storage-guards branch from 5dc261a to 890592b Compare June 16, 2026 04:17

mjc commented Jun 16, 2026

Copy link
Copy Markdown
Author

Tightened the io_uring CPU affinity path a bit more: a nonempty CPU list still sets the thread count, but we now reject mismatched with_cpus(...) / with_threads(...) combinations up front instead of letting them reach an index panic in build(). Added tests for both the mismatch and the empty-list “disable pinning” case.

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.58621% with 18 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
foyer-storage/src/io/engine/uring.rs 88.40% 8 Missing ⚠️
foyer-storage/src/io/device/file.rs 77.27% 5 Missing ⚠️
foyer-bench/src/main.rs 50.00% 1 Missing ⚠️
foyer-storage/src/engine/block/engine.rs 96.66% 1 Missing ⚠️
foyer-storage/src/io/device/fs.rs 88.88% 1 Missing ⚠️
foyer-storage/src/io/engine/mod.rs 85.71% 1 Missing ⚠️
foyer-storage/src/store.rs 83.33% 1 Missing ⚠️
Files with missing lines Coverage Δ
foyer-bench/src/main.rs 70.02% <50.00%> (+0.22%) ⬆️
foyer-storage/src/engine/block/engine.rs 93.46% <96.66%> (-0.08%) ⬇️
foyer-storage/src/io/device/fs.rs 92.30% <88.88%> (+0.47%) ⬆️
foyer-storage/src/io/engine/mod.rs 94.66% <85.71%> (-1.05%) ⬇️
foyer-storage/src/store.rs 86.33% <83.33%> (-0.10%) ⬇️
foyer-storage/src/io/device/file.rs 70.17% <77.27%> (+7.31%) ⬆️
foyer-storage/src/io/engine/uring.rs 87.81% <88.40%> (+0.85%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mjc

mjc commented Jun 16, 2026

Copy link
Copy Markdown
Author

I'll be back at this in a couple days to add more tests if y'all need, I try not to leave drive-by PRs. just ran out of time.

--

我过几天会再回来继续补测试,如果你们需要的话。我尽量不留那种丢完就跑的 PR,只是这次实在没时间了。

mjc added 2 commits June 19, 2026 09:31
Signed-off-by: Mika Cohen <mjc@kernel.org>
Signed-off-by: Mika Cohen <mjc@kernel.org>
@mjc
mjc force-pushed the mjc/storage-guards branch from 8699c15 to 5d4fded Compare June 19, 2026 15:31
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.

1 participant