On Fri, Jan 18, 2019 at 06:44:41PM +0900, Shin'ichiro Kawasaki wrote:
> To allow running tests using a null_blk device with the zoned mode
> disabled (current setup) as well as enabled, introduce the config
> the RUN_ZONED_TESTS config variable and the per-test flag CAN_BE_ZONED.
>
> RUN_ZONED_TESTS=1 indicates that tests run against null_blk will be
> executed twice, first with null_blk as a regular block device
> (RUN_FOR_ZONED=0) and a second time with null_blk set as a zoned block
> device (RUN_FOR_ZONED=1). This applies only to tests cases that have the
> variable CAN_BE_ZONED set to 1, indicating that the test case applies to
> zoned block devices. If CAN_BE_ZONED is not defined by a test case, the
> test is executed only with the regular null_blk device.
>
> _init_null_blk is modified to prepare null_blk as a zoned blocked device
> if RUN_FOR_ZONED is set and as a regular block device otherwise. To avoid
> "modprobe -r null_blk" failures, rmdir calls on all sysfs nullbX
> directories is added.
>
> When a zoned block device is specified in TEST_DEVS, failures of test
> cases that do not set CAN_BE_ZONED are avoided by automatically skipping
> the test. The new helper function _test_dev_is_zoned() is introduced to
> implement this.
>
> The use of the RUN_ZONED_TESTS variable requires that the kernel be
> compiled with CONFIG_BLK_DEV_ZONED enabled.
This is much better, thanks! Some comments below.
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> ---
> Documentation/running-tests.md | 13 +++++++++++++
> check | 29 ++++++++++++++++++++++++++---
> common/null_blk | 23 ++++++++++++++++++++++-
> common/shellcheck | 2 +-
> new | 4 ++++
> 5 files changed, 66 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/running-tests.md b/Documentation/running-tests.md
> index 8f32af3..459c96b 100644
> --- a/Documentation/running-tests.md
> +++ b/Documentation/running-tests.md
> @@ -82,6 +82,19 @@ passing the `-d` command line option or setting the `DEVICE_ONLY` variable.
> DEVICE_ONLY=1
> ```
>
> +### Zoned Block Device
> +
> +To run test cases for zoned block devices, set `RUN_ZONED_TESTS` variable.
> +When this variable is set and a test case can prepare a virtual devices such
> +as null_blk with zoned mode, the test case is executed twice: first with
> +non-zoned mode and second with zoned mode. The use of the RUN_ZONED_TESTS
> +variable requires that the kernel be compiled with CONFIG_BLK_DEV_ZONED
> +enabled.
> +
Minor proofreading:
To run test cases for zoned block devices, set the `RUN_ZONED_TESTS` variable.
When this variable is set and a test case can prepare a virtual device such as
`null_blk` with zoned mode, the test case is executed twice: first in non-zoned
mode and second in zoned mode. The use of the `RUN_ZONED_TESTS` variable
requires that the kernel be compiled with `CONFIG_BLK_DEV_ZONED` enabled.
> +```sh
> +RUN_ZONED_TESTS=1
> +```
> +
> ### Custom Setup
>
> The `config` file is really just a bash file that is sourced at the beginning
> diff --git a/check b/check
> index 6c6d9f5..4563d62 100755
> --- a/check
> +++ b/check
> @@ -17,7 +17,7 @@ _found_test() {
> local test_name="$1"
> local explicit="$2"
>
> - unset DESCRIPTION QUICK TIMED requires device_requires test test_device
> + unset DESCRIPTION QUICK TIMED CAN_BE_ZONED requires device_requires test test_device
>
> # shellcheck disable=SC1090
> if ! . "tests/${test_name}"; then
> @@ -182,11 +182,14 @@ _write_test_run() {
> _output_status() {
> local test="$1"
> local status="$2"
> + local zoned=" "
> +
> + if (( RUN_FOR_ZONED )); then zoned=" (zoned) "; fi
>
> if [[ -v DESCRIPTION ]]; then
> - printf '%-60s' "$test ($DESCRIPTION)"
> + printf '%-60s' "${test}${zoned}($DESCRIPTION)"
> else
> - printf '%-60s' "$test"
> + printf '%-60s' "${test}${zoned}"
> fi
> if [[ -z $status ]]; then
> echo
> @@ -391,10 +394,19 @@ _call_test() {
> fi
> }
>
> +_test_dev_is_zoned() {
> + if grep -qe "none" "${TEST_DEV_SYSFS}/queue/zoned" ; then
> + SKIP_REASON="${TEST_DEV} is not a zoned block device"
> + return 1
> + fi
> + return 0
> +}
> +
> _run_test() {
> TEST_NAME="$1"
> CHECK_DMESG=1
> DMESG_FILTER="cat"
> + RUN_FOR_ZONED=0
>
> # shellcheck disable=SC1090
> . "tests/${TEST_NAME}"
> @@ -407,6 +419,11 @@ _run_test() {
>
> RESULTS_DIR="$OUTPUT/nodev"
> _call_test test
> + if (( RUN_ZONED_TESTS && CAN_BE_ZONED )); then
> + RESULTS_DIR="$OUTPUT/nodev_zoned"
> + RUN_FOR_ZONED=1
> + _call_test test
> + fi
> else
> if [[ ${#TEST_DEVS[@]} -eq 0 ]]; then
> return 0
> @@ -420,6 +437,11 @@ _run_test() {
> local ret=0
> for TEST_DEV in "${TEST_DEVS[@]}"; do
> TEST_DEV_SYSFS="${TEST_DEV_SYSFS_DIRS["$TEST_DEV"]}"
> + if (( !CAN_BE_ZONED )) && _test_dev_is_zoned; then
> + SKIP_REASON="${TEST_DEV} is a zoned block device"
> + _output_notrun "$TEST_NAME => $(basename "$TEST_DEV")"
> + continue
> + fi
We should unset SKIP_REASON here from _test_dev_is_zoned just in case
device_requires forgets to set a SKIP_REASON.
> if declare -fF device_requires >/dev/null && ! device_requires; then
> _output_notrun "$TEST_NAME => $(basename "$TEST_DEV")"
> continue
> @@ -591,6 +613,7 @@ fi
> # Default configuration.
> : "${DEVICE_ONLY:=0}"
> : "${QUICK_RUN:=0}"
> +: "${RUN_ZONED_TESTS:=0}"
> : "${OUTPUT:=results}"
> if [[ -v EXCLUDE ]] && ! declare -p EXCLUDE | grep -q '^declare -a'; then
> # If EXCLUDE was not defined as an array, convert it to one.
> diff --git a/common/null_blk b/common/null_blk
> index 937ece0..fd035b7 100644
> --- a/common/null_blk
> +++ b/common/null_blk
> @@ -8,8 +8,29 @@ _have_null_blk() {
> _have_modules null_blk
> }
>
> +_null_blk_not_zoned() {
> + if [[ "${ZONED}" != "0" ]]; then
> + # shellcheck disable=SC2034
> + SKIP_REASON="null_blk zoned mode not supported"
> + return 1
> + fi
> + return 0
> +}
Is this still used anywhere in this version?
> _init_null_blk() {
> - if ! modprobe -r null_blk || ! modprobe null_blk "$@"; then
> + for d in /sys/kernel/config/nullb/*;
> + do [[ -d "$d" ]] && rmdir "$d"; done
I'd prefer to do this without globbing:
if [[ -d /sys/kernel/config/nullb ]]; then
find /sys/kernel/config/nullb -mindepth 1 -maxdepth 1 -type d -delete
fi
> + local _zoned=""
> + if [[ ${RUN_FOR_ZONED} -ne 0 ]] ; then
> + if ! _have_kernel_option BLK_DEV_ZONED ; then
> + echo -n "ZONED specified for kernel with "
> + echo "CONFIG_BLK_DEV_ZONED disabled"
> + return 1
> + fi
Let's avoid _have_kernel_option if we can, since that requires that the
user enabled CONFIG_IKCONFIG_PROC or installed their config in /boot. We
can just skip this check and the modprobe below will fail with
"null_blk: CONFIG_BLK_DEV_ZONED not enabled" in dmesg.
While you're here, this variable name doesn't need the leading
underscore.
> + _zoned="zoned=1"
> + fi
> + if ! modprobe -r null_blk || ! modprobe null_blk "$@" "${_zoned}" ; then
> return 1
> fi
>
> diff --git a/common/shellcheck b/common/shellcheck
> index 5f46c00..b9be7d2 100644
> --- a/common/shellcheck
> +++ b/common/shellcheck
> @@ -6,5 +6,5 @@
>
> # Suppress unused global variable warnings.
> _silence_sc2034() {
> - echo "$CGROUP2_DIR $CHECK_DMESG $DESCRIPTION $DMESG_FILTER $FIO_PERF_FIELDS $FIO_PERF_PREFIX $QUICK $SKIP_REASON ${TEST_RUN[*]} $TIMED" > /dev/null
> + echo "$CGROUP2_DIR $CHECK_DMESG $DESCRIPTION $DMESG_FILTER $FIO_PERF_FIELDS $FIO_PERF_PREFIX $QUICK $SKIP_REASON ${TEST_RUN[*]} $TIMED $CAN_BE_ZONED" > /dev/null
> }
> diff --git a/new b/new
> index 63e36cd..d7d5f7c 100755
> --- a/new
> +++ b/new
> @@ -145,6 +145,10 @@ DESCRIPTION=""
> # Alternatively, you can filter out any unimportant messages in dmesg like so:
> # DMESG_FILTER="grep -v sysfs"
>
> +# TODO: if this test can be run for both regular block devices and zoned block
> +# devices, uncomment the line below.
> +# CAN_BE_ZONED=1
> +
> # TODO: if this test has any extra requirements, it should define a requires()
> # function. If the test can be run, requires() should return 0. Otherwise, it
> # should return non-zero and set the \$SKIP_REASON variable. Usually,
> --
> 2.20.1
>
