Skip to content

Commit 1e1d64d

Browse files
amotinamotin
authored
Fix log vdev removal issues
When we clear the log, we should clear all the fields, not only zh_log. Otherwise remaining ZIL_REPLAY_NEEDED will prevent the vdev removal. Handle it also from the other side, when zh_log is already cleared, while zh_flags is not. spa_vdev_remove_log() asserts that allocated space on removed log device is zero. While it should be so in perfect world, it might be not if space leaked at any point. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alexander Motin <alexander.motin@TrueNAS.com> Closes #18277
1 parent f6205fd commit 1e1d64d

File tree

5 files changed

+113
-6
lines changed

5 files changed

+113
-6
lines changed

‎module/zfs/vdev_removal.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2151,7 +2151,6 @@ spa_vdev_remove_log(vdev_t *vd, uint64_t *txg)
21512151
ASSERT0P(vd->vdev_log_mg);
21522152
return (error);
21532153
}
2154-
ASSERT0(vd->vdev_stat.vs_alloc);
21552154

21562155
/*
21572156
* The evacuation succeeded. Remove any remaining MOS metadata

‎module/zfs/zil.c‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ zil_destroy(zilog_t *zilog, boolean_t keep_first)
10961096

10971097
zilog->zl_old_header = *zh; /* debugging aid */
10981098

1099-
if (BP_IS_HOLE(&zh->zh_log))
1099+
if (BP_IS_HOLE(&zh->zh_log) && zh->zh_flags == 0)
11001100
return (B_FALSE);
11011101

11021102
tx = dmu_tx_create(zilog->zl_os);
@@ -1166,6 +1166,15 @@ zil_claim(dsl_pool_t *dp, dsl_dataset_t *ds, void *txarg)
11661166
zilog = dmu_objset_zil(os);
11671167
zh = zil_header_in_syncing_context(zilog);
11681168
ASSERT3U(tx->tx_txg, ==, spa_first_txg(zilog->zl_spa));
1169+
1170+
/*
1171+
* If the log is empty, then there is nothing to do here.
1172+
*/
1173+
if (BP_IS_HOLE(&zh->zh_log)) {
1174+
dmu_objset_disown(os, B_FALSE, FTAG);
1175+
return (0);
1176+
}
1177+
11691178
first_txg = spa_min_claim_txg(zilog->zl_spa);
11701179

11711180
/*
@@ -1198,11 +1207,14 @@ zil_claim(dsl_pool_t *dp, dsl_dataset_t *ds, void *txarg)
11981207
if (spa_get_log_state(zilog->zl_spa) == SPA_LOG_CLEAR ||
11991208
(zilog->zl_spa->spa_uberblock.ub_checkpoint_txg != 0 &&
12001209
zh->zh_claim_txg == 0)) {
1201-
if (!BP_IS_HOLE(&zh->zh_log)) {
1210+
if (zilog->zl_spa->spa_uberblock.ub_checkpoint_txg != 0 &&
1211+
BP_GET_BIRTH(&zh->zh_log) < first_txg) {
12021212
(void) zil_parse(zilog, zil_clear_log_block,
12031213
zil_noop_log_record, tx, first_txg, B_FALSE);
1214+
} else {
1215+
zio_free(zilog->zl_spa, first_txg, &zh->zh_log);
12041216
}
1205-
BP_ZERO(&zh->zh_log);
1217+
memset(zh, 0, sizeof (zil_header_t));
12061218
if (os->os_encrypted)
12071219
os->os_next_write_raw[tx->tx_txg & TXG_MASK] = B_TRUE;
12081220
dsl_dataset_dirty(dmu_objset_ds(os), tx);
@@ -1224,7 +1236,7 @@ zil_claim(dsl_pool_t *dp, dsl_dataset_t *ds, void *txarg)
12241236
* or destroy beyond the last block we successfully claimed.
12251237
*/
12261238
ASSERT3U(zh->zh_claim_txg, <=, first_txg);
1227-
if (zh->zh_claim_txg == 0 && !BP_IS_HOLE(&zh->zh_log)) {
1239+
if (zh->zh_claim_txg == 0) {
12281240
(void) zil_parse(zilog, zil_claim_log_block,
12291241
zil_claim_log_record, tx, first_txg, B_FALSE);
12301242
zh->zh_claim_txg = first_txg;

‎tests/runfiles/common.run‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ tests = ['removal_all_vdev', 'removal_cancel', 'removal_check_space',
947947
'removal_with_write', 'removal_with_zdb', 'remove_expanded',
948948
'remove_mirror', 'remove_mirror_sanity', 'remove_raidz',
949949
'remove_indirect', 'remove_attach_mirror', 'removal_reservation',
950-
'removal_with_hole']
950+
'removal_with_hole', 'removal_with_missing_log']
951951
tags = ['functional', 'removal']
952952

953953
[tests/functional/rename_dirs]

‎tests/zfs-tests/tests/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1947,6 +1947,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
19471947
functional/removal/removal_with_faulted.ksh \
19481948
functional/removal/removal_with_ganging.ksh \
19491949
functional/removal/removal_with_hole.ksh \
1950+
functional/removal/removal_with_missing_log.ksh \
19501951
functional/removal/removal_with_indirect.ksh \
19511952
functional/removal/removal_with_remove.ksh \
19521953
functional/removal/removal_with_scrub.ksh \

‎tests/zfs-tests/tests/functional/removal/removal_with_missing_log.ksh‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#!/bin/ksh -p
2+
# SPDX-License-Identifier: CDDL-1.0
3+
#
4+
# CDDL HEADER START
5+
#
6+
# This file and its contents are supplied under the terms of the
7+
# Common Development and Distribution License ("CDDL"), version 1.0.
8+
# You may only use this file in accordance with the terms of version
9+
# 1.0 of the CDDL.
10+
#
11+
# A full copy of the text of the CDDL should have accompanied this
12+
# source. A copy of the CDDL is also available via the Internet at
13+
# http://www.illumos.org/license/CDDL.
14+
#
15+
# CDDL HEADER END
16+
#
17+
18+
#
19+
# Copyright (c) 2026, TrueNAS.
20+
#
21+
22+
. $STF_SUITE/include/libtest.shlib
23+
. $STF_SUITE/tests/functional/removal/removal.kshlib
24+
25+
#
26+
# DESCRIPTION:
27+
# Verify that a missing SLOG device can be removed even when
28+
# ZIL blocks exist on it.
29+
#
30+
# STRATEGY:
31+
# 1. Create a pool with a SLOG device
32+
# 2. Freeze the pool and write data to ZIL
33+
# 3. Export the pool (ZIL blocks remain uncommitted)
34+
# 4. Import with -N to claim logs without replay
35+
# 5. Export and clear SLOG device labels to simulate failure
36+
# 6. Import with -m (missing devices allowed)
37+
# 7. Remove the missing SLOG vdev
38+
# 8. Verify pool is healthy and space accounting is correct
39+
#
40+
41+
verify_runnable "global"
42+
43+
log_assert "Removal of missing SLOG with ZIL blocks succeeds"
44+
45+
function cleanup
46+
{
47+
poolexists $TESTPOOL && destroy_pool $TESTPOOL
48+
}
49+
50+
log_onexit cleanup
51+
52+
VDEV1="$(echo $DISKS | cut -d' ' -f1)"
53+
VDEV2="$(echo $DISKS | cut -d' ' -f2)"
54+
55+
# Create pool with SLOG and dataset
56+
log_must zpool create $TESTPOOL $VDEV1 log $VDEV2
57+
log_must zfs create $TESTPOOL/$TESTFS
58+
59+
# Create initial ZIL header (required before freezing)
60+
log_must dd if=/dev/zero of=/$TESTPOOL/$TESTFS/init \
61+
conv=fdatasync,fsync bs=1 count=1
62+
63+
# Freeze pool and write data to ZIL
64+
log_must zpool freeze $TESTPOOL
65+
log_must dd if=/dev/urandom of=/$TESTPOOL/$TESTFS/file1 \
66+
oflag=sync bs=128k count=128
67+
68+
# Export with uncommitted ZIL transactions
69+
log_must zpool export $TESTPOOL
70+
71+
# Import with -N to claim logs without mounting/replaying
72+
log_must zpool import -N $TESTPOOL
73+
log_must zpool export $TESTPOOL
74+
75+
# Clear SLOG labels to simulate device failure
76+
log_must zpool labelclear -f $VDEV2
77+
78+
# Import with missing SLOG allowed
79+
log_must zpool import -m $TESTPOOL
80+
log_must eval "zpool status $TESTPOOL | grep UNAVAIL"
81+
82+
# Remove the missing SLOG - should succeed
83+
log_must zpool remove $TESTPOOL $VDEV2
84+
log_must zpool wait -t remove $TESTPOOL
85+
sync_pool $TESTPOOL
86+
log_mustnot eval "zpool status -v $TESTPOOL | grep $VDEV2"
87+
88+
# Verify pool health
89+
log_must zpool scrub -w $TESTPOOL
90+
log_must check_pool_status $TESTPOOL "errors" "No known data errors"
91+
92+
# Verify space accounting is correct
93+
log_must zdb -c $TESTPOOL
94+
95+
log_pass "Removal of missing SLOG with ZIL blocks succeeded"

0 commit comments

Comments
 (0)