| Message ID | 20250318072835.3508696-3-yi.zhang@huaweicloud.com |
|---|---|
| State | New |
| Headers | show |
| Series | blktest: add unmap write zeroes tests | expand |
On Mar 18, 2025 / 15:28, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@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.zhang@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)
On 2025/4/3 15:43, Shinichiro Kawasaki wrote: > On Mar 18, 2025 / 15:28, Zhang Yi wrote: >> From: Zhang Yi <yi.zhang@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.zhang@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. > I copied these two helpers form the xfstests. :) >> 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. Sure. > >> + >> + 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(). > I'm afraid this approach will not work since we may set the SKIP_REASONS parameter. We cannot pass the device name in this manner as it will overlook the SKIP_REASONS setting when the caller invokes $(setup_test_device xxx), this function runs in a subshell. If you don't like _short_dev(), I think we can pass dname through a global variable, something like below: setup_test_device() { ... dpath=$(_real_dev /dev/mapper/test) dname=${dpath##*/} } if ! setup_test_device lbprz=0; then return 1 fi umap="$(< "/sys/block/${dname}/queue/write_zeroes_unmap")" What do you think? Thanks, Yi. >> +} >> + >> +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)
On Apr 28, 2025 / 12:32, Zhang Yi wrote: > On 2025/4/3 15:43, Shinichiro Kawasaki wrote: [...] > >> + > >> +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. > > Sure. > > > > >> + > >> + 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(). > > > > I'm afraid this approach will not work since we may set the > SKIP_REASONS parameter. We cannot pass the device name in this > manner as it will overlook the SKIP_REASONS setting when the caller > invokes $(setup_test_device xxx), this function runs in a subshell. Ah, that's right. SKIP_REASONS modification in subshell won't work. > > If you don't like _short_dev(), I think we can pass dname through a > global variable, something like below: > > setup_test_device() { > ... > dpath=$(_real_dev /dev/mapper/test) > dname=${dpath##*/} > } > > if ! setup_test_device lbprz=0; then > return 1 > fi > umap="$(< "/sys/block/${dname}/queue/write_zeroes_unmap")" > > What do you think? I think global variable is a bit dirty. So my suggestion is to still echo back the short device name from the helper, and set the SKIP_REASONS after calling the helper, as follows: diff --git a/tests/dm/003 b/tests/dm/003 index 1013eb5..e00fa99 100755 --- a/tests/dm/003 +++ b/tests/dm/003 @@ -20,13 +20,23 @@ device_requries() { } setup_test_device() { + local dev blk_sz dpath + if ! _configure_scsi_debug "$@"; then return 1 fi - local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" - local blk_sz="$(blockdev --getsz "$dev")" + if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap ]]; then + _exit_scsi_debug + return 1 + fi + + dev="/dev/${SCSI_DEBUG_DEVICES[0]}" + blk_sz="$(blockdev --getsz "$dev")" dmsetup create test --table "0 $blk_sz linear $dev 0" + + dpath=$(_real_dev /dev/mapper/test) + echo ${dpath##*/} } cleanup_test_device() { @@ -38,17 +48,21 @@ 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")" + local dname + if ! dname=$(setup_test_device lbprz=0); then + SKIP_REASONS+=("kernel does not support unmap write zeroes sysfs interface") + return 1 + fi + umap="$(cat "/sys/block/${dname}/queue/zoned")" if [[ $umap -ne 0 ]]; then echo "Test disable WRITE SAME with unmap failed." fi cleanup_test_device
On 2025/4/28 16:13, Shinichiro Kawasaki wrote: > On Apr 28, 2025 / 12:32, Zhang Yi wrote: >> On 2025/4/3 15:43, Shinichiro Kawasaki wrote: > [...] >>>> + >>>> +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. >> >> Sure. >> >>> >>>> + >>>> + 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(). >>> >> >> I'm afraid this approach will not work since we may set the >> SKIP_REASONS parameter. We cannot pass the device name in this >> manner as it will overlook the SKIP_REASONS setting when the caller >> invokes $(setup_test_device xxx), this function runs in a subshell. > > Ah, that's right. SKIP_REASONS modification in subshell won't work. > >> >> If you don't like _short_dev(), I think we can pass dname through a >> global variable, something like below: >> >> setup_test_device() { >> ... >> dpath=$(_real_dev /dev/mapper/test) >> dname=${dpath##*/} >> } >> >> if ! setup_test_device lbprz=0; then >> return 1 >> fi >> umap="$(< "/sys/block/${dname}/queue/write_zeroes_unmap")" >> >> What do you think? > > I think global variable is a bit dirty. So my suggestion is to still echo back > the short device name from the helper, and set the SKIP_REASONS after calling > the helper, as follows: > > diff --git a/tests/dm/003 b/tests/dm/003 > index 1013eb5..e00fa99 100755 > --- a/tests/dm/003 > +++ b/tests/dm/003 > @@ -20,13 +20,23 @@ device_requries() { > } > > setup_test_device() { > + local dev blk_sz dpath > + > if ! _configure_scsi_debug "$@"; then > return 1 Hmm, if we encounter an error here, the test will be skipped instead of returning a failure. This is not the expected outcome. Thanks, Yi. > fi > > - local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" > - local blk_sz="$(blockdev --getsz "$dev")" > + if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap ]]; then > + _exit_scsi_debug > + return 1 > + fi > + > + dev="/dev/${SCSI_DEBUG_DEVICES[0]}" > + blk_sz="$(blockdev --getsz "$dev")" > dmsetup create test --table "0 $blk_sz linear $dev 0" > + > + dpath=$(_real_dev /dev/mapper/test) > + echo ${dpath##*/} > } > > cleanup_test_device() { > @@ -38,17 +48,21 @@ 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")" > + local dname > + if ! dname=$(setup_test_device lbprz=0); then > + SKIP_REASONS+=("kernel does not support unmap write zeroes sysfs interface") > + return 1 > + fi > + umap="$(cat "/sys/block/${dname}/queue/zoned")" > if [[ $umap -ne 0 ]]; then > echo "Test disable WRITE SAME with unmap failed." > fi > cleanup_test_device >
On Apr 28, 2025 / 17:04, Zhang Yi wrote: > On 2025/4/28 16:13, Shinichiro Kawasaki wrote: > > On Apr 28, 2025 / 12:32, Zhang Yi wrote: > >> On 2025/4/3 15:43, Shinichiro Kawasaki wrote: > > [...] > >>>> + > >>>> +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. > >> > >> Sure. > >> > >>> > >>>> + > >>>> + 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(). > >>> > >> > >> I'm afraid this approach will not work since we may set the > >> SKIP_REASONS parameter. We cannot pass the device name in this > >> manner as it will overlook the SKIP_REASONS setting when the caller > >> invokes $(setup_test_device xxx), this function runs in a subshell. > > > > Ah, that's right. SKIP_REASONS modification in subshell won't work. > > > >> > >> If you don't like _short_dev(), I think we can pass dname through a > >> global variable, something like below: > >> > >> setup_test_device() { > >> ... > >> dpath=$(_real_dev /dev/mapper/test) > >> dname=${dpath##*/} > >> } > >> > >> if ! setup_test_device lbprz=0; then > >> return 1 > >> fi > >> umap="$(< "/sys/block/${dname}/queue/write_zeroes_unmap")" > >> > >> What do you think? > > > > I think global variable is a bit dirty. So my suggestion is to still echo back > > the short device name from the helper, and set the SKIP_REASONS after calling > > the helper, as follows: > > > > diff --git a/tests/dm/003 b/tests/dm/003 > > index 1013eb5..e00fa99 100755 > > --- a/tests/dm/003 > > +++ b/tests/dm/003 > > @@ -20,13 +20,23 @@ device_requries() { > > } > > > > setup_test_device() { > > + local dev blk_sz dpath > > + > > if ! _configure_scsi_debug "$@"; then > > return 1 > > Hmm, if we encounter an error here, the test will be skipped instead of > returning a failure. This is not the expected outcome. Ah, rigth. That's not good. How about to return differnt values for the failure case above, > > Thanks, > Yi. > > > fi > > > > - local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" > > - local blk_sz="$(blockdev --getsz "$dev")" > > + if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap ]]; then > > + _exit_scsi_debug > > + return 1 and this "should skip" case? diff --git a/tests/dm/003 b/tests/dm/003 index 1013eb5..5e617fd 100755 --- a/tests/dm/003 +++ b/tests/dm/003 @@ -19,14 +19,26 @@ device_requries() { _require_test_dev_sysfs queue/write_zeroes_unmap } +readonly TO_SKIP=255 + setup_test_device() { + local dev blk_sz dpath + if ! _configure_scsi_debug "$@"; then return 1 fi - local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" - local blk_sz="$(blockdev --getsz "$dev")" + if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap ]]; then + _exit_scsi_debug + return $TO_SKIP + fi + + dev="/dev/${SCSI_DEBUG_DEVICES[0]}" + blk_sz="$(blockdev --getsz "$dev")" dmsetup create test --table "0 $blk_sz linear $dev 0" + + dpath=$(_real_dev /dev/mapper/test) + echo "${dpath##*/}" } cleanup_test_device() { @@ -38,17 +50,25 @@ 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")" + local dname + dname=$(setup_test_device lbprz=0) + ret=$? + if ((ret)); then + if ((ret == TO_SKIP)); then + SKIP_REASONS+=("kernel does not support unmap write zeroes sysfs interface") + fi + return 1 + fi + umap="$(cat "/sys/block/${dname}/queue/zoned")" if [[ $umap -ne 0 ]]; then echo "Test disable WRITE SAME with unmap failed." fi cleanup_test_device
On 2025/4/28 17:25, Shinichiro Kawasaki wrote: > On Apr 28, 2025 / 17:04, Zhang Yi wrote: >> On 2025/4/28 16:13, Shinichiro Kawasaki wrote: >>> On Apr 28, 2025 / 12:32, Zhang Yi wrote: >>>> On 2025/4/3 15:43, Shinichiro Kawasaki wrote: >>> [...] >>>>>> + >>>>>> +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. >>>> >>>> Sure. >>>> >>>>> >>>>>> + >>>>>> + 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(). >>>>> >>>> >>>> I'm afraid this approach will not work since we may set the >>>> SKIP_REASONS parameter. We cannot pass the device name in this >>>> manner as it will overlook the SKIP_REASONS setting when the caller >>>> invokes $(setup_test_device xxx), this function runs in a subshell. >>> >>> Ah, that's right. SKIP_REASONS modification in subshell won't work. >>> >>>> >>>> If you don't like _short_dev(), I think we can pass dname through a >>>> global variable, something like below: >>>> >>>> setup_test_device() { >>>> ... >>>> dpath=$(_real_dev /dev/mapper/test) >>>> dname=${dpath##*/} >>>> } >>>> >>>> if ! setup_test_device lbprz=0; then >>>> return 1 >>>> fi >>>> umap="$(< "/sys/block/${dname}/queue/write_zeroes_unmap")" >>>> >>>> What do you think? >>> >>> I think global variable is a bit dirty. So my suggestion is to still echo back >>> the short device name from the helper, and set the SKIP_REASONS after calling >>> the helper, as follows: >>> >>> diff --git a/tests/dm/003 b/tests/dm/003 >>> index 1013eb5..e00fa99 100755 >>> --- a/tests/dm/003 >>> +++ b/tests/dm/003 >>> @@ -20,13 +20,23 @@ device_requries() { >>> } >>> >>> setup_test_device() { >>> + local dev blk_sz dpath >>> + >>> if ! _configure_scsi_debug "$@"; then >>> return 1 >> >> Hmm, if we encounter an error here, the test will be skipped instead of >> returning a failure. This is not the expected outcome. > > Ah, rigth. That's not good. > How about to return differnt values for the failure case above, > >> >> Thanks, >> Yi. >> >>> fi >>> >>> - local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" >>> - local blk_sz="$(blockdev --getsz "$dev")" >>> + if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap ]]; then >>> + _exit_scsi_debug >>> + return 1 > > and this "should skip" case? > > > diff --git a/tests/dm/003 b/tests/dm/003 > index 1013eb5..5e617fd 100755 > --- a/tests/dm/003 > +++ b/tests/dm/003 > @@ -19,14 +19,26 @@ device_requries() { > _require_test_dev_sysfs queue/write_zeroes_unmap > } > > +readonly TO_SKIP=255 > + > setup_test_device() { > + local dev blk_sz dpath > + > if ! _configure_scsi_debug "$@"; then > return 1 > fi > > - local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" > - local blk_sz="$(blockdev --getsz "$dev")" > + if [[ ! -f /sys/block/${SCSI_DEBUG_DEVICES[0]}/queue/write_zeroes_unmap ]]; then > + _exit_scsi_debug > + return $TO_SKIP > + fi > + > + dev="/dev/${SCSI_DEBUG_DEVICES[0]}" > + blk_sz="$(blockdev --getsz "$dev")" > dmsetup create test --table "0 $blk_sz linear $dev 0" > + > + dpath=$(_real_dev /dev/mapper/test) > + echo "${dpath##*/}" > } > > cleanup_test_device() { > @@ -38,17 +50,25 @@ 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")" > + local dname > + dname=$(setup_test_device lbprz=0) > + ret=$? > + if ((ret)); then > + if ((ret == TO_SKIP)); then > + SKIP_REASONS+=("kernel does not support unmap write zeroes sysfs interface") > + fi > + return 1 > + fi > + umap="$(cat "/sys/block/${dname}/queue/zoned")" > if [[ $umap -ne 0 ]]; then > echo "Test disable WRITE SAME with unmap failed." > fi > cleanup_test_device > Yeah, I believe this will work, and it looks good to me. Thank you for the suggestion！ Thanks, Yi.
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 +} + +# basename of a device +_short_dev() +{ + echo `basename $(_real_dev $1)` +} 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 +} + +setup_test_device() { + if ! _configure_scsi_debug "$@"; then + return 1 + fi + + local dev="/dev/${SCSI_DEBUG_DEVICES[0]}" + local blk_sz="$(blockdev --getsz "$dev")" + dmsetup create test --table "0 $blk_sz linear $dev 0" +} + +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")" + if [[ $umap -ne 0 ]]; then + echo "Test disable WRITE SAME with unmap failed." + fi + cleanup_test_device + + # enable WRITE SAME with unmap + setup_test_device lbprz=1 lbpws=1 + umap="$(cat "/sys/block/$(_short_dev /dev/mapper/test)/queue/write_zeroes_unmap")" + if [[ $umap -ne 1 ]]; then + echo "Test enable WRITE SAME with unmap failed." + fi + cleanup_test_device + + echo "Test complete" +} diff --git a/tests/dm/003.out b/tests/dm/003.out new file mode 100644 index 0000000..51a5405 --- /dev/null +++ b/tests/dm/003.out @@ -0,0 +1,2 @@ +Running dm/003 +Test complete