Skip to content

file_scan: refuse unsupported-fs roots up front instead of failing silently - #97

Merged
martinus merged 1 commit into
masterfrom
loud-unsupported-fs
Jul 22, 2026
Merged

file_scan: refuse unsupported-fs roots up front instead of failing silently#97
martinus merged 1 commit into
masterfrom
loud-unsupported-fs

Conversation

@martinus

Copy link
Copy Markdown
Owner

Pre-launch robustness: make the "pointed oans at the wrong filesystem" case
honest, since it's a predictable first-touch mistake for new users.

The problem (observed)

Point oans at a directory on ext4/tmpfs and, on a modern kernel where
FS_IOC_GETFSUUID succeeds, it would:

  1. print a single Warn: filesystem ... is not known to support deduplication
    (easily lost in progress output),
  2. walk the tree anyway, hashing every file,
  3. fail every extent op with per-file fiemap: Operation not supported spam,
  4. and still exit 0 with Nothing to deduplicate — looking like success.

(The other case — where get_uuid outright fails, e.g. older kernels — was
already handled loudly by filescan_seed_failed(); this fixes the modern
"succeeds but unsupported" gap.)

The fix

In check_file(), when the identified fs isn't btrfs/XFS, refuse the seed root
via seed_reject() with a clear Skipping X: its filesystem is not btrfs or XFS message instead of returning true. Consequences:

  • All roots unsupported → the existing filescan_seed_failed() path fires:
    loud error, exit 1, and no FIEMAP spam (the tree is never walked).
  • Mixed → supported roots scan normally; unsupported ones are skipped with
    the message.
  • locked_fs is now committed only after the support check (the UUID lands
    in a local first), so a rejected root can't pollute the fs-lock for a later
    supported root.

Behaviour change to note

A filesystem that reports a UUID but isn't btrfs or XFS is now skipped
rather than attempted. oans only deduplicates btrfs/XFS (via FIDEDUPERANGE),
so this converts a misleading exit-0 "success" into an honest, actionable
failure. If we ever want to support another dedupe-capable fs, add its magic to
is_fs_supported().

Before / after (oans -dr on a tmpfs dir)

Before: Warn: + fiemap: Operation not supported ×N + Nothing to deduplicate, exit 0.
After: Skipping <dir>: its filesystem is not btrfs or XFS ... + Error: could not determine the filesystem for any given path ..., exit 1, no FIEMAP spam.

Testing

  • New tests/integration/test_unsupported_fs.py (skips when the system temp
    dir happens to be a reflink fs).
  • scripts/verify.sh green: clean build, 91 tests, valgrind scan+dedupe+replay
    smoke clean.

🤖 Generated with Claude Code

…lently

When a seed root lived on a filesystem oans can't deduplicate (not btrfs or
XFS), check_file() warned once but returned true, so the walk hashed every
file only to have each FIEMAP/FIDEDUPERANGE fail with confusing per-file
"Operation not supported" errors — and the run still exited 0 with "Nothing to
deduplicate", looking like success.

Refuse such a root at seed time via seed_reject() and print a clear "Skipping
X: its filesystem is not btrfs or XFS" line. A run whose roots are *all*
unsupported now trips the existing filescan_seed_failed() path (loud error,
exit 1, no FIEMAP spam); a mix still scans the supported roots. locked_fs is
committed only once the fs is confirmed supported, so a rejected root can't
pollute the lock for a later supported root (the uuid now lands in a local
first).

This changes one behaviour: a filesystem that reports a UUID but isn't btrfs or
XFS is now skipped rather than attempted. oans only deduplicates btrfs/XFS, so
this turns a misleading success into an honest, actionable failure.

Adds test_unsupported_fs.py (skips when the system temp dir happens to be a
reflink fs). verify.sh green (91 tests + valgrind smoke).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martinus
martinus force-pushed the loud-unsupported-fs branch from 2447ea9 to 288d5ea Compare July 22, 2026 12:21
@martinus

Copy link
Copy Markdown
Owner Author

Ran a /simplify pass and applied its findings:

  • test now reuses the harness scratch_fstype() helper instead of a local duplicate (dropped the redundant subprocess import and the double probe);
  • reworded the generic end-of-run error in oans.c from "could not determine the filesystem…" to "none of the given paths are on a filesystem oans can deduplicate…", since with this change we now often have determined the fs — it's just unsupported.

The core check_file change was confirmed at the right depth (reuses seed_reject/filescan_seed_failed, commits locked_fs only after the support check). verify.sh still green (91 tests + valgrind).

@martinus
martinus merged commit 4b947a8 into master Jul 22, 2026
4 checks passed
@martinus
martinus deleted the loud-unsupported-fs branch July 22, 2026 12:22
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