Re: [PATCH] selftests: ublk: propagate recovery worker failures

[Ming Lei]

On Mon, Nov 09, 2026 at 11:58:13PM +0800, shao.mingyin@xxxxxxxxxx wrote:
> From: Shao Mingyin <shao.mingyin@xxxxxxxxxx>
> 
> The recovery tests update ERR_CODE in background subshells and use wait
> without PIDs. A worker can report a failure while the parent still prints
> PASS and exits with zero. Calling _show_result() from a failed worker also
> removes shared backing files while other workers may still be using them.
> 
> Wait for each recorded worker PID and aggregate its status in the parent,
> keeping failures ahead of skips and preserving errors across batches.
> Return worker and device-creation errors without reporting or cleaning up
> from the child, so the parent can finish waiting before shared cleanup.
> 
> Fixes: 57e13a2e8cd2 ("selftests: ublk: support user recovery")
> Fixes: 533c87e2ed74 ("selftests: ublk: add test for UBLK_F_QUIESCE")
> Fixes: 63276182c513 ("selftests: ublk: add user copy test cases")
> Signed-off-by: Shao Mingyin <shao.mingyin@xxxxxxxxxx>
> ---
> Tested on native x86_64 Bash 5.2 with the recovery scripts and common
> helpers from the unpatched and patched trees. Device and I/O operations
> were replaced by test-only stubs. Single-worker failures at every launch

Please run the real tests next time, it just needs one kernel with
ublk enabled, and stubs can't cover what the tests are supposed to
verify.

> position, multiple failures, skips, signal termination and device-creation
> errors no longer produce a final PASS or a zero exit status. Successful
> runs still pass, and cleanup follows all worker exits. Prerequisite skips
> still exit before preparation. The 71 scenarios passed their expected
> checks on each tree. This does not exercise real ublk devices or I/O.
> 
>  tools/testing/selftests/ublk/test_common.sh   | 23 +++++++++++++++++--
>  .../testing/selftests/ublk/test_recover_01.sh | 20 ++++++++++++----
>  .../testing/selftests/ublk/test_recover_02.sh | 20 ++++++++++++----
>  .../testing/selftests/ublk/test_recover_03.sh | 20 ++++++++++++----
>  .../testing/selftests/ublk/test_recover_04.sh | 14 ++++++++---
>  5 files changed, 80 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/ublk/test_common.sh b/tools/testing/selftests/ublk/test_common.sh
> index af2ea4fa1..c5adb4d29 100755
> --- a/tools/testing/selftests/ublk/test_common.sh
> +++ b/tools/testing/selftests/ublk/test_common.sh
> @@ -379,6 +379,26 @@ run_io_and_kill_daemon()
>  	fi
>  }
>  
> +# Collect recovery workers in the parent before removing shared backing files.
> +_wait_recovery_tests()
> +{
> +	local pid
> +	local ret
> +
> +	for pid in "$@"; do
> +		if wait "$pid"; then
> +			continue
> +		else
> +			ret=$?
> +		fi

Nit: the above can be simplified as:

		wait "$pid" && continue
		ret=$?

Not a big deal.

BTW, test_stress_*.sh have the same issue (ERR_CODE set in background
worker + bare 'wait'), which may be handled in a follow-up.

Reviewed-by: Ming Lei <tom.leiming@xxxxxxxxx>


Thanks, 
Ming