metaslab: Repair overlapping space map entries automatically - #18852
metaslab: Repair overlapping space map entries automatically#18852jameshilliard wants to merge 1 commit into
Conversation
d4d1b5f to
30b9839
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a conservative, automatic repair path for metaslab free-space map replay so pools don’t panic during metaslab_load() when on-disk space map entries are duplicate or partially overlapping. The approach quarantines ambiguous ranges (never making them allocatable), updates accounting accordingly, and persists the repaired canonical state via condensation, with new diagnostics and test-only injection plus regression/unit coverage.
Changes:
- Add checked range-tree operations (
try_add/try_remove) and a newspace_map_load_length_repair()path to conservatively replay/repair metaslab free-space maps and track repair details. - Teach metaslab loading/accounting to handle deferred/unloadable metaslabs and to persist conservative repaired state via condensation + history/kstat reporting.
- Add
zhack metaslab corruptinjection and new ZTS + unit tests to cover repair/quarantine behavior and range-tree checked operations.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/zfs-tests/tests/Makefile.am | Registers the new metaslab repair ZTS test script. |
| tests/zfs-tests/tests/functional/cli_root/zhack/zhack_metaslab_repair.ksh | New functional test validating repair/quarantine behavior and data preservation. |
| tests/unit/test_range_tree.c | New unit tests covering checked range-tree insertion/removal semantics. |
| tests/unit/Makefile.am | Builds and links the new test_range_tree unit test binary. |
| tests/unit/.gitignore | Ignores the new unit test binary. |
| tests/runfiles/common.run | Adds zhack_metaslab_repair to the default ZTS runfile set. |
| module/zfs/vdev_trim.c | Makes TRIM paths tolerate metaslab load failures (skip/abort/suspend instead of asserting). |
| module/zfs/vdev_raidz.c | Makes RAID-Z expansion copy conservatively when metaslab free map can’t be loaded. |
| module/zfs/vdev_initialize.c | Makes initialize paths tolerate metaslab load failures (abort/suspend instead of asserting). |
| module/zfs/space_map.c | Implements conservative repair-capable free-space map replay and repair result reporting. |
| module/zfs/spa_log_spacemap.c | Avoids strict debug-load/assertion paths and adjusts accounting behavior for special metaslab load states. |
| module/zfs/range_tree.c | Refactors add/remove internals to support non-panicking “try” operations for overlap/missing-range detection. |
| module/zfs/metaslab.c | Core metaslab repair/quarantine/defer logic, accounting reconciliation, new kstats, and history logging hooks. |
| include/sys/space_map.h | Adds space_map_load_result_t and declares space_map_load_length_repair(). |
| include/sys/range_tree.h | Declares zfs_range_tree_try_add() / zfs_range_tree_try_remove() and documents semantics. |
| include/sys/metaslab_impl.h | Adds metaslab load-state tracking and retained repair metadata fields. |
| cmd/zhack.c | Adds zhack metaslab corrupt for test-only injection of overlap/summary corruption patterns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
30b9839 to
3e133fa
Compare
3e133fa to
e23dfc5
Compare
e23dfc5 to
1b48eb8
Compare
1b48eb8 to
065065a
Compare
065065a to
3509fa8
Compare
3509fa8 to
e9cb83b
Compare
e9cb83b to
dbc05de
Compare
dbc05de to
90bf640
Compare
90bf640 to
40785b8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/zfs-tests/tests/functional/cli_root/zhack/zhack_metaslab_repair.ksh:173
mktemp -dwithout a template is not portable (commonly fails on BSD/macOS where a template or-tis required). This can cause the test to fail before running any assertions. Use a portablemktempinvocation with an explicit template (and consider wrapping it inlog_must).
tmpdir=$(mktemp -d)
tests/unit/test_range_tree.c:144
- This declares
retafter a statement, which can break builds when compiled with stricter C dialects or with-Werror=declaration-after-statement(commonly enabled in this project/toolchains). Moveint ret;to the start of the block to avoid a build failure.
int
main(int argc, char **argv)
{
zfs_btree_init();
int ret = munit_suite_main(&range_tree_test_suite, NULL, argc, argv);
zfs_btree_fini();
return (ret);
}
40785b8 to
27d29df
Compare
A duplicate or partially overlapping space map entry currently trips a range tree assertion while loading a metaslab, making the pool unusable. Add all-or-nothing range-tree operations and use them when replaying metaslab space maps. If an operation conflicts with reconstructed state, keep its complete range allocated for the rest of replay. Reconstruct invalid allocation summaries and condense repaired maps into a canonical conservative representation. Account metaslabs with invalid allocation summaries as fully allocated until they can be reconstructed, and schedule them for repair during import. Defer repair until pending TXG and log-space-map state has drained, then retry it explicitly. Keep transient I/O errors retryable and quarantine metaslabs whose maps cannot be reconstructed safely. Reject malformed entries during repair replay while preserving strict assertions for normal space-map consumers. Record completed repairs in pool history and expose aggregate repair kstats. Make trim, initialize, and RAID-Z expansion paths handle load failures without asserting or reporting false completion. Add unit coverage for the checked range-tree operations and zhack/ZTS coverage for duplicate, partial, chained, summary, pending-state, strict-replay, malformed-entry quarantine, and data-integrity cases. This cannot restore blocks overwritten before detection, but prevents ambiguous ranges from being reused. Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
27d29df to
4f90770
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
tests/zfs-tests/tests/functional/cli_root/zhack/zhack_metaslab_repair.ksh:136
- Use the existing ZTS convention for splitting $DISKS into an array (
set -A ... $DISKS) to match other tests and avoid relying onread -A ... <<<.
read -r -A vdevs <<< "$DISKS"
tests/zfs-tests/tests/functional/cli_root/zhack/zhack_metaslab_repair.ksh:59
- Use the existing ZTS convention for splitting $DISKS into an array.
read -A ... <<<is less common across the test suite; most tests useset -A(e.g. tests/zfs-tests/tests/functional/cli_root/zdb/zdb_003_pos.ksh:53).
This issue also appears on line 136 of the same file.
read -r -A vdevs <<< "$DISKS"
Motivation and Context
Duplicate or partially overlapping operations in an on-disk metaslab
space map currently reach strict range-tree assertions during
metaslab_load(). The resulting panic or blocked loader can make anotherwise importable pool unusable.
A production failure was observed after a
zfs rewrite -rvxoperationunder OpenZFS 2.4.1 and matches the failure class discussed in #13995 and
#17094. Unlike a general range-tree recovery switch, this change is
limited to replaying on-disk metaslab space maps.
The repair is intentionally conservative. An ambiguous range is never
made allocatable, which can retain space that was actually free. This
cannot recover data that was overwritten before the corruption was
detected.
Description
Clean entries retain the same one-lookup path as strict replay, and the
private quarantine tree is allocated only after damage is detected.
remove its complete range from the free tree and keep that range
allocated for all later overlapping operations.
entries with pool, vdev, metaslab, object, TXG, and sync-pass context.
Export aggregate repair counters through the metaslab kstat and record
completed repairs in pool history, including the old and reconstructed
allocation summaries.
space.
smp_allocsummary as fully allocated,then schedule it for repair during import and reconstruct the summary
from the replayed map.
conservative state replaces the corrupt history with a canonical
space map.
has drained. Explicitly dirty and flush the metaslab until it is
retryable, then dispatch a repair load independently of ordinary
metaslab preloading.
assign it zero allocator weight, disable trimming for it, and account
all remaining capacity as unavailable after pending changes drain.
log-space-map debug loading, and RAID-Z expansion handle load failures
without asserting or reporting false completion. RAID-Z expansion
conservatively copies a complete metaslab when its free map cannot be
loaded.
zhack metaslab corruptfor test-only injection of duplicate andpartial ALLOC/FREE entries, chained overlaps, and invalid summaries.
The ZTS regression covers pools with and without log space maps,
preserves pending log-space-map state, waits for automatic condensation
and pool-history reporting with a bounded retry, performs strict
zdb -breplay, checks unchanged file data for every corruption class, and
verifies that an unreadable map is quarantined.
including adjacency merging, splitting, rejection of duplicate and
partial operations, and unchanged state after rejected operations.
Normal
space_map_load()callers retain strict range-tree behavior.How Has This Been Tested?
The following local validation passed:
make unit: 90/90 tests passedmake testscheckmake checkstyle, including C style, ShellCheck, checkbashisms, and mandocscripts/cstyle.plon every modified C/header filescripts/commitcheck.sh HEAD^..HEADgit diff --checkTypes of Changes
Checklist
command is a test-only
zhackinterface.Signed-off-by.