mirror of
https://github.com/freebsd/freebsd-src.git
synced 2026-10-28 11:25:21 +01:00
spa_errlog: don't let one unresolvable entry hide the whole error log
process_error_log() returns as soon as process_error_block() fails on an entry, so one error log entry whose dataset cannot be opened fails ZFS_IOC_ERROR_LOG as a whole. "zpool status" still says "N data errors, use '-v' for a list", but "zpool status -v" lists nothing and prints "errors: List of errors unavailable" on stderr. Two ways to get there: an encrypted filesystem whose key is unloaded after its error was recorded, where the DS_HOLD_FLAG_DECRYPT hold in find_top_affected_fs() fails with EACCES; and an error in a snapshot recorded while the key was unloaded, which is keyed by the snapshot itself and not purged when that snapshot is destroyed, so the hold fails with ENOENT.0c4064d9a0("Fix zpool status in case of unloaded keys") already reports a block whose birth txg was never resolved as a bare bookmark. Do the same when the dataset walk fails: report the bookmark, which zpool(8) prints as "fs:<0xobj>" or "<0xobjset>:<0xobj>", and go on with the rest of the log. The entry stays in the log until it can be evaluated. Only ENOMEM and EFAULT still propagate, since they come from copyout_entry() and zpool_get_errlog() grows its buffer on ENOMEM and retries. zpool-features.7 promised an "access denied" error instead; correct it. zpool_status_005_pos stopped catching this in4eca03faaf("Fixes in head_errlog feature with encryption"), which replaced its "Permanent errors have been detected" assertion with a stdout grep for "permission denied" that never matches the stderr message. Restore the assertion and add a test for each trigger. Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Alexander Motin <[email protected]> Signed-off-by: Ameer Hamza <[email protected]> Closes #19110
This commit is contained in:
1 parent
44aa82a6ce
commit
fa4bc4dec7
7 files changed
+231
-13
No files matched your search
@@ -669,7 +669,7 @@ in the output of
|
||||
.Nm zpool Cm status .
|
||||
In case of encrypted filesystems with unloaded keys we are unable to check
|
||||
their snapshots or clones for errors and these will not be reported.
|
||||
An "access denied" error will be reported.
|
||||
The error block itself is reported against the filesystem, without a filename.
|
||||
.Pp
|
||||
\*[instant-never]
|
||||
.
|
||||
|
||||
+30
-9
@@ -462,6 +462,19 @@ out:
|
||||
return (error);
|
||||
}
|
||||
|
||||
/*
|
||||
* Report an error block whose datasets could not be checked.
|
||||
*/
|
||||
static int
|
||||
copyout_unresolved(uint64_t dataset, zbookmark_err_phys_t *zep, void *uaddr,
|
||||
uint64_t *count)
|
||||
{
|
||||
zbookmark_phys_t zb;
|
||||
|
||||
zep_to_zb(dataset, zep, &zb);
|
||||
return (copyout_entry(&zb, uaddr, count));
|
||||
}
|
||||
|
||||
static int
|
||||
process_error_block(spa_t *spa, uint64_t head_ds, zbookmark_err_phys_t *zep,
|
||||
void *uaddr, uint64_t *count)
|
||||
@@ -473,15 +486,8 @@ process_error_block(spa_t *spa, uint64_t head_ds, zbookmark_err_phys_t *zep,
|
||||
* the key is not loaded. In this case do not proceed to
|
||||
* check_filesystem(), instead do the accounting here.
|
||||
*/
|
||||
if (zep->zb_birth == 0 || head_ds == 0) {
|
||||
zbookmark_phys_t zb;
|
||||
zep_to_zb(head_ds, zep, &zb);
|
||||
int error = copyout_entry(&zb, uaddr, count);
|
||||
if (error != 0) {
|
||||
return (error);
|
||||
}
|
||||
return (0);
|
||||
}
|
||||
if (zep->zb_birth == 0 || head_ds == 0)
|
||||
return (copyout_unresolved(head_ds, zep, uaddr, count));
|
||||
|
||||
uint64_t top_affected_fs;
|
||||
uint64_t init_count = *count;
|
||||
@@ -512,6 +518,21 @@ process_error_block(spa_t *spa, uint64_t head_ds, zbookmark_err_phys_t *zep,
|
||||
|
||||
list_destroy(&clones_list);
|
||||
}
|
||||
|
||||
/*
|
||||
* ENOMEM and EFAULT concern the userland buffer and must be returned.
|
||||
* Any other failure to check the datasets, e.g. because the key is not
|
||||
* loaded or a snapshot was destroyed, must not hide the other entries:
|
||||
* report the block as above unless check_filesystem() already copied
|
||||
* out part of it, and leave the entry in the log.
|
||||
*/
|
||||
if (error != 0 && error != ENOMEM && error != EFAULT) {
|
||||
if (init_count == *count)
|
||||
error = copyout_unresolved(head_ds, zep, uaddr, count);
|
||||
else
|
||||
error = 0;
|
||||
}
|
||||
|
||||
if (error == 0 && init_count == *count) {
|
||||
/*
|
||||
* If we reach this point, no errors have been detected
|
||||
|
||||
@@ -598,8 +598,8 @@ tests = ['zpool_status_001_pos', 'zpool_status_002_pos',
|
||||
'zpool_status_003_pos', 'zpool_status_004_pos',
|
||||
'zpool_status_005_pos', 'zpool_status_006_pos',
|
||||
'zpool_status_007_pos', 'zpool_status_008_pos',
|
||||
'zpool_status_009_pos',
|
||||
'zpool_status_features_001_pos']
|
||||
'zpool_status_009_pos', 'zpool_status_010_pos',
|
||||
'zpool_status_011_pos', 'zpool_status_features_001_pos']
|
||||
tags = ['functional', 'cli_root', 'zpool_status']
|
||||
|
||||
[tests/functional/cli_root/zpool_sync]
|
||||
|
||||
@@ -1407,6 +1407,8 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
|
||||
functional/cli_root/zpool_status/zpool_status_007_pos.ksh \
|
||||
functional/cli_root/zpool_status/zpool_status_008_pos.ksh \
|
||||
functional/cli_root/zpool_status/zpool_status_009_pos.ksh \
|
||||
functional/cli_root/zpool_status/zpool_status_010_pos.ksh \
|
||||
functional/cli_root/zpool_status/zpool_status_011_pos.ksh \
|
||||
functional/cli_root/zpool_status/zpool_status_features_001_pos.ksh \
|
||||
functional/cli_root/zpool_sync/cleanup.ksh \
|
||||
functional/cli_root/zpool_sync/setup.ksh \
|
||||
|
||||
@@ -67,8 +67,14 @@ log_must zpool sync $TESTPOOL2
|
||||
log_must zpool scrub $TESTPOOL2
|
||||
log_must zpool wait -t scrub $TESTPOOL2
|
||||
log_must zpool status -v $TESTPOOL2
|
||||
log_mustnot eval "zpool status -v $TESTPOOL2 | \
|
||||
# The failure this guards against is printed on stderr, and no file list
|
||||
# is printed at all, so both of these have to be checked.
|
||||
log_mustnot eval "zpool status -v $TESTPOOL2 2>&1 | \
|
||||
grep \"permission denied\""
|
||||
log_must eval "zpool status -v $TESTPOOL2 | \
|
||||
grep \"Permanent errors have been detected\""
|
||||
log_must eval "zpool status -v $TESTPOOL2 | \
|
||||
grep '$TESTPOOL2/$TESTFS1:<0x'"
|
||||
log_mustnot eval "zpool status -v $TESTPOOL2 | grep '$file'"
|
||||
|
||||
log_must eval "cat /$TESTPOOL2/pwd | zfs load-key $TESTPOOL2/$TESTFS1"
|
||||
|
||||
+87
@@ -0,0 +1,87 @@
|
||||
#!/bin/ksh -p
|
||||
# SPDX-License-Identifier: CDDL-1.0
|
||||
|
||||
#
|
||||
# This file and its contents are supplied under the terms of the
|
||||
# Common Development and Distribution License ("CDDL"), version 1.0.
|
||||
# You may only use this file in accordance with the terms of version
|
||||
# 1.0 of the CDDL.
|
||||
#
|
||||
# A full copy of the text of the CDDL should have accompanied this
|
||||
# source. A copy of the CDDL is also available via the Internet at
|
||||
# https://opensource.org/license/CDDL-1.0.
|
||||
#
|
||||
|
||||
#
|
||||
# Copyright (c) 2026 by iXsystems, Inc.
|
||||
#
|
||||
|
||||
#
|
||||
# DESCRIPTION:
|
||||
# Verify an error whose filesystem cannot be resolved does not hide the
|
||||
# rest of the error log.
|
||||
#
|
||||
# STRATEGY:
|
||||
# 1. Create a pool with a plain and an encrypted filesystem, and a file in
|
||||
# each.
|
||||
# 2. Corrupt both files and scrub with the key loaded, so both errors are
|
||||
# recorded with a resolvable head filesystem.
|
||||
# 3. Unmount the encrypted filesystem and unload its key.
|
||||
# 4. Verify 'zpool status -v' still reports the error in the plain
|
||||
# filesystem, and the encrypted one without a filename.
|
||||
#
|
||||
|
||||
. $STF_SUITE/include/libtest.shlib
|
||||
|
||||
verify_runnable "both"
|
||||
|
||||
function cleanup
|
||||
{
|
||||
destroy_pool $TESTPOOL2
|
||||
rm -f $TESTDIR/vdev_a
|
||||
}
|
||||
|
||||
log_assert "An unresolvable error log entry does not hide the other errors"
|
||||
log_onexit cleanup
|
||||
|
||||
typeset passphrase="password"
|
||||
typeset plainfile="/$TESTPOOL2/plain/$TESTFILE0"
|
||||
typeset encfile="/$TESTPOOL2/$TESTFS1/$TESTFILE0"
|
||||
|
||||
truncate -s $MINVDEVSIZE $TESTDIR/vdev_a
|
||||
log_must zpool create -f -o feature@head_errlog=enabled $TESTPOOL2 \
|
||||
$TESTDIR/vdev_a
|
||||
|
||||
log_must zfs create -o primarycache=none $TESTPOOL2/plain
|
||||
log_must eval "echo $passphrase > /$TESTPOOL2/pwd"
|
||||
log_must zfs create -o encryption=aes-256-ccm -o keyformat=passphrase \
|
||||
-o keylocation=file:///$TESTPOOL2/pwd -o primarycache=none \
|
||||
$TESTPOOL2/$TESTFS1
|
||||
|
||||
log_must dd if=/dev/urandom of=$plainfile bs=1024 count=1024 oflag=sync
|
||||
log_must dd if=/dev/urandom of=$encfile bs=1024 count=1024 oflag=sync
|
||||
|
||||
corrupt_blocks_at_level $plainfile 0
|
||||
corrupt_blocks_at_level $encfile 0
|
||||
|
||||
# Scrub with the key loaded, so both entries name a resolvable filesystem.
|
||||
log_must zpool scrub -w $TESTPOOL2
|
||||
log_must eval "zpool status -v $TESTPOOL2 | grep '$plainfile'"
|
||||
log_must eval "zpool status -v $TESTPOOL2 | grep '$encfile'"
|
||||
|
||||
log_must zfs umount $TESTPOOL2/$TESTFS1
|
||||
log_must zfs unload-key $TESTPOOL2/$TESTFS1
|
||||
|
||||
log_must zpool status -v $TESTPOOL2
|
||||
# The failure this guards against is printed on stderr, and no file list
|
||||
# is printed at all, so both of these have to be checked.
|
||||
log_mustnot eval "zpool status -v $TESTPOOL2 2>&1 | \
|
||||
grep 'List of errors unavailable'"
|
||||
log_must eval "zpool status -v $TESTPOOL2 | \
|
||||
grep 'Permanent errors have been detected'"
|
||||
log_must eval "zpool status -v $TESTPOOL2 | grep '$plainfile'"
|
||||
# The head filesystem still resolves; only the file name does not.
|
||||
log_must eval "zpool status -v $TESTPOOL2 | \
|
||||
grep '$TESTPOOL2/$TESTFS1:<0x'"
|
||||
|
||||
log_pass "An unresolvable error log entry does not hide the other errors"
|
||||
+102
@@ -0,0 +1,102 @@
|
||||
#!/bin/ksh -p
|
||||
# SPDX-License-Identifier: CDDL-1.0
|
||||
|
||||
#
|
||||
# This file and its contents are supplied under the terms of the
|
||||
# Common Development and Distribution License ("CDDL"), version 1.0.
|
||||
# You may only use this file in accordance with the terms of version
|
||||
# 1.0 of the CDDL.
|
||||
#
|
||||
# A full copy of the text of the CDDL should have accompanied this
|
||||
# source. A copy of the CDDL is also available via the Internet at
|
||||
# https://opensource.org/license/CDDL-1.0.
|
||||
#
|
||||
|
||||
#
|
||||
# Copyright (c) 2026 by iXsystems, Inc.
|
||||
#
|
||||
|
||||
#
|
||||
# DESCRIPTION:
|
||||
# Verify an error log entry left behind by a destroyed snapshot does not
|
||||
# hide the rest of the error log.
|
||||
#
|
||||
# STRATEGY:
|
||||
# 1. Create a pool with a plain and an encrypted filesystem.
|
||||
# 2. Snapshot the encrypted filesystem and unlink the file, so the block
|
||||
# only exists in the snapshot, then unload the key.
|
||||
# 3. Inject an error in the snapshot and one in the plain filesystem, and
|
||||
# scrub. With the key unloaded the snapshot entry is keyed by the
|
||||
# snapshot itself rather than by its head filesystem.
|
||||
# 4. Load the key and destroy the snapshot, which leaves its entry in the
|
||||
# log keyed by a dataset that no longer exists.
|
||||
# 5. Verify 'zpool status -v' still reports the error in the plain
|
||||
# filesystem, and reports the unresolvable one as a bare bookmark.
|
||||
#
|
||||
|
||||
. $STF_SUITE/include/libtest.shlib
|
||||
|
||||
verify_runnable "both"
|
||||
|
||||
function cleanup
|
||||
{
|
||||
log_must zinject -c all
|
||||
destroy_pool $TESTPOOL2
|
||||
rm -f $TESTDIR/vdev_a
|
||||
}
|
||||
|
||||
log_assert "A destroyed snapshot's error log entry does not hide the others"
|
||||
log_onexit cleanup
|
||||
|
||||
typeset passphrase="password"
|
||||
typeset plainfile="/$TESTPOOL2/plain/$TESTFILE0"
|
||||
typeset encfile="/$TESTPOOL2/$TESTFS1/$TESTFILE0"
|
||||
|
||||
truncate -s $MINVDEVSIZE $TESTDIR/vdev_a
|
||||
log_must zpool create -f -o feature@head_errlog=enabled $TESTPOOL2 \
|
||||
$TESTDIR/vdev_a
|
||||
|
||||
log_must zfs create -o primarycache=none $TESTPOOL2/plain
|
||||
log_must eval "echo $passphrase > /$TESTPOOL2/pwd"
|
||||
log_must zfs create -o encryption=aes-256-ccm -o keyformat=passphrase \
|
||||
-o keylocation=file:///$TESTPOOL2/pwd -o primarycache=none \
|
||||
$TESTPOOL2/$TESTFS1
|
||||
|
||||
log_must dd if=/dev/urandom of=$plainfile bs=1024 count=512 oflag=sync
|
||||
log_must dd if=/dev/urandom of=$encfile bs=1024 count=512 oflag=sync
|
||||
|
||||
typeset -i plainobjset=$(get_prop objsetid $TESTPOOL2/plain)
|
||||
typeset -i plainobj=$(get_objnum $plainfile)
|
||||
typeset -i encobj=$(get_objnum $encfile)
|
||||
|
||||
log_must zfs snapshot $TESTPOOL2/$TESTFS1@snap1
|
||||
typeset -i snapobjset=$(get_prop objsetid $TESTPOOL2/$TESTFS1@snap1)
|
||||
log_must rm -f $encfile
|
||||
log_must zpool sync $TESTPOOL2
|
||||
|
||||
log_must zfs umount $TESTPOOL2/$TESTFS1
|
||||
log_must zfs unload-key $TESTPOOL2/$TESTFS1
|
||||
|
||||
# The snapshot entry is recorded while the key is unloaded, so it is keyed
|
||||
# by the snapshot rather than by the head filesystem.
|
||||
log_must zinject -b $(printf '%x:%x:0:0' $snapobjset $encobj) $TESTPOOL2
|
||||
log_must zinject -b $(printf '%x:%x:0:0' $plainobjset $plainobj) $TESTPOOL2
|
||||
log_must zpool scrub -w $TESTPOOL2
|
||||
log_must zinject -c all
|
||||
|
||||
log_must zfs load-key $TESTPOOL2/$TESTFS1
|
||||
|
||||
log_must zfs destroy $TESTPOOL2/$TESTFS1@snap1
|
||||
log_must zpool sync $TESTPOOL2
|
||||
|
||||
log_must zpool status -v $TESTPOOL2
|
||||
log_mustnot eval "zpool status -v $TESTPOOL2 2>&1 | \
|
||||
grep 'List of errors unavailable'"
|
||||
log_must eval "zpool status -v $TESTPOOL2 | \
|
||||
grep 'Permanent errors have been detected'"
|
||||
log_must eval "zpool status -v $TESTPOOL2 | grep '$plainfile'"
|
||||
# Neither the dataset nor the file name resolves any more.
|
||||
log_must eval "zpool status -v $TESTPOOL2 | \
|
||||
grep -E '<0x[0-9a-f]+>:<0x[0-9a-f]+>'"
|
||||
|
||||
log_pass "A destroyed snapshot's error log entry does not hide the others"
|
||||
Reference in new issue
Block a user