From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pve-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 4095E1FF16E for <inbox@lore.proxmox.com>; Mon, 31 Mar 2025 15:42:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DF5F46E52; Mon, 31 Mar 2025 15:42:15 +0200 (CEST) From: Stoiko Ivanov <s.ivanov@proxmox.com> To: pve-devel@lists.proxmox.com Date: Mon, 31 Mar 2025 15:41:28 +0200 Message-Id: <20250331134128.168524-9-s.ivanov@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250331134128.168524-1-s.ivanov@proxmox.com> References: <20250331134128.168524-1-s.ivanov@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.061 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LOTSOFHASH 0.25 Emails with lots of hash-like gibberish SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH zfsonlinux 8/8] cherry-pick fix for ABI break from zfs 2.3.2-staging X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com> without this patch many common operations break when running with a kernel module < 2.3.1. Noticed while testing replication with our current 2.2.7 module and userspace from 2.3.1 Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> --- ...ount-matches-and-injections-for-each.patch | 500 ++++++++++++++++++ debian/patches/series | 1 + 2 files changed, 501 insertions(+) create mode 100644 debian/patches/0012-Revert-zinject-count-matches-and-injections-for-each.patch diff --git a/debian/patches/0012-Revert-zinject-count-matches-and-injections-for-each.patch b/debian/patches/0012-Revert-zinject-count-matches-and-injections-for-each.patch new file mode 100644 index 000000000..9f829f124 --- /dev/null +++ b/debian/patches/0012-Revert-zinject-count-matches-and-injections-for-each.patch @@ -0,0 +1,500 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Rob Norris <rob.norris@klarasystems.com> +Date: Tue, 25 Mar 2025 07:49:10 +1100 +Subject: [PATCH] Revert "zinject: count matches and injections for each + handler" (#17137) + +Adding fields to zinject_record_t unexpectedly extended zfs_cmd_t, +preventing some things working properly with 2.3.1 userspace tools +against 2.3.0 kernel module. + +This reverts commit fabdd502f4f04e27d057aedc7fb7697e7bd95b74. + +Sponsored-by: Klara, Inc. +Sponsored-by: Wasabi Technology, Inc. + +Signed-off-by: Rob Norris <rob.norris@klarasystems.com> +Reviewed-by: Alexander Motin <mav@FreeBSD.org> +Reviewed-by: Tony Hutter <hutter2@llnl.gov> +(cherry picked from commit 5f7037067e3113332ebfcb2913fd5d5183898540) +Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> +--- + cmd/zinject/zinject.c | 64 +++----- + include/sys/zfs_ioctl.h | 2 - + module/zfs/zio_inject.c | 58 ++----- + tests/runfiles/common.run | 2 +- + tests/zfs-tests/tests/Makefile.am | 1 - + .../cli_root/zinject/zinject_counts.ksh | 142 ------------------ + 6 files changed, 36 insertions(+), 233 deletions(-) + delete mode 100755 tests/zfs-tests/tests/functional/cli_root/zinject/zinject_counts.ksh + +diff --git a/cmd/zinject/zinject.c b/cmd/zinject/zinject.c +index 4374e69a7f94d9529f9db33c8646bd30010f0ec7..fdb2221eaea63f7f7b290323eaf0d810c2b76697 100644 +--- a/cmd/zinject/zinject.c ++++ b/cmd/zinject/zinject.c +@@ -434,29 +434,26 @@ print_data_handler(int id, const char *pool, zinject_record_t *record, + + if (*count == 0) { + (void) printf("%3s %-15s %-6s %-6s %-8s %3s %-4s " +- "%-15s %-6s %-15s\n", "ID", "POOL", "OBJSET", "OBJECT", +- "TYPE", "LVL", "DVAs", "RANGE", "MATCH", "INJECT"); ++ "%-15s\n", "ID", "POOL", "OBJSET", "OBJECT", "TYPE", ++ "LVL", "DVAs", "RANGE"); + (void) printf("--- --------------- ------ " +- "------ -------- --- ---- --------------- " +- "------ ------\n"); ++ "------ -------- --- ---- ---------------\n"); + } + + *count += 1; + +- char rangebuf[32]; +- if (record->zi_start == 0 && record->zi_end == -1ULL) +- snprintf(rangebuf, sizeof (rangebuf), "all"); +- else +- snprintf(rangebuf, sizeof (rangebuf), "[%llu, %llu]", +- (u_longlong_t)record->zi_start, +- (u_longlong_t)record->zi_end); ++ (void) printf("%3d %-15s %-6llu %-6llu %-8s %-3d 0x%02x ", ++ id, pool, (u_longlong_t)record->zi_objset, ++ (u_longlong_t)record->zi_object, type_to_name(record->zi_type), ++ record->zi_level, record->zi_dvas); + + +- (void) printf("%3d %-15s %-6llu %-6llu %-8s %-3d 0x%02x %-15s " +- "%6lu %6lu\n", id, pool, (u_longlong_t)record->zi_objset, +- (u_longlong_t)record->zi_object, type_to_name(record->zi_type), +- record->zi_level, record->zi_dvas, rangebuf, +- record->zi_match_count, record->zi_inject_count); ++ if (record->zi_start == 0 && ++ record->zi_end == -1ULL) ++ (void) printf("all\n"); ++ else ++ (void) printf("[%llu, %llu]\n", (u_longlong_t)record->zi_start, ++ (u_longlong_t)record->zi_end); + + return (0); + } +@@ -474,14 +471,11 @@ print_device_handler(int id, const char *pool, zinject_record_t *record, + return (0); + + if (*count == 0) { +- (void) printf("%3s %-15s %-16s %-5s %-10s %-9s " +- "%-6s %-6s\n", +- "ID", "POOL", "GUID", "TYPE", "ERROR", "FREQ", +- "MATCH", "INJECT"); ++ (void) printf("%3s %-15s %-16s %-5s %-10s %-9s\n", ++ "ID", "POOL", "GUID", "TYPE", "ERROR", "FREQ"); + (void) printf( + "--- --------------- ---------------- " +- "----- ---------- --------- " +- "------ ------\n"); ++ "----- ---------- ---------\n"); + } + + *count += 1; +@@ -489,10 +483,9 @@ print_device_handler(int id, const char *pool, zinject_record_t *record, + double freq = record->zi_freq == 0 ? 100.0f : + (((double)record->zi_freq) / ZI_PERCENTAGE_MAX) * 100.0f; + +- (void) printf("%3d %-15s %llx %-5s %-10s %8.4f%% " +- "%6lu %6lu\n", id, pool, (u_longlong_t)record->zi_guid, +- iotype_to_str(record->zi_iotype), err_to_str(record->zi_error), +- freq, record->zi_match_count, record->zi_inject_count); ++ (void) printf("%3d %-15s %llx %-5s %-10s %8.4f%%\n", id, pool, ++ (u_longlong_t)record->zi_guid, iotype_to_str(record->zi_iotype), ++ err_to_str(record->zi_error), freq); + + return (0); + } +@@ -510,25 +503,18 @@ print_delay_handler(int id, const char *pool, zinject_record_t *record, + return (0); + + if (*count == 0) { +- (void) printf("%3s %-15s %-16s %-10s %-5s %-9s " +- "%-6s %-6s\n", +- "ID", "POOL", "GUID", "DELAY (ms)", "LANES", "FREQ", +- "MATCH", "INJECT"); +- (void) printf("--- --------------- ---------------- " +- "---------- ----- --------- " +- "------ ------\n"); ++ (void) printf("%3s %-15s %-15s %-15s %s\n", ++ "ID", "POOL", "DELAY (ms)", "LANES", "GUID"); ++ (void) printf("--- --------------- --------------- " ++ "--------------- ----------------\n"); + } + + *count += 1; + +- double freq = record->zi_freq == 0 ? 100.0f : +- (((double)record->zi_freq) / ZI_PERCENTAGE_MAX) * 100.0f; +- +- (void) printf("%3d %-15s %llx %10llu %5llu %8.4f%% " +- "%6lu %6lu\n", id, pool, (u_longlong_t)record->zi_guid, ++ (void) printf("%3d %-15s %-15llu %-15llu %llx\n", id, pool, + (u_longlong_t)NSEC2MSEC(record->zi_timer), + (u_longlong_t)record->zi_nlanes, +- freq, record->zi_match_count, record->zi_inject_count); ++ (u_longlong_t)record->zi_guid); + + return (0); + } +diff --git a/include/sys/zfs_ioctl.h b/include/sys/zfs_ioctl.h +index a8c3ffc76455c4165252b367d39df8f9ba0efc6e..7297ac7f4b3ec9e11d76e85105011c528adbabd5 100644 +--- a/include/sys/zfs_ioctl.h ++++ b/include/sys/zfs_ioctl.h +@@ -421,8 +421,6 @@ typedef struct zinject_record { + uint64_t zi_nlanes; + uint32_t zi_cmd; + uint32_t zi_dvas; +- uint64_t zi_match_count; /* count of times matched */ +- uint64_t zi_inject_count; /* count of times injected */ + } zinject_record_t; + + #define ZINJECT_NULL 0x1 +diff --git a/module/zfs/zio_inject.c b/module/zfs/zio_inject.c +index f90044299cef4b936e71100f2c01e14750e4c7b6..05b8da3d4e51781bc988de7a8e247e8686a0f91f 100644 +--- a/module/zfs/zio_inject.c ++++ b/module/zfs/zio_inject.c +@@ -129,9 +129,6 @@ static boolean_t + zio_match_handler(const zbookmark_phys_t *zb, uint64_t type, int dva, + zinject_record_t *record, int error) + { +- boolean_t matched = B_FALSE; +- boolean_t injected = B_FALSE; +- + /* + * Check for a match against the MOS, which is based on type + */ +@@ -140,8 +137,9 @@ zio_match_handler(const zbookmark_phys_t *zb, uint64_t type, int dva, + record->zi_object == DMU_META_DNODE_OBJECT) { + if (record->zi_type == DMU_OT_NONE || + type == record->zi_type) +- matched = B_TRUE; +- goto done; ++ return (freq_triggered(record->zi_freq)); ++ else ++ return (B_FALSE); + } + + /* +@@ -155,20 +153,10 @@ zio_match_handler(const zbookmark_phys_t *zb, uint64_t type, int dva, + (record->zi_dvas == 0 || + (dva != ZI_NO_DVA && (record->zi_dvas & (1ULL << dva)))) && + error == record->zi_error) { +- matched = B_TRUE; +- goto done; +- } +- +-done: +- if (matched) { +- record->zi_match_count++; +- injected = freq_triggered(record->zi_freq); ++ return (freq_triggered(record->zi_freq)); + } + +- if (injected) +- record->zi_inject_count++; +- +- return (injected); ++ return (B_FALSE); + } + + /* +@@ -189,11 +177,8 @@ zio_handle_panic_injection(spa_t *spa, const char *tag, uint64_t type) + continue; + + if (handler->zi_record.zi_type == type && +- strcmp(tag, handler->zi_record.zi_func) == 0) { +- handler->zi_record.zi_match_count++; +- handler->zi_record.zi_inject_count++; ++ strcmp(tag, handler->zi_record.zi_func) == 0) + panic("Panic requested in function %s\n", tag); +- } + } + + rw_exit(&inject_lock); +@@ -351,8 +336,6 @@ zio_handle_label_injection(zio_t *zio, int error) + + if (zio->io_vd->vdev_guid == handler->zi_record.zi_guid && + (offset >= start && offset <= end)) { +- handler->zi_record.zi_match_count++; +- handler->zi_record.zi_inject_count++; + ret = error; + break; + } +@@ -443,16 +426,12 @@ zio_handle_device_injection_impl(vdev_t *vd, zio_t *zio, int err1, int err2) + + if (handler->zi_record.zi_error == err1 || + handler->zi_record.zi_error == err2) { +- handler->zi_record.zi_match_count++; +- + /* + * limit error injection if requested + */ + if (!freq_triggered(handler->zi_record.zi_freq)) + continue; + +- handler->zi_record.zi_inject_count++; +- + /* + * For a failed open, pretend like the device + * has gone away. +@@ -488,8 +467,6 @@ zio_handle_device_injection_impl(vdev_t *vd, zio_t *zio, int err1, int err2) + break; + } + if (handler->zi_record.zi_error == ENXIO) { +- handler->zi_record.zi_match_count++; +- handler->zi_record.zi_inject_count++; + ret = SET_ERROR(EIO); + break; + } +@@ -532,8 +509,6 @@ zio_handle_ignored_writes(zio_t *zio) + handler->zi_record.zi_cmd != ZINJECT_IGNORED_WRITES) + continue; + +- handler->zi_record.zi_match_count++; +- + /* + * Positive duration implies # of seconds, negative + * a number of txgs +@@ -546,10 +521,8 @@ zio_handle_ignored_writes(zio_t *zio) + } + + /* Have a "problem" writing 60% of the time */ +- if (random_in_range(100) < 60) { +- handler->zi_record.zi_inject_count++; ++ if (random_in_range(100) < 60) + zio->io_pipeline &= ~ZIO_VDEV_IO_STAGES; +- } + break; + } + +@@ -573,9 +546,6 @@ spa_handle_ignored_writes(spa_t *spa) + handler->zi_record.zi_cmd != ZINJECT_IGNORED_WRITES) + continue; + +- handler->zi_record.zi_match_count++; +- handler->zi_record.zi_inject_count++; +- + if (handler->zi_record.zi_duration > 0) { + VERIFY(handler->zi_record.zi_timer == 0 || + ddi_time_after64( +@@ -657,6 +627,9 @@ zio_handle_io_delay(zio_t *zio) + if (handler->zi_record.zi_cmd != ZINJECT_DELAY_IO) + continue; + ++ if (!freq_triggered(handler->zi_record.zi_freq)) ++ continue; ++ + if (vd->vdev_guid != handler->zi_record.zi_guid) + continue; + +@@ -679,12 +652,6 @@ zio_handle_io_delay(zio_t *zio) + ASSERT3U(handler->zi_record.zi_nlanes, >, + handler->zi_next_lane); + +- handler->zi_record.zi_match_count++; +- +- /* Limit the use of this handler if requested */ +- if (!freq_triggered(handler->zi_record.zi_freq)) +- continue; +- + /* + * We want to issue this IO to the lane that will become + * idle the soonest, so we compare the soonest this +@@ -756,9 +723,6 @@ zio_handle_io_delay(zio_t *zio) + */ + min_handler->zi_next_lane = (min_handler->zi_next_lane + 1) % + min_handler->zi_record.zi_nlanes; +- +- min_handler->zi_record.zi_inject_count++; +- + } + + mutex_exit(&inject_delay_mtx); +@@ -781,11 +745,9 @@ zio_handle_pool_delay(spa_t *spa, hrtime_t elapsed, zinject_type_t command) + handler = list_next(&inject_handlers, handler)) { + ASSERT3P(handler->zi_spa_name, !=, NULL); + if (strcmp(spa_name(spa), handler->zi_spa_name) == 0) { +- handler->zi_record.zi_match_count++; + uint64_t pause = + SEC2NSEC(handler->zi_record.zi_duration); + if (pause > elapsed) { +- handler->zi_record.zi_inject_count++; + delay = pause - elapsed; + } + id = handler->zi_id; +diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run +index 8e1ffab5b4ebecd247719a23648295b73719eb3f..ee1f29595222a3a140084bb81a65c2061d611de6 100644 +--- a/tests/runfiles/common.run ++++ b/tests/runfiles/common.run +@@ -159,7 +159,7 @@ tests = ['json_sanity'] + tags = ['functional', 'cli_root', 'json'] + + [tests/functional/cli_root/zinject] +-tests = ['zinject_args', 'zinject_counts', 'zinject_probe'] ++tests = ['zinject_args', 'zinject_probe'] + pre = + post = + tags = ['functional', 'cli_root', 'zinject'] +diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am +index 24eeac11299f4dc4fea096f41e8bca3c8731403a..52a0bd02818455147c152390c71018694a3723d6 100644 +--- a/tests/zfs-tests/tests/Makefile.am ++++ b/tests/zfs-tests/tests/Makefile.am +@@ -615,7 +615,6 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ + functional/cli_root/json/setup.ksh \ + functional/cli_root/json/json_sanity.ksh \ + functional/cli_root/zinject/zinject_args.ksh \ +- functional/cli_root/zinject/zinject_counts.ksh \ + functional/cli_root/zinject/zinject_probe.ksh \ + functional/cli_root/zdb/zdb_002_pos.ksh \ + functional/cli_root/zdb/zdb_003_pos.ksh \ +diff --git a/tests/zfs-tests/tests/functional/cli_root/zinject/zinject_counts.ksh b/tests/zfs-tests/tests/functional/cli_root/zinject/zinject_counts.ksh +deleted file mode 100755 +index 19b223aba46cba9a1689762affc6ef7d0325abc3..0000000000000000000000000000000000000000 +--- a/tests/zfs-tests/tests/functional/cli_root/zinject/zinject_counts.ksh ++++ /dev/null +@@ -1,142 +0,0 @@ +-#!/bin/ksh -p +-# +-# CDDL HEADER START +-# +-# The contents of this file are subject to the terms of the +-# Common Development and Distribution License (the "License"). +-# You may not use this file except in compliance with the License. +-# +-# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +-# or https://opensource.org/licenses/CDDL-1.0. +-# See the License for the specific language governing permissions +-# and limitations under the License. +-# +-# When distributing Covered Code, include this CDDL HEADER in each +-# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +-# If applicable, add the following below this CDDL HEADER, with the +-# fields enclosed by brackets "[]" replaced with your own identifying +-# information: Portions Copyright [yyyy] [name of copyright owner] +-# +-# CDDL HEADER END +-# +- +-# +-# Copyright (c) 2025, Klara, Inc. +-# +- +-# +-# This test sets various injections, does some IO to trigger them. and then +-# checks the "match" and "inject" counters on the injection records to ensure +-# that they're being counted properly. +-# +-# Note that this is a test of the counters, not injection generally. We're +-# usually only looking for the counters moving at all, not caring too much +-# about their actual values. +- +-. $STF_SUITE/include/libtest.shlib +- +-verify_runnable "global" +- +-log_assert "Check zinject counts are displayed and advanced as expected." +- +-DISK1=${DISKS%% *} +- +-function cleanup +-{ +- zinject -c all +- default_cleanup_noexit +-} +- +-log_onexit cleanup +- +-default_mirror_setup_noexit $DISKS +- +-# Call zinject, get the match and inject counts, and make sure they look +-# plausible for the requested frequency. +-function check_count_freq +-{ +- typeset -i freq=$1 +- +- # assuming a single rule, with the match and inject counts in the +- # last two columns +- typeset rule=$(zinject | grep -m 1 -oE '^ *[0-9].*[0-9]$') +- +- log_note "check_count_freq: using rule: $rule" +- +- typeset -a record=($(echo $rule | grep -oE ' [0-9]+ +[0-9]+$')) +- typeset -i match=${record[0]} +- typeset -i inject=${record[1]} +- +- log_note "check_count_freq: freq=$freq match=$match inject=$inject" +- +- # equality check, for 100% frequency, or if we've never matched the rule +- if [[ $match -eq 0 || $freq -eq 100 ]] ; then +- return [[ $match -eq 0 $inject ]] +- fi +- +- # Compute the expected injection count, and compare. Because we're +- # not testing the fine details here, it's considered good-enough for +- # the injection account to be within +/- 10% of the expected count. +- typeset -i expect=$(($match * $freq / 100)) +- typeset -i diff=$((($expect - $inject) / 10)) +- return [[ $diff -ge -1 && $diff -le 1 ]] +-} +- +-# Test device IO injections by injecting write errors, doing some writes, +-# and making sure the count moved +-function test_device_injection +-{ +- for freq in 100 50 ; do +- log_must zinject -d $DISK1 -e io -T write -f $freq $TESTPOOL +- +- log_must dd if=/dev/urandom of=/$TESTPOOL/file bs=1M count=1 +- log_must zpool sync +- +- log_must check_count_freq $freq +- +- log_must zinject -c all +- done +-} +- +-# Test object injections by writing a file, injecting checksum errors and +-# trying to read it back +-function test_object_injection +-{ +- log_must dd if=/dev/urandom of=/$TESTPOOL/file bs=1M count=1 +- zpool sync +- +- for freq in 100 50 ; do +- log_must zinject -t data -e checksum -f $freq /$TESTPOOL/file +- +- cat /tank/file > /dev/null || true +- +- log_must check_count_freq $freq +- +- log_must zinject -c all +- done +-} +- +-# Test delay injections, by injecting delays and writing +-function test_delay_injection +-{ +- for freq in 100 50 ; do +- log_must zinject -d $DISK1 -D 50:1 -f $freq $TESTPOOL +- +- log_must dd if=/dev/urandom of=/$TESTPOOL/file bs=1M count=1 +- zpool sync +- +- log_must check_count_freq $freq +- +- log_must zinject -c all +- done +-} +- +-# Disable cache, to ensure reads induce IO +-log_must zfs set primarycache=none $TESTPOOL +- +-# Test 'em all. +-log_must test_device_injection +-log_must test_object_injection +-log_must test_delay_injection +- +-log_pass "zinject counts are displayed and advanced as expected." diff --git a/debian/patches/series b/debian/patches/series index e3103f9b4..71bce2b7e 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -9,3 +9,4 @@ 0009-zpool-status-tighten-bounds-for-noalloc-stat-availab.patch 0010-linux-zvols-correctly-detect-flush-requests-17131.patch 0011-contrib-initramfs-use-LVM-autoactivation-for-activat.patch +0012-Revert-zinject-count-matches-and-injections-for-each.patch -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel