libzutil: keep valid spare and l2cache paths on import#18742
Open
MorganaFuture wants to merge 1 commit into
Open
libzutil: keep valid spare and l2cache paths on import#18742MorganaFuture wants to merge 1 commit into
MorganaFuture wants to merge 1 commit into
Conversation
Unlike the vdev tree, whose paths are read from the scanned labels and whose label paths are added to the candidate name list (so the exact match in fix_paths() preserves a persistent name whenever its symlink is still alive), spare and l2cache configs are obtained from the pool's MOS by a tryimport. When the on-disk aux label carries no path (labels last written before eb4a36b) or a stale one, the MOS path never appears among the scanned names and fix_paths() rewrites it to the best scanned candidate. With the default libblkid discovery that is the bare /dev basename, which is not stable across reboots. Since 5536c0d ("Sync AUX label during pool import") the rewritten path is also synced back to the MOS and the aux labels, making the reversion permanent. This is the remaining piece of openzfs#13170, where a cache device added by a /dev/disk/by-* name comes back as e.g. nvme0n1p4 after an export and import. It also answers the question raised there about openzfs#15817: that change fixed the reversion for aux vdevs whose labels contain a matching path, but pools carried over from older releases regress on their first import and are then stuck with the unstable name. Keep an existing spare or l2cache path when the device can still be opened exclusively and the label found there matches the expected vdev guid, mirroring the checks applied to scanned candidates. The MOS-supplied path is not trusted beyond that: like the device scan, we only open paths naming a device type which is safe to probe, so a crafted or damaged pool cannot make the import listing open FIFOs or side-effectful device nodes. Only keep paths when the search locations were not chosen by the caller, since importing from user supplied directories is the documented way to deliberately rewrite a pool's device paths; -d and -s imports keep the current behavior. Pools whose aux path was already rewritten to a bare /dev name by an earlier import keep that name, since it is still valid; a one-time import from the wanted directory, e.g. -d /dev/disk/by-id, remains the way to restore a persistent path there. Add a ZTS case covering the new and the preserved behavior: an aux path which still refers to the expected device is kept on a default import, is rewritten on a -d import, and is replaced by a scanned name once it goes stale. Signed-off-by: MorganaFuture <103630661+MorganaFuture@users.noreply.github.com> Closes openzfs#13170
github-actions
Bot
added
the
Status: Work in Progress
github-actions
Bot
added
Status: Code Review Needed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Fixes #13170. A cache (L2ARC) or spare device added by a persistent
/dev/disk/by-*name comes back as a bare kernel name (e.g.nvme0n1p4)after an export and import, losing path persistence across reboots.
Description
Unlike the vdev tree, whose paths come from the scanned labels, spare and
l2cache configs are read from the pool's MOS by a tryimport. When the aux
label carries no path (labels written before eb4a36b) or a stale one,
the MOS path is absent from the scanned name list and
fix_paths()rewrites it to the best scanned candidate, which under the default libblkid
discovery is the unstable bare
/devbasename. Since 5536c0d ("Sync AUXlabel during pool import") the rewrite is also synced back to the MOS,
making the reversion permanent. This is the case #15817 did not cover:
pools carried over from older releases regress on their first import.
The fix keeps an existing spare or l2cache path when the device can still
be opened exclusively and the label found there matches the expected vdev
guid, mirroring the checks applied to scanned candidates. The MOS-supplied
path is only opened after confirming it names a device type that is safe
to probe. Paths are preserved only when the search locations were not
chosen by the caller, so
-d/-simports keep the existing behavior ofdeliberately rewriting paths.
How Has This Been Tested?
Built and tested on Linux aarch64 (kernel 7.0). Added a ZTS case
(
zpool_import_aux_paths, scsi_debug-backed) asserting that a custompersistent aux path is kept on a default import, rewritten under
-d, andreplaced with a scanned name once stale. Ran the
cli_root/zpool_importgroup before and after the change: the FAIL set is unchanged (no
regression). Broader coverage is left to CI.
Types of changes
Checklist:
Signed-off-by.