On Mar 18, 2025 / 15:28, Zhang Yi wrote:
> From: Zhang Yi <yi.zh...@huawei.com>
>
> Test block device unmap write zeroes sysfs interface with device-mapper
> stacked devices. The /sys/block/<disk>/queue/write_zeroes_unmap
> interface should return 1 if the underlying devices support the unmap
> write zeroes command, and it should return 0 otherwise.
>
> Signed-off-by: Zhang Yi <yi.zh...@huawei.com>
> ---
> common/rc | 16 ++++++++++++++
> tests/dm/003 | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
> tests/dm/003.out | 2 ++
> 3 files changed, 75 insertions(+)
> create mode 100755 tests/dm/003
> create mode 100644 tests/dm/003.out
>
> diff --git a/common/rc b/common/rc
> index bc6c2e4..60c21f2 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -615,3 +615,19 @@ _io_uring_restore()
> echo "$IO_URING_DISABLED" > /proc/sys/kernel/io_uring_disabled
> fi
> }
> +
> +# get real device path name by following link
> +_real_dev()
> +{
> + local dev=$1
> + if [ -b "$dev" ] && [ -L "$dev" ]; then
> + dev=`readlink -f "$dev"`
> + fi
> + echo $dev
> +}
This helper function looks useful, and it looks reasonable to add it.
> +
> +# basename of a device
> +_short_dev()
> +{
> + echo `basename $(_real_dev $1)`
> +}
But I'm not sure about this one. The name "_short_dev" is not super
clear for me.
> diff --git a/tests/dm/003 b/tests/dm/003
> new file mode 100755
> index 0000000..1013eb5
> --- /dev/null
> +++ b/tests/dm/003
> @@ -0,0 +1,57 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2025 Huawei.
> +#
> +# Test block device unmap write zeroes sysfs interface with device-mapper
> +# stacked devices.
> +
> +. tests/dm/rc
> +. common/scsi_debug
> +
> +DESCRIPTION="test unmap write zeroes sysfs interface with dm devices"
> +QUICK=1
> +
> +requires() {
> + _have_scsi_debug
> +}
> +
> +device_requries() {
> + _require_test_dev_sysfs queue/write_zeroes_unmap
> +}
Same comment as the 1st patch: device_requries() does not work here.
> +
> +setup_test_device() {
> + if ! _configure_scsi_debug "$@"; then
> + return 1
> + fi
In same manner as the 1st patch, I suggest to check /queue/write_zeroes_unmap
here.
if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap
]]; then
_exit_scsi_debug
SKIP_REASONS+=("kernel does not support unmap write zeroes
sysfs interface")
return 1
fi
The caller will need to check setup_test_device() return value.
> +
> + local dev="/dev/${SCSI_DEBUG_DEVICES[0]}"
> + local blk_sz="$(blockdev --getsz "$dev")"
> + dmsetup create test --table "0 $blk_sz linear $dev 0"
I suggest to call _real_dev() here, and echo back the device name.
dpath=$(_real_dev /dev/mapper/test)
echo ${dpath##*/}
The bash parameter expansion ${xxx##*/} works in same manner as the basename
command. The caller can receive the device name in a local variable. This will
avoid a bit of code duplication, and allow to avoid _short_dev().
> +}
> +
> +cleanup_test_device() {
> + dmsetup remove test
> + _exit_scsi_debug
> +}
> +
> +test() {
> + echo "Running ${TEST_NAME}"
> +
> + # disable WRITE SAME with unmap
> + setup_test_device lbprz=0
> + umap="$(cat "/sys/block/$(_short_dev
> /dev/mapper/test)/queue/write_zeroes_unmap")"
I suggest to modify the two lines above as follows, to match with the other
suggested changes:
local dname umap
if ! dname=$(setup_test_device lbprz=0); then
return 1
fi
umap="$(< "/sys/block/${dname}/queue/write_zeroes_unmap")"
(Please note that the suggested changes are untested)
