Re: [PATCH blktests v4 1/3] bcache: add bcache/001

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

Thanks. I ran this test case and it passes in my environment. Good.

Please find my comments in line. Most of them are nit ones, and I do no
care them much. Just wanted to hear your opinion about my comment on partprobe.

On Feb 12, 2026 / 16:23, Daniel Wagner wrote:
[...]
> diff --git a/tests/bcache/001 b/tests/bcache/001
> new file mode 100755
> index 000000000000..7258d87566cb
> --- /dev/null
> +++ b/tests/bcache/001
[...]
> +test_device_array() {
> +	echo "Running ${TEST_NAME}"
> +
> +	if [[ ${#TEST_DEV_ARRAY[@]} -lt 3 ]]; then
> +		SKIP_REASONS+=("requires at least 3 devices")
> +		return 1
> +	fi
> +
> +	_setup_bcache "${TEST_DEV_ARRAY[@]}"
> +
> +	local bcache_nodes

Nit: I think "local -a bcache_nodes" is the better.

[...]

> diff --git a/tests/bcache/rc b/tests/bcache/rc
> new file mode 100644
> index 000000000000..cfd4094c2fe0
> --- /dev/null
> +++ b/tests/bcache/rc
> @@ -0,0 +1,375 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2026 Daniel Wagner, SUSE Labs
> +
> +. common/rc
> +
> +declare BCACHE_DEVS_LIST
> +
> +BCACHE_MAX_RETRIES=5
> +
> +group_requires() {
> +	_have_kernel_options MD BCACHE BCACHE_DEBUG AUTOFS_FS
> +	_have_program make-bcache
> +	_have_crypto_algorithm crc32c
> +}
> +
> +_bcache_wipe_devs() {
> +	local devs=("$@")
> +	local dev
> +
> +	for dev in "${devs[@]}"; do
> +		# Attempt a clean wipe first
> +		if wipefs --all --quiet "${dev}" 2>/dev/null; then
> +			continue
> +		fi
> +
> +		# Overwrite the first 10MB to clear stubborn partition tables or metadata
> +		if ! dd if=/dev/zero of="${dev}" bs=1M count=10 conv=notrunc status=none; then
> +			echo "Error: dd failed on ${dev}" >&2
> +		fi
> +
> +		# Wipe the Tail (Last 5MB)
> +		# bcache often places backup superblocks at the end of the device.
> +		local dev_size_mb
> +		dev_size_mb=$(blockdev --getsize64 "$dev" | awk '{print int($1 / 1024 / 1024)}')
> +
> +		if [ "$dev_size_mb" -gt 10 ]; then
> +			local seek_pos=$((dev_size_mb - 5))
> +			dd if=/dev/zero of="${dev}" bs=1M count=5 seek=$seek_pos conv=fsync status=none
> +		fi
> +
> +		# Refresh kernel partition table & wait for udev
> +		partprobe "$dev" 2>/dev/null

I think _have_program partprobe is required, or can we replace it with
"blockdev --rereadpt"?

> +		udevadm settle
> +
> +		# Try wiping again after clearing the headers
> +		if ! wipefs --all --quiet --force "${dev}"; then
> +			echo "Warning: Failed to wipe ${dev} even after dd." >&2
> +		fi
> +	done
> +}
> +
> +_bcache_register() {
> +	local devs=("$@")
> +	local dev timeout=0
> +
> +	while [[ ! -w /sys/fs/bcache/register ]] && (( timeout < 10 )); do
> +		sleep 1
> +		(( timeout ++ ))
> +	done
> +
> +	if [[ ! -w /sys/fs/bcache/register ]]; then
> +		echo "ERROR: bcache registration interface not found." >&2
> +		return 1
> +	fi
> +
> +	for dev in "${devs[@]}"; do
> +		local tmp_err
> +
> +		tmp_err="/tmp/bcache_reg_$$.err"
> +		if ! echo "${dev}" > /sys/fs/bcache/register 2> "${tmp_err}"; then
> +			local err_msg
> +
> +			err_msg=$(< "${tmp_err}")
> +			if [[ "${err_msg}" != *"Device or resource busy"* ]]; then
> +				echo "ERROR: Failed to register ${dev}: ${err_msg:-"Unknown error"}" >&2
> +			fi
> +		fi
> +		rm -f "${tmp_err}"
> +	done
> +}
> +
> +_create_bcache() {
> +	local -a cdevs=()
> +	local -a bdevs=()
> +	local -a ARGS=()
> +	local -a created_devs=()
> +	local bucket_size="64k"
> +	local block_size="4k"

Nit: I think "local dev" can be added here.

> +
> +	while [[ $# -gt 0 ]]; do
> +		case $1 in
> +			--cache)
> +				shift
> +				# Collect arguments until the next flag or end of input
> +				while [[ $# -gt 0 && ! $1 =~ ^-- ]]; do
> +					cdevs+=("$1")
> +					shift
> +				done
> +				;;
> +			--bdev)
> +				shift
> +				# Collect arguments until the next flag or end of input
> +				while [[ $# -gt 0 && ! $1 =~ ^-- ]]; do
> +					bdevs+=("$1")
> +					shift
> +				done
> +				;;
> +			--bucket-size)
> +				bucket_size="$2"
> +				shift 2
> +				;;
> +			--block-size)
> +				block_size="$2"
> +				shift 2
> +				;;
> +			--writeback)
> +				ARGS+=(--writeback)
> +				shift 1
> +				;;
> +			--discard)
> +				ARGS+=(--discard)
> +				shift 1
> +				;;
> +			*)
> +				echo "WARNING: unknown argument: $1"
> +				shift
> +				;;
> +		esac
> +	done
> +
> +	# add /dev prefix to device names
> +	cdevs=( "${cdevs[@]/#/\/dev\/}" )
> +	bdevs=( "${bdevs[@]/#/\/dev\/}" )
> +
> +	# make-bcache expects empty/cleared devices
> +	_bcache_wipe_devs "${cdevs[@]}" "${bdevs[@]}"
> +
> +	local -a cmd
> +	cmd=(make-bcache --wipe-bcache \
> +			--bucket "${bucket_size}" \
> +			--block "${block_size}")
> +	for dev in "${cdevs[@]}"; do cmd+=("--cache" "${dev}"); done
> +	for dev in "${bdevs[@]}"; do cmd+=("--bdev" "${dev}"); done
> +	cmd+=("${ARGS[@]}")
> +
> +	local output rc
> +	output=$("${cmd[@]}" 2>&1)
> +	rc="$?"
> +	if [[ "${rc}" -ne 0 ]]; then
> +		echo "ERROR: make-bcache failed:" >&2
> +		echo "$output" >&2
> +		return 1
> +	fi
> +
> +	local cset_uuid
> +	cset_uuid=$(echo "$output" | awk '/Set UUID:/ {print $3}' | head -n 1)
> +	if [[ -z "${cset_uuid}" ]]; then
> +		echo "ERROR: Could not extract cset UUID from make-bcache output" >&2
> +		return 1
> +	fi
> +
> +	local -a bdev_uuids
> +	mapfile -t bdev_uuids < <(echo "$output" | awk '
> +	  $1 == "UUID:" { last_uuid = $2 }
> +	  $1 == "version:" && $2 == "1" { print last_uuid}
> +	')
> +
> +	_bcache_register "${cdevs[@]}" "${bdevs[@]}"
> +	udevadm settle
> +
> +	for uuid in "${bdev_uuids[@]}"; do
> +		local link found
> +
> +		link=/dev/bcache/by-uuid/"${uuid}"
> +		found=false
> +
> +		for ((i=0; i<BCACHE_MAX_RETRIES; i++)); do
> +			if [[ -L "${link}" ]]; then
> +				created_devs+=("$(readlink -f "${link}")")
> +				found=true
> +				break
> +			fi
> +
> +			# poke udev to create the links
> +			udevadm trigger "block/$(basename "$(readlink -f "${link}" 2>/dev/null || echo "notfound")")" 2>/dev/null
> +			sleep 1
> +		done
> +
> +		if [[ "${found}" == "false" ]]; then
> +			echo "WARNING: Could not find device node for UUID ${uuid} after ${BCACHE_MAX_RETRIES}s" >&2
> +		fi
> +	done
> +
> +	printf "%s\n" "${created_devs[@]}"
> +}
> +
> +_remove_bcache() {
> +	local -a cdevs=()
> +	local -a bdevs=()
> +	local -a csets=()
> +	local -a bcache_devs=()
> +	local uuid
> +
> +	while [[ $# -gt 0 ]]; do
> +		case $1 in
> +			--cache)
> +				shift
> +				# Collect arguments until the next flag or end of input
> +				while [[ $# -gt 0 && ! $1 =~ ^-- ]]; do
> +					cdevs+=("$1")
> +					shift
> +				done
> +				;;
> +			--bdev)
> +				shift
> +				# Collect arguments until the next flag or end of input
> +				while [[ $# -gt 0 && ! $1 =~ ^-- ]]; do
> +					bdevs+=("$1")
> +					shift
> +				done
> +				;;
> +			--bcache)
> +				shift
> +				# Collect arguments until the next flag or end of input
> +				while [[ $# -gt 0 && ! $1 =~ ^-- ]]; do
> +					bcache_devs+=("$1")
> +					shift
> +				done
> +				;;
> +			*)
> +				echo "WARNING: unknown argument: $1"
> +				shift
> +				;;
> +		esac
> +	done
> +
> +	for dev in "${bcache_devs[@]}"; do
> +		local bcache bcache_dir
> +
> +		if mountpoint -q "${dev}" 2>/dev/null; then
> +			umount -l "${dev}"

Nit: The -q and -l options can be replaced with longer one for readability,
     --quiet and --lazy.

> +		fi
> +
> +		bcache="${dev##*/}"
> +		bcache_dir=/sys/block/"${bcache}"/bcache
> +		if [ -f "${bcache_dir}"/stop ]; then
> +			echo 1 > "${bcache_dir}"/stop
> +		fi
> +	done
> +
> +	# The cache could be detached, thus go through all caches and
> +	# look for the cdev in there.
> +	local cset_path
> +	for cset_path in /sys/fs/bcache/*-*-*-*-*; do
> +		local cache_link match_found
> +
> +		match_found=false
> +		for cache_link in "${cset_path}"/cache[0-9]*; do
> +			local full_sys_path _cdev cdev
> +
> +			full_sys_path="$(readlink -f "$cache_link")"
> +			_cdev="$(basename "${full_sys_path%/bcache}")"
> +
> +			for cdev in "${cdevs[@]}"; do
> +				if [ "${_cdev}" == "$(basename "${cdev}")"  ]; then
> +					match_found=true
> +					break 2
> +				fi
> +			done
> +		done
> +
> +		if [ "${match_found}" = false ]; then
> +			continue
> +		fi
> +
> +		cset="$(basename "${cset_path}")"
> +		if [ -d /sys/fs/bcache/"${cset}" ]; then
> +			echo 1 > /sys/fs/bcache/"${cset}"/unregister
> +			csets+=("${cset}")
> +		fi
> +	done
> +
> +	udevadm settle
> +
> +	local timeout
> +	for cset in "${csets[@]}"; do
> +		timeout=0
> +		while [[ -d /sys/fs/bcache/"${cset}" ]] && (( timeout < 10 )); do
> +			sleep 0.5
> +			(( timeout++ ))
> +		done
> +	done
> +
> +	_bcache_wipe_devs "${cdevs[@]}" "${bdevs[@]}"
> +}
> +
> +_cleanup_bcache() {
> +	local cset dev bcache bcache_devs cset_path

Nit: I think 'bdev' can be added in the list above.

> +	local -a csets=()
> +
> +	read -r -a bcache_devs <<< "${BCACHE_DEVS_LIST:-}"
> +
> +	# Don't let successive Ctrl-Cs interrupt the cleanup processes
> +	trap '' SIGINT
> +
> +	shopt -s nullglob
> +	for bcache  in /sys/block/bcache* ; do
> +		[ -e "${bcache}" ] || continue
> +
> +		if [[ -f "${bcache}/bcache/backing_dev_name" ]]; then
> +			bdev=$(basename "$(cat "${bcache}/bcache/backing_dev_name")")
> +
> +			for dev in "${bcache_devs[@]}"; do
> +				if [[ "${bdev}" == "$(basename "${dev}")" ]]; then
> +					echo "WARNING: Stopping bcache device ${bdev}"
> +					echo 1 > /sys/block/"${bdev}"/bcache/stop 2>/dev/null
> +					break
> +				fi
> +			done
> +		fi
> +	done
> +
> +	for cset_path in /sys/fs/bcache/*-*-*-*-*; do
> +		local cache_link match_found
> +
> +		match_found=false
> +		for cache_link in "${cset_path}"/cache[0-9]*; do
> +			local full_sys_path cdev
> +
> +			full_sys_path="$(readlink -f "$cache_link")"
> +			cdev="$(basename "${full_sys_path%/bcache}")"
> +
> +			for dev in "${bcache_devs[@]}"; do
> +				if [ "${cdev}" == "$(basename "${dev}")" ]; then
> +					match_found=true
> +					break 2
> +				fi
> +			done
> +		done
> +
> +		if [ "${match_found}" = false ]; then
> +			continue
> +		fi
> +
> +		cset="$(basename "${cset_path}")"
> +		if [ -d /sys/fs/bcache/"${cset}" ]; then
> +			echo "WARNING: Unregistering cset $(basename "${cset}")"
> +			echo 1 > /sys/fs/bcache/"${cset}"/unregister
> +			csets+=("${cset}")
> +		fi
> +	done
> +	shopt -u nullglob
> +
> +	udevadm settle
> +
> +	local timeout
> +	for cset in "${csets[@]}"; do
> +		timeout=0
> +		while [[ -d /sys/fs/bcache/"${cset}" ]] && (( timeout < 10 )); do
> +			sleep 0.5
> +			(( timeout++ ))
> +		done
> +	done
> +
> +	_bcache_wipe_devs "${bcache_devs[@]}"
> +
> +	trap SIGINT
> +}
> +
> +_setup_bcache() {
> +	BCACHE_DEVS_LIST="$*"
> +
> +	_register_test_cleanup _cleanup_bcache
> +}
> 
> -- 
> 2.53.0
>