Skip to content

spa: Take the RAID-Z layout from labels newer than the config - #19405

Open
tigerblue77 wants to merge 2 commits into
openzfs:masterfrom
tigerblue77:raidz-expand-cachefile
Open

tigerblue77 wants to merge 2 commits into
openzfs:masterfrom
tigerblue77:raidz-expand-cachefile

Conversation

@tigerblue77

@tigerblue77 tigerblue77 commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Motivation and Context

Closes #19404.

In the completion txg of a RAID-Z expansion, raidz_reflow_complete_sync() removes raidz_expanding, adds an entry to raidz_expand_txgs and clears the reflow offset of the uberblock. The cachefile is rewritten a few txgs later, by vdev_online(). A crash in between leaves a cachefile that still says the vdev is expanding. Opening the pool from it reads every row from its pre-expansion location, since the offset is now 0, and fails with EIO ("unable to open rootbp in dsl_pool_init"). So does any copy of the cachefile taken during the expansion.

  • Reproduced: zdb -U <cachefile> fails, and so do spa_tryimport() and spa_import() with the cached config, the kernel side of zpool import -c.
  • zpool import -c imports pools at boot from /etc/zfs/zpool.cache (zfs-import-cache.service, the init.d and initramfs scripts). It then only gets the pool back through its fallback to a device scan, which I did not test.

Writing the cachefile sooner only narrows the window: a simulated crash still landed in it.

Description

Once the uberblock is selected, and before the MOS is read, spa_ld_label_layout() (called from spa_ld_mos_init()) looks at each top-level RAID-Z vdev that the config shows expanding. vdev_top_label_read_config() finds the newest label its leaves hold among those written no later than the selected uberblock. If that label is newer than the config (spa_config_txg), vdev_raidz_config_from_label() replaces the expansion state of the vdev with the label's.

  • A label is written in every txg that changes its vdev's config, so that label describes the layout the MOS was written with; even labels of a txg whose uberblock never reached the disk are left out. The cachefile holds the MOS config as of spa_config_txg, so a label with a higher txg postdates it.
  • Only expanding vdevs are looked at: at the width of the config, a newer label can only differ by having completed that expansion, since an attach changes the width and a completion is not undone. No label is read for other vdevs, nor for a config of txg 0 (a device scan); zpool import -c reads them once, since its import gets the MOS config.
  • The label must describe the same vdev at the same parity and width, with a valid expansion state; otherwise the config is kept and the load notes why.
  • When a label was taken, an open also rewrites the cachefile. Only the untrusted vdev tree changes, and the MOS config replaces it; if the load still fails, spa_load_best() keeps the config regenerated from it.

Not covered:

  • A cachefile from before the attach, which lacks the new child: fails as before.
  • A rewind older than every label on the vdev, or (with a cachefile from before it) a rewind of a completed expansion to between the attach and the first reflow progress: as before.
  • Adjacent: a crash in the window also skips the vdev_online(..., ZFS_ONLINE_EXPAND) of spa_raidz_expand_thread(), so the added capacity stays unavailable until zpool online -e; autoexpand=on alone does not bring it back (RAID-Z expansion: a crash right after completion leaves the added capacity unavailable #19420). This PR does not change that.

How Has This Been Tested?

In userspace only (libzpool, zdb; --with-config=user --enable-debug, Ubuntu 24.04 x86_64), against c4da9f8, whose libzpool the test commit keeps. Not run: the new ZTS test raidz_expand_stale_cachefile, or any ZTS test (no kernel module here). The kernel modules were not built. The branch was then rebased onto 22013c3, which only changed the context of the test lists.

  • ZTS analog in libzpool (pause the reflow, copy the cachefile, complete, export, tryimport and import from the copy), raidz1 2→3, raidz2 3→4, raidz3 4→5: EIO on c4da9f8. This branch imports, reads every byte back, scrubs clean, and notes once that it used the label's layout.
  • Simulated crash in the window, raidz1 3→4 and raidz2 4→5, zdb -U <cachefile>: EIO on c4da9f8, opens here.
  • Attach-time copy, 12 txgs after completion: fails on c4da9f8, opens here, also with -t 47/50/56 (-t 43/46 fail on both: every label is at txg 47).
  • Writable spa_open() from that copy: fails on c4da9f8. Here it opens, and the cachefile is rewritten without raidz_expanding.
  • A copy edited to another parity: the label is refused, with a load note saying why. With the current cachefile, an open reads as many labels as on c4da9f8 (strace).
  • ztest, every run exited 0: -X -T 300 three times, -X -T 300 -r 6 -R 2 and -r 6 -R 3, -K raidz -T 300 -P 60.
  • make checkstyle passes. shellcheck, checkbashisms and mandoc are not installed here.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist

raidz_expand_stale_cachefile copies the cachefile of a raidz pool
while an expansion is paused, lets the expansion complete, exports
the pool and imports it from the copy.  The copy says that the vdev
is expanding, as the cachefile still does when a crash comes after
the expansion has completed and before the cachefile is rewritten.

Reading the MOS with that state takes every row to be at its
pre-expansion location, since the uberblock no longer carries a
reflow offset, so the import from the cachefile fails with EIO and
zpool import -c falls back to scanning the devices.  The test fails
when that fallback is needed.  It also checks that the import took
the layout from the labels, that the data reads back and that the
cachefile it imported into no longer says the vdev is expanding.
The import writes to the test's own cachefile, not the system one.

Signed-off-by: Tigerblue77 <37409593+tigerblue77@users.noreply.github.com>
When a RAID-Z expansion completes, raidz_reflow_complete_sync()
removes raidz_expanding and adds an entry to raidz_expand_txgs in the
labels and the MOS config of the completion txg, and clears the
reflow offset of its uberblock.  The cachefile is only rewritten a
few txgs later, when vdev_online() expands the vdev.  A crash in
between leaves a cachefile that says the vdev is still expanding.
Opening the pool from it reads the MOS as if the reflow were in
progress with nothing copied, every row at its pre-expansion
location, and fails with EIO: "unable to open rootbp in
dsl_pool_init".  The labels describe the vdev as it is, so an
import that scans the devices works, and zpool import -c only gets
there through its fallback.

Writing the cachefile sooner would only narrow that window.  Make
the load independent of it instead: once the uberblock is selected,
and before the MOS is read, find for each top-level RAID-Z vdev that
the config shows expanding the newest label its leaves hold that was
not written after that uberblock, and when that label is newer than
the config, take the expansion state of the vdev from it.  A label is
written in every txg that changes the config of its vdev, so that
label describes the layout the MOS was written with.  An even label
written in a txg whose uberblock never reached the disk is newer
than the uberblock, so it is not used.

Only a vdev that the config shows expanding is looked at: at the
width of the config, a newer label can only differ by having
completed that expansion, since an attach changes the width and a
completion is not undone.  No label is read for any other vdev, nor
for a config of txg 0, which is what a device scan passes, and the
import that follows a tryimport, whose config comes from the MOS,
reads none once the expansion has completed.  The label must
describe the same vdev at the same width and parity, with a valid
expansion state; otherwise the config is kept and the load notes
why.  Once a label has been taken, the config is rewritten after the
pool is open, which an open from the cachefile would not otherwise
do, since the txg of its config has not changed.

Not covered: a cachefile from before the attach, which lacks the new
child; a rewind to a txg older than every label left on the vdev;
and, opened with a cachefile from before it, a rewind of a completed
expansion to the txgs between the attach and the first reflow
progress, which vdev_uberblock_load() does not refuse.

Signed-off-by: Tigerblue77 <37409593+tigerblue77@users.noreply.github.com>
@tigerblue77
tigerblue77 force-pushed the raidz-expand-cachefile branch from 42c06c2 to 1368cd4 Compare October 15, 2026 20:56
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.

RAID-Z expansion: a crash just after completion leaves a cachefile that cannot open the pool

1 participant