[PATCH blktests v3 00/13] Implement zoned block device support

[PATCH blktests v3 00/13] Implement zoned block device support

[Shin'ichiro Kawasaki]

The current blktests infrastucture and test cases do not support zoned block
devices and no specific test cases exist to test these block devices special
features (zone report and reset, sequential write constraint). This patch series
implement this missing support.

The series addresses two aspects: the first 4 patches introduce changes to the
common scripts and configuration to allow existing test cases to run against a
 null_blk device with zone mode enabled (new RUN_ZONED_TESTS config variable)
as well as whitelisting tests that can support zoned block devices
(new CAN_BE_ZONED test flag). Helper functions are introduced to facilitate
checking a device zone model.

The second part, composed of the last 9 patches, introduce the new zbd test
group to cover zoned block device specific test cases. All these test cases are
implemented using the test_device() function so that target devices can be
specified in the TEST_DEVS config variable, to cover a variety of zoned block
devices: physical real drives, partitions and dm-linear setups on top of zoned
block devices, etc. Furthermore, using the infrastructure changes of the first
part, the TEST_DEVS definition can be left empty, resulting in the zbd test
cases to be run against an automatically created null_blk device with zoned
mode enabled.

5 test cases are added to the new zbd test group to check the kernel ioctl and
sysfs interface, zone report operation, zone reset and write command handling.
These tests are simple but only a start. We will in the future send more test
cases to cover at least the regressions and bugs found and fixed in the zoned
block device code since its introduction with kernel 4.10.

Another still to be added part is support for host-managed ZBC emulation in
scsi-debug to further improve test coverage without requiring a physical SMR
disk. This work is ongoing and will be added to blktests once the relevant
scsi-debug changes are accepted in the kernel.

Changes from v2:
* Replaced ZONED with RUN_FOR_ZONED and CAN_BE_ZONED variables
* Introduce fallback_device mechanism instead of relying on group rc script
* Dropped set_scheduler() move to common

Changes from v1:
* Fixed _test_dev_is_zoned
* Added _have_fio_zbd_zonemode
* Added patch 10 to move _dd to common/rc
* Addressed various nit commented on the list

Masato Suzuki (6):
  tests: Introduce zbd test group
  zbd/001: sysfs and ioctl consistency test
  zbd/002: report zone test
  zbd/003: Test sequential zones reset
  zbd/004: Check write split accross sequential zones
  zbd/005: Test write ordering

Shin'ichiro Kawasaki (7):
  config: Introduce RUN_ZONED_TESTS variable and CAN_BE_ZONED flag
  common: Introduce _have_fio_zbd_zonemode() helper function
  block/004: Adjust fio conditions for zoned block devices
  block: Whitelist tests supporting zoned block devices
  check: Introduce fallback_device() and cleanup_fallback_device()
  common: Introduce _dd() helper function
  src: Introduce zbdioctl program

 Documentation/running-tests.md |  13 +++
 check                          |  54 ++++++++-
 common/fio                     |   9 ++
 common/null_blk                |  23 +++-
 common/rc                      |  31 ++++++
 common/shellcheck              |   2 +-
 new                            |   4 +
 src/.gitignore                 |   1 +
 src/Makefile                   |   3 +-
 src/zbdioctl.c                 |  83 ++++++++++++++
 tests/block/004                |  18 ++-
 tests/block/005                |   1 +
 tests/block/006                |   1 +
 tests/block/010                |   1 +
 tests/block/011                |   1 +
 tests/block/016                |   1 +
 tests/block/017                |   1 +
 tests/block/020                |   1 +
 tests/block/021                |   1 +
 tests/block/023                |   1 +
 tests/zbd/001                  |  75 +++++++++++++
 tests/zbd/001.out              |   2 +
 tests/zbd/002                  | 108 ++++++++++++++++++
 tests/zbd/002.out              |   2 +
 tests/zbd/003                  |  78 +++++++++++++
 tests/zbd/003.out              |   2 +
 tests/zbd/004                  |  90 +++++++++++++++
 tests/zbd/004.out              |   2 +
 tests/zbd/005                  |  67 +++++++++++
 tests/zbd/005.out              |   2 +
 tests/zbd/rc                   | 195 +++++++++++++++++++++++++++++++++
 31 files changed, 866 insertions(+), 7 deletions(-)
 create mode 100644 src/zbdioctl.c
 create mode 100755 tests/zbd/001
 create mode 100644 tests/zbd/001.out
 create mode 100755 tests/zbd/002
 create mode 100644 tests/zbd/002.out
 create mode 100755 tests/zbd/003
 create mode 100644 tests/zbd/003.out
 create mode 100755 tests/zbd/004
 create mode 100644 tests/zbd/004.out
 create mode 100755 tests/zbd/005
 create mode 100644 tests/zbd/005.out
 create mode 100644 tests/zbd/rc

-- 
2.20.1

[PATCH blktests v3 01/13] config: Introduce RUN_ZONED_TESTS variable and CAN_BE_ZONED flag

[Shin'ichiro Kawasaki]

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.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 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.
+
+```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
 			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
+}
+
 _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
+
+	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
+		_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

Re: [PATCH blktests v3 01/13] config: Introduce RUN_ZONED_TESTS variable and CAN_BE_ZONED flag

[Omar Sandoval]

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@wdc.com>
> ---
>  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
> 

Re: [PATCH blktests v3 01/13] config: Introduce RUN_ZONED_TESTS variable and CAN_BE_ZONED flag

[Shinichiro Kawasaki]

On 1/26/19 5:59 AM, Omar Sandoval wrote:
> 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.

Hi Omar, thank you again for your review.

>> +### 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:

Thanks. Will apply.

>> @@ -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.

OK. Will add unset.

>> 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?

I left it by mistake. Will remove it.

>>   _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

Thanks. Will replace with the suggested code.

>> +	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.

OK. Will remove the kernel option check.

> 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

Will rename it.

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH blktests v3 02/13] common: Introduce _have_fio_zbd_zonemode() helper function

[Shin'ichiro Kawasaki]

Fio zbd zone mode is necessary for zoned block devices. Introduce the
helper function _have_fio_zbd_zonemode() to check that the installed
fio version supports the option --zonemode=zbd.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 common/fio | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/fio b/common/fio
index e407088..4c771fc 100644
--- a/common/fio
+++ b/common/fio
@@ -17,6 +17,15 @@ _have_fio() {
 	return 0
 }
 
+_have_fio_zbd_zonemode() {
+	_have_fio || return $?
+	if ! fio --cmdhelp=zonemode 2>&1 | grep -q zbd ; then
+		SKIP_REASON="Fio version too old (does not support --zonemode=zbd)"
+		return 1
+	fi
+	return 0
+}
+
 declare -A FIO_TERSE_FIELDS
 FIO_TERSE_FIELDS=(
 	# Read status
-- 
2.20.1

[PATCH blktests v3 03/13] block/004: Adjust fio conditions for zoned block devices

[Shin'ichiro Kawasaki]

For a random write pattern to a zoned block device, fio requires --direct=1
and --zonemode=zbd options as well as deadline I/O scheduler to be
specified. Specify these options and set the I/O scheduler if the target
device is a zoned block device. Before doing that, also make sure that the
deadline scheduler is available and that fio supports the zbd zone mode.
Set CAN_BE_ZONED flag to run this test case for zoned block devices.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/block/004 | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/tests/block/004 b/tests/block/004
index 4c14c4b..7340d33 100755
--- a/tests/block/004
+++ b/tests/block/004
@@ -8,6 +8,7 @@
 
 DESCRIPTION="run lots of flushes"
 TIMED=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_fio
@@ -16,10 +17,25 @@ requires() {
 test_device() {
 	echo "Running ${TEST_NAME}"
 
+	local directio=""
+	local zbdmode=""
+
+	if _test_dev_is_zoned; then
+		if ! _have_fio_zbd_zonemode; then
+			echo "${SKIP_REASON}"
+			return 1
+		fi
+
+		_test_dev_queue_set scheduler deadline
+
+		directio="--direct=1"
+		zbdmode="--zonemode=zbd"
+	fi
+
 	FIO_PERF_FIELDS=("write iops")
 	_fio_perf --bs=4k --rw=randwrite --norandommap --fsync=1 \
 		--number_ios=256 --numjobs=64 --name=flushes \
-		--filename="$TEST_DEV"
+		${directio} ${zbdmode} --filename="$TEST_DEV"
 
 	echo "Test complete"
 }
-- 
2.20.1

Re: [PATCH blktests v3 03/13] block/004: Adjust fio conditions for zoned block devices

[Omar Sandoval]

On Fri, Jan 18, 2019 at 06:44:43PM +0900, Shin'ichiro Kawasaki wrote:
> For a random write pattern to a zoned block device, fio requires --direct=1
> and --zonemode=zbd options as well as deadline I/O scheduler to be
> specified. Specify these options and set the I/O scheduler if the target
> device is a zoned block device. Before doing that, also make sure that the
> deadline scheduler is available and that fio supports the zbd zone mode.
> Set CAN_BE_ZONED flag to run this test case for zoned block devices.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  tests/block/004 | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/block/004 b/tests/block/004
> index 4c14c4b..7340d33 100755
> --- a/tests/block/004
> +++ b/tests/block/004
> @@ -8,6 +8,7 @@
>  
>  DESCRIPTION="run lots of flushes"
>  TIMED=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_fio
> @@ -16,10 +17,25 @@ requires() {
>  test_device() {
>  	echo "Running ${TEST_NAME}"
>  
> +	local directio=""
> +	local zbdmode=""
> +
> +	if _test_dev_is_zoned; then
> +		if ! _have_fio_zbd_zonemode; then
> +			echo "${SKIP_REASON}"
> +			return 1
> +		fi

This will be marked as a failure instead of skipped. This check can be
in device_requires instead:

device_requires() {
	! _test_dev_is_zoned || _have_fio_zbd_zonemode
}

> +		_test_dev_queue_set scheduler deadline
> +
> +		directio="--direct=1"
> +		zbdmode="--zonemode=zbd"
> +	fi
> +
>  	FIO_PERF_FIELDS=("write iops")
>  	_fio_perf --bs=4k --rw=randwrite --norandommap --fsync=1 \
>  		--number_ios=256 --numjobs=64 --name=flushes \
> -		--filename="$TEST_DEV"
> +		${directio} ${zbdmode} --filename="$TEST_DEV"

I'm surprised that shellcheck is smart enough to see that directio and
zbdmode are always one word so this doesn't need quotes :)

>  
>  	echo "Test complete"
>  }
> -- 
> 2.20.1
> 

Re: [PATCH blktests v3 03/13] block/004: Adjust fio conditions for zoned block devices

[Shinichiro Kawasaki]

On 1/26/19 6:09 AM, Omar Sandoval wrote:
> On Fri, Jan 18, 2019 at 06:44:43PM +0900, Shin'ichiro Kawasaki wrote:
>> @@ -16,10 +17,25 @@ requires() {
>>   test_device() {
>>   	echo "Running ${TEST_NAME}"
>>   
>> +	local directio=""
>> +	local zbdmode=""
>> +
>> +	if _test_dev_is_zoned; then
>> +		if ! _have_fio_zbd_zonemode; then
>> +			echo "${SKIP_REASON}"
>> +			return 1
>> +		fi
> 
> This will be marked as a failure instead of skipped. This check can be
> in device_requires instead:
> 
> device_requires() {
> 	! _test_dev_is_zoned || _have_fio_zbd_zonemode
> }

Thanks. Will change as suggested.

>> +		_test_dev_queue_set scheduler deadline
>> +
>> +		directio="--direct=1"
>> +		zbdmode="--zonemode=zbd"
>> +	fi
>> +
>>   	FIO_PERF_FIELDS=("write iops")
>>   	_fio_perf --bs=4k --rw=randwrite --norandommap --fsync=1 \
>>   		--number_ios=256 --numjobs=64 --name=flushes \
>> -		--filename="$TEST_DEV"
>> +		${directio} ${zbdmode} --filename="$TEST_DEV"
> 
> I'm surprised that shellcheck is smart enough to see that directio and
> zbdmode are always one word so this doesn't need quotes :)

Yes, and if I quote ${directio} and ${zbdmode}, fio fails with error message 
"unable to open '' job file". I leave them without quotes.

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH blktests v3 04/13] block: Whitelist tests supporting zoned block devices

[Shin'ichiro Kawasaki]

Define CAN_BE_ZONED=1 in block/005, block/006, block/010, block/011,
block/016, block/017, block/020, block/021 and block/023 as all these
tests should execute without any problem against null_blk with zoned
mode enabled or zoned block devices specified in TEST_DEVS.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/block/005 | 1 +
 tests/block/006 | 1 +
 tests/block/010 | 1 +
 tests/block/011 | 1 +
 tests/block/016 | 1 +
 tests/block/017 | 1 +
 tests/block/020 | 1 +
 tests/block/021 | 1 +
 tests/block/023 | 1 +
 9 files changed, 9 insertions(+)

diff --git a/tests/block/005 b/tests/block/005
index 65eba22..8ab6791 100755
--- a/tests/block/005
+++ b/tests/block/005
@@ -8,6 +8,7 @@
 
 DESCRIPTION="switch schedulers while doing IO"
 TIMED=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_fio
diff --git a/tests/block/006 b/tests/block/006
index 630d478..0b8a3c0 100755
--- a/tests/block/006
+++ b/tests/block/006
@@ -12,6 +12,7 @@
 
 DESCRIPTION="run null-blk in blocking mode"
 TIMED=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_null_blk && _have_module_param null_blk blocking && _have_fio
diff --git a/tests/block/010 b/tests/block/010
index 76b301f..b81208e 100644
--- a/tests/block/010
+++ b/tests/block/010
@@ -12,6 +12,7 @@
 
 DESCRIPTION="run I/O on null_blk with shared and non-shared tags"
 TIMED=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_null_blk && _have_module_param null_blk shared_tags && _have_fio
diff --git a/tests/block/011 b/tests/block/011
index 8e10900..c3432a6 100755
--- a/tests/block/011
+++ b/tests/block/011
@@ -8,6 +8,7 @@
 
 DESCRIPTION="disable PCI device while doing I/O"
 TIMED=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_fio && _have_program setpci
diff --git a/tests/block/016 b/tests/block/016
index 10ec4ba..c70b7d0 100755
--- a/tests/block/016
+++ b/tests/block/016
@@ -11,6 +11,7 @@
 
 DESCRIPTION="send a signal to a process waiting on a frozen queue"
 QUICK=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_null_blk
diff --git a/tests/block/017 b/tests/block/017
index cea29be..e4a9259 100755
--- a/tests/block/017
+++ b/tests/block/017
@@ -11,6 +11,7 @@
 
 DESCRIPTION="do I/O and check the inflight counter"
 QUICK=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_null_blk
diff --git a/tests/block/020 b/tests/block/020
index a377ea2..39dde66 100755
--- a/tests/block/020
+++ b/tests/block/020
@@ -11,6 +11,7 @@
 
 DESCRIPTION="run null-blk on different schedulers with only one hardware tag"
 QUICK=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_null_blk && _have_fio
diff --git a/tests/block/021 b/tests/block/021
index 0ca5a17..a1bbf45 100755
--- a/tests/block/021
+++ b/tests/block/021
@@ -11,6 +11,7 @@
 
 DESCRIPTION="read/write nr_requests on null-blk with different schedulers"
 QUICK=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_null_blk
diff --git a/tests/block/023 b/tests/block/023
index b0739f7..0f20f4a 100755
--- a/tests/block/023
+++ b/tests/block/023
@@ -10,6 +10,7 @@
 
 DESCRIPTION="do I/O on all null_blk queue modes"
 QUICK=1
+CAN_BE_ZONED=1
 
 requires() {
 	_have_null_blk
-- 
2.20.1

Re: [PATCH blktests v3 04/13] block: Whitelist tests supporting zoned block devices

[Omar Sandoval]

On Fri, Jan 18, 2019 at 06:44:44PM +0900, Shin'ichiro Kawasaki wrote:
> Define CAN_BE_ZONED=1 in block/005, block/006, block/010, block/011,
> block/016, block/017, block/020, block/021 and block/023 as all these
> tests should execute without any problem against null_blk with zoned
> mode enabled or zoned block devices specified in TEST_DEVS.

Several of these tests do random I/O, did I miss a change that makes fio
use zonemode=zbd or something?

> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  tests/block/005 | 1 +
>  tests/block/006 | 1 +
>  tests/block/010 | 1 +
>  tests/block/011 | 1 +
>  tests/block/016 | 1 +
>  tests/block/017 | 1 +
>  tests/block/020 | 1 +
>  tests/block/021 | 1 +
>  tests/block/023 | 1 +
>  9 files changed, 9 insertions(+)
> 
> diff --git a/tests/block/005 b/tests/block/005
> index 65eba22..8ab6791 100755
> --- a/tests/block/005
> +++ b/tests/block/005
> @@ -8,6 +8,7 @@
>  
>  DESCRIPTION="switch schedulers while doing IO"
>  TIMED=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_fio
> diff --git a/tests/block/006 b/tests/block/006
> index 630d478..0b8a3c0 100755
> --- a/tests/block/006
> +++ b/tests/block/006
> @@ -12,6 +12,7 @@
>  
>  DESCRIPTION="run null-blk in blocking mode"
>  TIMED=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_null_blk && _have_module_param null_blk blocking && _have_fio
> diff --git a/tests/block/010 b/tests/block/010
> index 76b301f..b81208e 100644
> --- a/tests/block/010
> +++ b/tests/block/010
> @@ -12,6 +12,7 @@
>  
>  DESCRIPTION="run I/O on null_blk with shared and non-shared tags"
>  TIMED=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_null_blk && _have_module_param null_blk shared_tags && _have_fio
> diff --git a/tests/block/011 b/tests/block/011
> index 8e10900..c3432a6 100755
> --- a/tests/block/011
> +++ b/tests/block/011
> @@ -8,6 +8,7 @@
>  
>  DESCRIPTION="disable PCI device while doing I/O"
>  TIMED=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_fio && _have_program setpci
> diff --git a/tests/block/016 b/tests/block/016
> index 10ec4ba..c70b7d0 100755
> --- a/tests/block/016
> +++ b/tests/block/016
> @@ -11,6 +11,7 @@
>  
>  DESCRIPTION="send a signal to a process waiting on a frozen queue"
>  QUICK=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_null_blk
> diff --git a/tests/block/017 b/tests/block/017
> index cea29be..e4a9259 100755
> --- a/tests/block/017
> +++ b/tests/block/017
> @@ -11,6 +11,7 @@
>  
>  DESCRIPTION="do I/O and check the inflight counter"
>  QUICK=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_null_blk
> diff --git a/tests/block/020 b/tests/block/020
> index a377ea2..39dde66 100755
> --- a/tests/block/020
> +++ b/tests/block/020
> @@ -11,6 +11,7 @@
>  
>  DESCRIPTION="run null-blk on different schedulers with only one hardware tag"
>  QUICK=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_null_blk && _have_fio
> diff --git a/tests/block/021 b/tests/block/021
> index 0ca5a17..a1bbf45 100755
> --- a/tests/block/021
> +++ b/tests/block/021
> @@ -11,6 +11,7 @@
>  
>  DESCRIPTION="read/write nr_requests on null-blk with different schedulers"
>  QUICK=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_null_blk
> diff --git a/tests/block/023 b/tests/block/023
> index b0739f7..0f20f4a 100755
> --- a/tests/block/023
> +++ b/tests/block/023
> @@ -10,6 +10,7 @@
>  
>  DESCRIPTION="do I/O on all null_blk queue modes"
>  QUICK=1
> +CAN_BE_ZONED=1
>  
>  requires() {
>  	_have_null_blk
> -- 
> 2.20.1
> 

Re: [PATCH blktests v3 04/13] block: Whitelist tests supporting zoned block devices

[Shinichiro Kawasaki]

On 1/26/19 6:11 AM, Omar Sandoval wrote:
> On Fri, Jan 18, 2019 at 06:44:44PM +0900, Shin'ichiro Kawasaki wrote:
>> Define CAN_BE_ZONED=1 in block/005, block/006, block/010, block/011,
>> block/016, block/017, block/020, block/021 and block/023 as all these
>> tests should execute without any problem against null_blk with zoned
>> mode enabled or zoned block devices specified in TEST_DEVS.
> 
> Several of these tests do random I/O, did I miss a change that makes fio
> use zonemode=zbd or something?

Five of the test cases white listed call fio with random "read" I/O. Random 
"write" I/O requires zonemode=zbd, but "read" I/O doesn't. block/006, block/010 
and block/020 specify --rw=randread option explicitly. block/005 and block/011 
call _run_fio_rand_io() and the function specifies --rw=randread. For these test 
cases, current fio options can be applied to the zoned block devices without 
zonmode=zbd.

>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>> ---
>>   tests/block/005 | 1 +
>>   tests/block/006 | 1 +
>>   tests/block/010 | 1 +
>>   tests/block/011 | 1 +
>>   tests/block/016 | 1 +
>>   tests/block/017 | 1 +
>>   tests/block/020 | 1 +
>>   tests/block/021 | 1 +
>>   tests/block/023 | 1 +
>>   9 files changed, 9 insertions(+)
>>
>> diff --git a/tests/block/005 b/tests/block/005
>> index 65eba22..8ab6791 100755
>> --- a/tests/block/005
>> +++ b/tests/block/005
>> @@ -8,6 +8,7 @@
>>   
>>   DESCRIPTION="switch schedulers while doing IO"
>>   TIMED=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_fio
>> diff --git a/tests/block/006 b/tests/block/006
>> index 630d478..0b8a3c0 100755
>> --- a/tests/block/006
>> +++ b/tests/block/006
>> @@ -12,6 +12,7 @@
>>   
>>   DESCRIPTION="run null-blk in blocking mode"
>>   TIMED=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_null_blk && _have_module_param null_blk blocking && _have_fio
>> diff --git a/tests/block/010 b/tests/block/010
>> index 76b301f..b81208e 100644
>> --- a/tests/block/010
>> +++ b/tests/block/010
>> @@ -12,6 +12,7 @@
>>   
>>   DESCRIPTION="run I/O on null_blk with shared and non-shared tags"
>>   TIMED=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_null_blk && _have_module_param null_blk shared_tags && _have_fio
>> diff --git a/tests/block/011 b/tests/block/011
>> index 8e10900..c3432a6 100755
>> --- a/tests/block/011
>> +++ b/tests/block/011
>> @@ -8,6 +8,7 @@
>>   
>>   DESCRIPTION="disable PCI device while doing I/O"
>>   TIMED=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_fio && _have_program setpci
>> diff --git a/tests/block/016 b/tests/block/016
>> index 10ec4ba..c70b7d0 100755
>> --- a/tests/block/016
>> +++ b/tests/block/016
>> @@ -11,6 +11,7 @@
>>   
>>   DESCRIPTION="send a signal to a process waiting on a frozen queue"
>>   QUICK=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_null_blk
>> diff --git a/tests/block/017 b/tests/block/017
>> index cea29be..e4a9259 100755
>> --- a/tests/block/017
>> +++ b/tests/block/017
>> @@ -11,6 +11,7 @@
>>   
>>   DESCRIPTION="do I/O and check the inflight counter"
>>   QUICK=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_null_blk
>> diff --git a/tests/block/020 b/tests/block/020
>> index a377ea2..39dde66 100755
>> --- a/tests/block/020
>> +++ b/tests/block/020
>> @@ -11,6 +11,7 @@
>>   
>>   DESCRIPTION="run null-blk on different schedulers with only one hardware tag"
>>   QUICK=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_null_blk && _have_fio
>> diff --git a/tests/block/021 b/tests/block/021
>> index 0ca5a17..a1bbf45 100755
>> --- a/tests/block/021
>> +++ b/tests/block/021
>> @@ -11,6 +11,7 @@
>>   
>>   DESCRIPTION="read/write nr_requests on null-blk with different schedulers"
>>   QUICK=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_null_blk
>> diff --git a/tests/block/023 b/tests/block/023
>> index b0739f7..0f20f4a 100755
>> --- a/tests/block/023
>> +++ b/tests/block/023
>> @@ -10,6 +10,7 @@
>>   
>>   DESCRIPTION="do I/O on all null_blk queue modes"
>>   QUICK=1
>> +CAN_BE_ZONED=1
>>   
>>   requires() {
>>   	_have_null_blk
>> -- 
>> 2.20.1
>>
> 


-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH blktests v3 05/13] check: Introduce fallback_device() and cleanup_fallback_device()

[Shin'ichiro Kawasaki]

These optional functions can be defined by a test case script. When
defined and TEST_DEVS is empty, the fallback_device() is executed before
runing the test case. The fallback_device() function intializes a virtual
device to run the test case and return the device to be set in TEST_DEVS.
After running the test case, cleanup_fallback_device() is executed to
clean up the device.

This feature allows to run test cases with test_device() function even if
TEST_DEVS is not set in the config, using virtaul devices such as null_blk.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 check | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/check b/check
index 4563d62..4b66995 100755
--- a/check
+++ b/check
@@ -407,6 +407,7 @@ _run_test() {
 	CHECK_DMESG=1
 	DMESG_FILTER="cat"
 	RUN_FOR_ZONED=0
+	FALLBACK_DEVICE=0
 
 	# shellcheck disable=SC1090
 	. "tests/${TEST_NAME}"
@@ -425,6 +426,23 @@ _run_test() {
 			_call_test test
 		fi
 	else
+		if [[ ${#TEST_DEVS[@]} -eq 0 ]] && \
+			declare -fF fallback_device >/dev/null && \
+			declare -fF cleanup_fallback_device >/dev/null; then
+			if ! test_dev=$(fallback_device); then
+				_warning "$TEST_NAME: fallback_device call failure"
+				return 0
+			fi
+			if ! sysfs_dir="$(_find_sysfs_dir "$test_dev")"; then
+				_warning "$TEST_NAME: could not find sysfs directory for ${test_dev}"
+				cleanup_fallback_device
+				return 0
+			fi
+			TEST_DEVS=( "${test_dev}" )
+			TEST_DEV_SYSFS_DIRS["$test_dev"]="$sysfs_dir"
+			FALLBACK_DEVICE=1
+		fi
+
 		if [[ ${#TEST_DEVS[@]} -eq 0 ]]; then
 			return 0
 		fi
@@ -451,6 +469,13 @@ _run_test() {
 				ret=1
 			fi
 		done
+
+		if (( FALLBACK_DEVICE )); then
+			cleanup_fallback_device
+			unset TEST_DEV_SYSFS_DIRS["${TEST_DEVS[0]}"]
+			TEST_DEVS=()
+		fi
+
 		return $ret
 	fi
 }
-- 
2.20.1

Re: [PATCH blktests v3 05/13] check: Introduce fallback_device() and cleanup_fallback_device()

[Omar Sandoval]

On Fri, Jan 18, 2019 at 06:44:45PM +0900, Shin'ichiro Kawasaki wrote:
> These optional functions can be defined by a test case script. When
> defined and TEST_DEVS is empty, the fallback_device() is executed before
> runing the test case. The fallback_device() function intializes a virtual
> device to run the test case and return the device to be set in TEST_DEVS.
> After running the test case, cleanup_fallback_device() is executed to
> clean up the device.
> 
> This feature allows to run test cases with test_device() function even if
> TEST_DEVS is not set in the config, using virtaul devices such as null_blk.
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  check | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/check b/check
> index 4563d62..4b66995 100755
> --- a/check
> +++ b/check
> @@ -407,6 +407,7 @@ _run_test() {
>  	CHECK_DMESG=1
>  	DMESG_FILTER="cat"
>  	RUN_FOR_ZONED=0
> +	FALLBACK_DEVICE=0
>  
>  	# shellcheck disable=SC1090
>  	. "tests/${TEST_NAME}"
> @@ -425,6 +426,23 @@ _run_test() {
>  			_call_test test
>  		fi
>  	else
> +		if [[ ${#TEST_DEVS[@]} -eq 0 ]] && \
> +			declare -fF fallback_device >/dev/null && \
> +			declare -fF cleanup_fallback_device >/dev/null; then

We should check in _found_test that cleanup_fallback_device is defined
if fallback_device is defined, and vice versa. Then, we can just check
for fallback_device here.

> +			if ! test_dev=$(fallback_device); then
> +				_warning "$TEST_NAME: fallback_device call failure"
> +				return 0
> +			fi
> +			if ! sysfs_dir="$(_find_sysfs_dir "$test_dev")"; then
> +				_warning "$TEST_NAME: could not find sysfs directory for ${test_dev}"
> +				cleanup_fallback_device
> +				return 0
> +			fi
> +			TEST_DEVS=( "${test_dev}" )
> +			TEST_DEV_SYSFS_DIRS["$test_dev"]="$sysfs_dir"
> +			FALLBACK_DEVICE=1
> +		fi
> +
>  		if [[ ${#TEST_DEVS[@]} -eq 0 ]]; then
>  			return 0
>  		fi
> @@ -451,6 +469,13 @@ _run_test() {
>  				ret=1
>  			fi
>  		done
> +
> +		if (( FALLBACK_DEVICE )); then
> +			cleanup_fallback_device
> +			unset TEST_DEV_SYSFS_DIRS["${TEST_DEVS[0]}"]
> +			TEST_DEVS=()
> +		fi
> +
>  		return $ret
>  	fi
>  }
> -- 
> 2.20.1
> 

Re: [PATCH blktests v3 05/13] check: Introduce fallback_device() and cleanup_fallback_device()

[Shinichiro Kawasaki]

On 1/26/19 6:14 AM, Omar Sandoval wrote:
> On Fri, Jan 18, 2019 at 06:44:45PM +0900, Shin'ichiro Kawasaki wrote:
>> These optional functions can be defined by a test case script. When
>> defined and TEST_DEVS is empty, the fallback_device() is executed before
>> runing the test case. The fallback_device() function intializes a virtual
>> device to run the test case and return the device to be set in TEST_DEVS.
>> After running the test case, cleanup_fallback_device() is executed to
>> clean up the device.
>>
>> This feature allows to run test cases with test_device() function even if
>> TEST_DEVS is not set in the config, using virtaul devices such as null_blk.
>>
>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>> ---
>>   check | 25 +++++++++++++++++++++++++
>>   1 file changed, 25 insertions(+)
>>
>> diff --git a/check b/check
>> index 4563d62..4b66995 100755
>> --- a/check
>> +++ b/check
>> @@ -407,6 +407,7 @@ _run_test() {
>>   	CHECK_DMESG=1
>>   	DMESG_FILTER="cat"
>>   	RUN_FOR_ZONED=0
>> +	FALLBACK_DEVICE=0
>>   
>>   	# shellcheck disable=SC1090
>>   	. "tests/${TEST_NAME}"
>> @@ -425,6 +426,23 @@ _run_test() {
>>   			_call_test test
>>   		fi
>>   	else
>> +		if [[ ${#TEST_DEVS[@]} -eq 0 ]] && \
>> +			declare -fF fallback_device >/dev/null && \
>> +			declare -fF cleanup_fallback_device >/dev/null; then
> 
> We should check in _found_test that cleanup_fallback_device is defined
> if fallback_device is defined, and vice versa. Then, we can just check
> for fallback_device here.

Thanks. Will change the patch as suggested. I noticed that fallback_device and 
cleanup_fallback_device should be unset at the beginning of _found_test. Will 
add it also.

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH blktests v3 06/13] common: Introduce _dd() helper function

[Shin'ichiro Kawasaki]

To analyze dd command failures found by blktests, need to confirm dd
command options. Introduce the helper function which executes dd and
records dd command options in FULL file for quick analysis.

Reviewed-by: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 common/rc | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/common/rc b/common/rc
index 153a323..fe0e5d8 100644
--- a/common/rc
+++ b/common/rc
@@ -214,3 +214,34 @@ _test_dev_in_hotplug_slot() {
 _filter_xfs_io_error() {
 	sed -e 's/^\(.*\)64\(: .*$\)/\1\2/'
 }
+
+# Issue dd command with five arguments and record command line in FULL file.
+# args: target device, r/w, start sector, sector len, block size in bytes
+_dd() {
+	local target_dev=${1}
+	local rw=${2}
+	local -i start_sector=${3}
+	local -i start_byte=$(( start_sector * 512 ))
+	local -i sector_count=${4}
+	local -i bs=${5}
+	local -i block_count=$(( sector_count * 512 / bs ))
+
+	local _cmd="dd bs=${bs} count=${block_count}"
+
+	if [[ ${rw} = "read" ]]; then
+		_cmd="${_cmd} if=${target_dev} of=/dev/null"
+		_cmd="${_cmd} iflag=skip_bytes skip=${start_byte}"
+	elif [[ ${rw} = "write" ]]; then
+		_cmd="${_cmd} if=/dev/zero of=${target_dev}"
+		_cmd="${_cmd} oflag=seek_bytes,direct seek=${start_byte}"
+	fi
+
+	echo "${_cmd}" >> "$FULL" 2>&1
+
+	if ! eval "${_cmd}" >> "$FULL" 2>&1 ; then
+		echo "dd command failed"
+		return 1
+	fi
+
+	sync
+}
-- 
2.20.1

Re: [PATCH blktests v3 06/13] common: Introduce _dd() helper function

[Omar Sandoval]

On Fri, Jan 18, 2019 at 06:44:46PM +0900, Shin'ichiro Kawasaki wrote:
> To analyze dd command failures found by blktests, need to confirm dd
> command options. Introduce the helper function which executes dd and
> records dd command options in FULL file for quick analysis.
> 
> Reviewed-by: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  common/rc | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 153a323..fe0e5d8 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -214,3 +214,34 @@ _test_dev_in_hotplug_slot() {
>  _filter_xfs_io_error() {
>  	sed -e 's/^\(.*\)64\(: .*$\)/\1\2/'
>  }
> +
> +# Issue dd command with five arguments and record command line in FULL file.
> +# args: target device, r/w, start sector, sector len, block size in bytes
> +_dd() {
> +	local target_dev=${1}
> +	local rw=${2}
> +	local -i start_sector=${3}
> +	local -i start_byte=$(( start_sector * 512 ))
> +	local -i sector_count=${4}
> +	local -i bs=${5}
> +	local -i block_count=$(( sector_count * 512 / bs ))
> +
> +	local _cmd="dd bs=${bs} count=${block_count}"
> +
> +	if [[ ${rw} = "read" ]]; then
> +		_cmd="${_cmd} if=${target_dev} of=/dev/null"
> +		_cmd="${_cmd} iflag=skip_bytes skip=${start_byte}"
> +	elif [[ ${rw} = "write" ]]; then
> +		_cmd="${_cmd} if=/dev/zero of=${target_dev}"
> +		_cmd="${_cmd} oflag=seek_bytes,direct seek=${start_byte}"
> +	fi

This doesn't seem to be abstracting away anything too complicated. I'd
rather you remove the layer of indirection and open-code calls to dd.

> +	echo "${_cmd}" >> "$FULL" 2>&1
> +
> +	if ! eval "${_cmd}" >> "$FULL" 2>&1 ; then
> +		echo "dd command failed"
> +		return 1
> +	fi
> +
> +	sync
> +}
> -- 
> 2.20.1
> 

Re: [PATCH blktests v3 06/13] common: Introduce _dd() helper function

[Shinichiro Kawasaki]

On 1/26/19 6:17 AM, Omar Sandoval wrote:
> On Fri, Jan 18, 2019 at 06:44:46PM +0900, Shin'ichiro Kawasaki wrote:
>> To analyze dd command failures found by blktests, need to confirm dd
>> command options. Introduce the helper function which executes dd and
>> records dd command options in FULL file for quick analysis.
>>
>> Reviewed-by: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
>> ---
>>   common/rc | 31 +++++++++++++++++++++++++++++++
>>   1 file changed, 31 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index 153a323..fe0e5d8 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -214,3 +214,34 @@ _test_dev_in_hotplug_slot() {
>>   _filter_xfs_io_error() {
>>   	sed -e 's/^\(.*\)64\(: .*$\)/\1\2/'
>>   }
>> +
>> +# Issue dd command with five arguments and record command line in FULL file.
>> +# args: target device, r/w, start sector, sector len, block size in bytes
>> +_dd() {
>> +	local target_dev=${1}
>> +	local rw=${2}
>> +	local -i start_sector=${3}
>> +	local -i start_byte=$(( start_sector * 512 ))
>> +	local -i sector_count=${4}
>> +	local -i bs=${5}
>> +	local -i block_count=$(( sector_count * 512 / bs ))
>> +
>> +	local _cmd="dd bs=${bs} count=${block_count}"
>> +
>> +	if [[ ${rw} = "read" ]]; then
>> +		_cmd="${_cmd} if=${target_dev} of=/dev/null"
>> +		_cmd="${_cmd} iflag=skip_bytes skip=${start_byte}"
>> +	elif [[ ${rw} = "write" ]]; then
>> +		_cmd="${_cmd} if=/dev/zero of=${target_dev}"
>> +		_cmd="${_cmd} oflag=seek_bytes,direct seek=${start_byte}"
>> +	fi
> 
> This doesn't seem to be abstracting away anything too complicated. I'd
> rather you remove the layer of indirection and open-code calls to dd.

OK. Will replace _dd function call with dd command call.

-- 
Best Regards,
Shin'ichiro Kawasaki

[PATCH blktests v3 07/13] src: Introduce zbdioctl program

[Shin'ichiro Kawasaki]

zbdioctl implements calls to zoned block devices ioctls that are not
supported currently by sys-utils blkzone utility, namely BLKGETZONESZ
and BLKGETNRZONES.

Reviewed-by: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 src/.gitignore |  1 +
 src/Makefile   |  3 +-
 src/zbdioctl.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 src/zbdioctl.c

diff --git a/src/.gitignore b/src/.gitignore
index 8c95785..2108f56 100644
--- a/src/.gitignore
+++ b/src/.gitignore
@@ -6,3 +6,4 @@
 /nbdsetsize
 /sg/dxfer-from-dev
 /sg/syzkaller1
+/zbdioctl
diff --git a/src/Makefile b/src/Makefile
index c4094b4..5a0556f 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -5,7 +5,8 @@ C_TARGETS := \
 	sg/dxfer-from-dev \
 	sg/syzkaller1 \
 	nbdsetsize \
-	loop_change_fd
+	loop_change_fd \
+	zbdioctl
 
 CXX_TARGETS := \
 	discontiguous-io
diff --git a/src/zbdioctl.c b/src/zbdioctl.c
new file mode 100644
index 0000000..1ea72e8
--- /dev/null
+++ b/src/zbdioctl.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-3.0+
+// Copyright (C) 2018 Western Digital Corporation or its affiliates.
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <linux/blkzoned.h>
+#include <linux/types.h>
+
+#if !defined(BLKGETZONESZ) || !defined(BLKGETNRZONES)
+
+int main(int argc, char **argv)
+{
+	return EXIT_FAILURE;
+}
+
+#else
+
+struct request {
+	const char *name;
+	unsigned long code;
+} requests[] = {
+	{ "-s", BLKGETZONESZ},
+	{ "-n", BLKGETNRZONES},
+	{ NULL, 0},
+};
+
+void usage(const char *progname)
+{
+	int i = 0;
+
+	fprintf(stderr, "usage: %s <request> <device file>\n", progname);
+	fprintf(stderr, "<request> can be:\n");
+	while (requests[i].name) {
+		fprintf(stderr, "\t%s\n", requests[i].name);
+		i++;
+	}
+	exit(EXIT_FAILURE);
+}
+
+int main(int argc, char **argv)
+{
+	int i = 0, fd, ret;
+	unsigned int val;
+	unsigned long code = 0;
+
+	if (argc != 3)
+		usage(argv[0]);
+
+	while (requests[i].name) {
+		if (strcmp(argv[1], requests[i].name) == 0) {
+			code = requests[i].code;
+			break;
+		}
+		i++;
+	}
+	if (code == 0)
+		usage(argv[0]);
+
+	fd = open(argv[2], O_RDWR);
+	if (fd < 0) {
+		perror("open");
+		return EXIT_FAILURE;
+	}
+
+	ret = ioctl(fd, code, &val);
+	if (ret < 0) {
+		perror("ioctl");
+		close(fd);
+		return EXIT_FAILURE;
+	}
+
+	printf("%u\n", val);
+
+	close(fd);
+
+	return EXIT_SUCCESS;
+}
+
+#endif
+
-- 
2.20.1

[PATCH blktests v3 08/13] tests: Introduce zbd test group

[Shin'ichiro Kawasaki]

From: Masato Suzuki <masato.suzuki@wdc.com>

The zoned block device (zbd) test group is used to gather all tests
specific to zoned block devices (null_blk device with zoned mode enabled,
SMR disks, dm-linear on top of zoned devices, etc). Execution of this group
requires that the kernel be compiled with the block layer
CONFIG_BLK_DEV_ZONED option enabled and also requires the null_blk driver
to have zoned mode support (added in kernel 4.19).

This group rc script implements _fallback_null_blk_zoned() helper function
which initailize a null_blk device with zoned mode. Each of the zbd group
test cases calls it in fallback_device() function. This allows the zbd
group test cases fallback to the null_blk device even if the TEST_DEVS
is empty. With this, all tests scripts can be written by only defining
the test_device() function while allowing operation on both null_blk and
user specified devices.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Masato Suzuki <masato.suzuki@wdc.com>
---
 tests/zbd/rc | 195 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 195 insertions(+)
 create mode 100644 tests/zbd/rc

diff --git a/tests/zbd/rc b/tests/zbd/rc
new file mode 100644
index 0000000..a6c7696
--- /dev/null
+++ b/tests/zbd/rc
@@ -0,0 +1,195 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Western Digital Corporation or its affiliates.
+#
+# Tests for Zone Block Device.
+
+. common/rc
+. common/null_blk
+
+#
+# Test requirement check functions
+#
+
+group_requires() {
+	_have_root || return $?
+	_have_program blkzone || return $?
+	_have_program dd || return $?
+	_have_kernel_option BLK_DEV_ZONED || return $?
+	_have_modules null_blk && _have_module_param null_blk zoned
+}
+
+group_device_requires() {
+	_test_dev_is_zoned
+}
+
+_fallback_null_blk_zoned() {
+	if ! _init_null_blk zone_size=4 gb=1 zoned=1 ; then
+		return 1
+	fi
+	echo /dev/nullb0
+}
+
+#
+# Zone types and conditions
+#
+export ZONE_TYPE_CONVENTIONAL=1
+export ZONE_TYPE_SEQ_WRITE_REQUIRED=2
+export ZONE_TYPE_SEQ_WRITE_PREFERRED=3
+
+export ZONE_COND_EMPTY=1
+export ZONE_COND_IMPLICIT_OPEN=2
+export ZONE_COND_FULL=14
+
+export ZONE_TYPE_ARRAY=(
+	[1]="CONVENTIONAL"
+	[2]="SEQ_WRITE_REQUIRED"
+	[3]="SEQ_WRITE_PREFERRED"
+)
+
+export ZONE_COND_ARRAY=(
+	[0]="NOT_WP"
+	[1]="EMPTY"
+	[2]="IMPLICIT_OPEN"
+	[3]="EXPLICIT_OPEN"
+	[4]="CLOSE"
+	[13]="READ_ONLY"
+	[14]="FULL"
+	[15]="OFFLINE"
+)
+
+# sysfs variable array indices
+export SV_CAPACITY=0
+export SV_CHUNK_SECTORS=1
+export SV_PHYS_BLK_SIZE=2
+export SV_PHYS_BLK_SECTORS=3
+export SV_NR_ZONES=4
+
+#
+# Helper functions
+#
+
+# Obtain zone related sysfs variables and keep in a global array until put
+# function call.
+_get_sysfs_variable() {
+	unset SYSFS_VARS
+	local _dir=${TEST_DEV_SYSFS}
+	SYSFS_VARS[$SV_CAPACITY]=$(<"${_dir}"/size)
+	SYSFS_VARS[$SV_CHUNK_SECTORS]=$(<"${_dir}"/queue/chunk_sectors)
+	SYSFS_VARS[$SV_PHYS_BLK_SIZE]=$(<"${_dir}"/queue/physical_block_size)
+	SYSFS_VARS[$SV_PHYS_BLK_SECTORS]=$((SYSFS_VARS[SV_PHYS_BLK_SIZE] / 512))
+
+	# If the nr_zones sysfs attribute exists, get its value. Otherwise,
+	# calculate its value based on the total capacity and zone size, taking
+	# into account that the last zone can be smaller than other zones.
+	if [[ -e ${TEST_DEV_SYSFS}/queue/nr_zones ]]; then
+		SYSFS_VARS[$SV_NR_ZONES]=$(<"${_dir}"/queue/nr_zones)
+	else
+		SYSFS_VARS[$SV_NR_ZONES]=$(( (SYSFS_VARS[SV_CAPACITY] - 1) \
+				/ SYSFS_VARS[SV_CHUNK_SECTORS] + 1 ))
+	fi
+}
+
+_put_sysfs_variable() {
+	unset SYSFS_VARS
+}
+
+# Issue zone report command and keep reported information in global arrays
+# until put function call.
+_get_blkzone_report() {
+	local target_dev=${1}
+
+	# Initialize arrays to store parsed blkzone reports.
+	# Number of reported zones is set in REPORTED_COUNT.
+	# The arrays have REPORTED_COUNT+1 elements with additional one at tail
+	# to simplify loop operation.
+	ZONE_STARTS=()
+	ZONE_LENGTHS=()
+	ZONE_WPTRS=()
+	ZONE_CONDS=()
+	ZONE_TYPES=()
+	NR_CONV_ZONES=0
+	REPORTED_COUNT=0
+
+	TMP_REPORT_FILE=${TMPDIR}/blkzone_report
+	if ! blkzone report "${target_dev}" > "${TMP_REPORT_FILE}"; then
+		echo "blkzone command failed"
+		return $?
+	fi
+
+	local _IFS=$IFS
+	local -i loop=0
+	IFS=$' ,:'
+	while read -r -a _tokens
+	do
+		ZONE_STARTS+=($((_tokens[1])))
+		ZONE_LENGTHS+=($((_tokens[3])))
+		ZONE_WPTRS+=($((_tokens[5])))
+		ZONE_CONDS+=($((${_tokens[11]%\(*})))
+		ZONE_TYPES+=($((${_tokens[13]%\(*})))
+		if [[ ${ZONE_TYPES[-1]} -eq ${ZONE_TYPE_CONVENTIONAL} ]]; then
+			(( NR_CONV_ZONES++ ))
+		fi
+		(( loop++ ))
+	done < "${TMP_REPORT_FILE}"
+	IFS="$_IFS"
+	REPORTED_COUNT=${loop}
+
+	if [[ ${REPORTED_COUNT} -eq 0 ]] ; then
+		echo "blkzone report returned no zone"
+		return 1
+	fi
+
+	# Set value to allow additioanl element access at array end
+	local -i max_idx=$((REPORTED_COUNT - 1))
+	ZONE_STARTS+=( $((ZONE_STARTS[max_idx] + ZONE_LENGTHS[max_idx])) )
+	ZONE_LENGTHS+=( "${ZONE_LENGTHS[max_idx]}" )
+	ZONE_WPTRS+=( "${ZONE_WPTRS[max_idx]}" )
+	ZONE_CONDS+=( "${ZONE_CONDS[max_idx]}" )
+	ZONE_TYPES+=( "${ZONE_TYPES[max_idx]}" )
+
+	rm -f "${TMP_REPORT_FILE}"
+}
+
+_put_blkzone_report() {
+	unset ZONE_STARTS
+	unset ZONE_LENGTHS
+	unset ZONE_WPTRS
+	unset ZONE_CONDS
+	unset ZONE_TYPES
+	unset REPORTED_COUNT
+	unset NR_CONV_ZONES
+}
+
+# Issue reset zone command with zone count option.
+# Call _get_blkzone_report() beforehand.
+_reset_zones() {
+	local target_dev=${1}
+	local -i idx=${2}
+	local -i count=${3}
+
+	if ! blkzone reset -o "${ZONE_STARTS[idx]}" -c "${count}" \
+	     "${target_dev}" >> "$FULL" 2>&1 ; then
+		echo "blkzone reset command failed"
+		return 1
+	fi
+}
+
+# Search zones and find two contiguous sequential required zones.
+# Return index of the first zone of the found two zones.
+# Call _get_blkzone_report() beforehand.
+_find_two_contiguous_seq_zones() {
+	local -i type_seq=${ZONE_TYPE_SEQ_WRITE_REQUIRED}
+
+	for ((idx = NR_CONV_ZONES; idx < REPORTED_COUNT; idx++)); do
+		if [[ ${ZONE_TYPES[idx]} -eq ${type_seq} &&
+		      ${ZONE_TYPES[idx+1]} -eq ${type_seq} ]]; then
+			echo "${idx}"
+			return 0
+		fi
+	done
+
+	echo "Contiguous sequential write required zones not found"
+	return 1
+}
+
-- 
2.20.1

[PATCH blktests v3 09/13] zbd/001: sysfs and ioctl consistency test

[Shin'ichiro Kawasaki]

From: Masato Suzuki <masato.suzuki@wdc.com>

Obtain a zoned block device attributes from sysfs and using ioctl calls
and confirm the consistency of the values.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Masato Suzuki <masato.suzuki@wdc.com>
---
 tests/zbd/001     | 75 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/zbd/001.out |  2 ++
 2 files changed, 77 insertions(+)
 create mode 100755 tests/zbd/001
 create mode 100644 tests/zbd/001.out

diff --git a/tests/zbd/001 b/tests/zbd/001
new file mode 100755
index 0000000..ccb6e6c
--- /dev/null
+++ b/tests/zbd/001
@@ -0,0 +1,75 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Western Digital Corporation or its affiliates.
+#
+# Check zoned block device sysfs and ioctl parameter availability and
+# confirm consistency among them.
+
+. tests/zbd/rc
+
+DESCRIPTION="sysfs and ioctl"
+QUICK=1
+CAN_BE_ZONED=1
+
+requires() {
+	_have_src_program zbdioctl
+}
+
+fallback_device() {
+	_fallback_null_blk_zoned
+}
+
+cleanup_fallback_device() {
+	_exit_null_blk
+}
+
+test_device() {
+	echo "Running ${TEST_NAME}"
+
+	# Get and keep sysfs variables
+	_get_sysfs_variable "${TEST_DEV}" || return $?
+	local -i capacity=${SYSFS_VARS[$SV_CAPACITY]}
+	local -i chunk_sectors=${SYSFS_VARS[$SV_CHUNK_SECTORS]}
+
+	# Calculate expected number of zones based on the total capacity and
+	# zone size, taking into account that the last zone can be smaller
+	# than other zones.
+	local -i nr_zones=$(( (capacity - 1) / chunk_sectors + 1 ))
+
+	# Compare sysfs values and ioctl values
+	if [[ -e "${TEST_DEV_SYSFS}"/queue/nr_zones ]]; then
+		local -i sysfs_nr_zones=${SYSFS_VARS[$SV_NR_ZONES]}
+		local -i ioctl_nr_zones
+		local -i ioctl_zonesize
+
+		ioctl_zonesize=$(src/zbdioctl -s "${TEST_DEV}")
+		if [[ ${chunk_sectors} -ne ${ioctl_zonesize} ]]; then
+			echo -n "ioctl zone size:${ioctl_zonesize} != "
+			echo "sysfs chunk_sectors:${chunk_sectors}"
+			return 1
+		fi
+
+		ioctl_nr_zones=$(src/zbdioctl -n "${TEST_DEV}")
+		if [[ ${nr_zones} -ne ${ioctl_nr_zones} ]]; then
+			echo -n "ioctl nr_zones:${ioctl_nr_zones} != "
+			echo "nr_zones:${nr_zones}"
+			return 1
+		fi
+
+		if [[ ${nr_zones} -ne ${sysfs_nr_zones} ]]; then
+			echo -n "sysfs nr_zones:${sysfs_nr_zones} != "
+			echo "nr_zones:${nr_zones}"
+			return 1
+		fi
+	fi
+
+	_put_sysfs_variable
+	{
+		echo "Capacity: ${capacity} sectors"
+		echo "Zone: ${chunk_sectors} sectors"
+		echo "Number of zones: ${nr_zones} zones"
+	} >> "$FULL" 2>&1
+
+	echo "Test complete"
+}
+
diff --git a/tests/zbd/001.out b/tests/zbd/001.out
new file mode 100644
index 0000000..7d72a8f
--- /dev/null
+++ b/tests/zbd/001.out
@@ -0,0 +1,2 @@
+Running zbd/001
+Test complete
-- 
2.20.1

[PATCH blktests v3 10/13] zbd/002: report zone test

[Shin'ichiro Kawasaki]

From: Masato Suzuki <masato.suzuki@wdc.com>

Get a report for all zones and confirm that the reported values are
valid and consistent with regard to the device capacity and zone size.

Signed-off-by: Masato Suzuki <masato.suzuki@wdc.com>
---
 tests/zbd/002     | 108 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/zbd/002.out |   2 +
 2 files changed, 110 insertions(+)
 create mode 100755 tests/zbd/002
 create mode 100644 tests/zbd/002.out

diff --git a/tests/zbd/002 b/tests/zbd/002
new file mode 100755
index 0000000..e197827
--- /dev/null
+++ b/tests/zbd/002
@@ -0,0 +1,108 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Western Digital Corporation or its affiliates.
+#
+# Issue report zone command and confirm consistency of reported values.
+
+. tests/zbd/rc
+
+DESCRIPTION="report zone"
+CAN_BE_ZONED=1
+
+fallback_device() {
+	_fallback_null_blk_zoned
+}
+
+cleanup_fallback_device() {
+	_exit_null_blk
+}
+
+_check_blkzone_report() {
+	# Utilize local variables as much as possible to speed up loop execution
+	local -i max_idx=$((REPORTED_COUNT - 1))
+	local -i chunk_sectors=${SYSFS_VARS[$SV_CHUNK_SECTORS]}
+	local -i cur_start=${ZONE_STARTS[0]}
+	local -i next_start=0
+	local -i len=0
+	local -i wptr=0
+	local -i cond=0
+	local -i zone_type=0
+
+	# Confirm number of reported zones
+	if [[ ${REPORTED_COUNT} -ne ${SYSFS_VARS[$SV_NR_ZONES]} ]]; then
+		echo "The number of zones reported differ from sysfs nr_zones"
+		echo -n "Reported zones count: ${REPORTED_COUNT}  "
+		echo "sysfs nr_zones: ${SYSFS_VARS[$SV_NR_ZONES]}"
+		return 1
+	fi
+
+	# Check consistency between last zone size and capacity
+	local -i last_zone_end=$((ZONE_STARTS[max_idx] + ZONE_LENGTHS[max_idx]))
+	if [[ ${last_zone_end} -gt ${SYSFS_VARS[$SV_CAPACITY]} ]]; then
+		echo "Last zone start sector + length exceeds capacity"
+		echo -n "Capacity: ${SYSFS_VARS[$SV_CAPACITY]}, "
+		echo "last zone start sector + length: ${last_zone_end}"
+		return 1
+	fi
+
+	# Check each zone parameter validity and that all zones are contiguous
+	for ((idx = 0; idx <= max_idx; idx++)); do
+
+		next_start=${ZONE_STARTS[$((idx+1))]}
+		len=${ZONE_LENGTHS[$idx]}
+		wptr=${ZONE_WPTRS[$idx]}
+		cond=${ZONE_CONDS[$idx]}
+		zone_type=${ZONE_TYPES[$idx]}
+
+		# Check two zones are contiguous
+		if [[ $((cur_start+len)) -ne ${next_start} ]]; then
+			echo -n "Zones are not contiguous at zone ${idx}. "
+			echo "cur:${cur_start}+${len}, next:${next_start}"
+			return 1
+		fi
+
+		# Check zone size
+		if [[ ${len} -ne ${chunk_sectors} &&
+		      ${idx} -ne ${max_idx} ]]; then
+			echo -n "Zone size is not same as chunk_sectors "
+			echo -n "at zone ${idx}. "
+			echo "size: ${len}, chunk_sectors: ${chunk_sectors}"
+			return 1
+		fi
+
+		# Check write pointer
+		if [[ ${wptr} -lt 0 || ${wptr} -gt ${len} ]]; then
+			echo -n "Write pointer is invalid at zone ${idx}. "
+			echo "wp:${wptr}"
+			return 1
+		fi
+
+		# Check zone condition
+		if [[ ! ${ZONE_COND_ARRAY[cond]} ]]; then
+			echo -n "Zone condition is incorrect at zone ${idx}. "
+			echo "condition: ${cond}"
+			return 1
+		fi
+
+		# Check zone type
+		if [[ ! ${ZONE_TYPE_ARRAY[zone_type]} ]]; then
+			echo -n "Zone type is incorrect at zone ${idx}. "
+			echo "type: ${zone_type}"
+			return 1
+		fi
+		cur_start=${next_start}
+	done
+	return 0
+}
+
+test_device() {
+	echo "Running ${TEST_NAME}"
+
+	_get_sysfs_variable "${TEST_DEV}" || return $?
+	_get_blkzone_report "${TEST_DEV}" || return $?
+	_check_blkzone_report || return $?
+	_put_blkzone_report
+	_put_sysfs_variable
+
+	echo "Test complete"
+}
diff --git a/tests/zbd/002.out b/tests/zbd/002.out
new file mode 100644
index 0000000..7317541
--- /dev/null
+++ b/tests/zbd/002.out
@@ -0,0 +1,2 @@
+Running zbd/002
+Test complete
-- 
2.20.1

[PATCH blktests v3 11/13] zbd/003: Test sequential zones reset

[Shin'ichiro Kawasaki]

From: Masato Suzuki <masato.suzuki@wdc.com>

Test zone reset operation to make sure that the BLKRESETZONE ioctl call
works as expected but also that the zone sector remapping that may be
done for logical devices (partitions or dm-linear devices) is correct.

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Masato Suzuki <masato.suzuki@wdc.com>
---
 tests/zbd/003     | 78 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/zbd/003.out |  2 ++
 2 files changed, 80 insertions(+)
 create mode 100755 tests/zbd/003
 create mode 100644 tests/zbd/003.out

diff --git a/tests/zbd/003 b/tests/zbd/003
new file mode 100755
index 0000000..b51f713
--- /dev/null
+++ b/tests/zbd/003
@@ -0,0 +1,78 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Western Digital Corporation or its affiliates.
+#
+# Confirm that reset zone command for 2 contiguous sequential write
+# requried zones works as expected.
+
+. tests/zbd/rc
+
+DESCRIPTION="reset sequential required zones"
+QUICK=1
+CAN_BE_ZONED=1
+
+requires() {
+	_have_program blkzone
+}
+
+fallback_device() {
+	_fallback_null_blk_zoned
+}
+
+cleanup_fallback_device() {
+	_exit_null_blk
+}
+
+test_device() {
+	local -i zone_idx
+	local -a target_zones
+	local -i phys_blk_size
+
+	echo "Running ${TEST_NAME}"
+
+	# Get physical block size for dd
+	_get_sysfs_variable "${TEST_DEV}" || return $?
+	phys_blk_size=${SYSFS_VARS[$SV_PHYS_BLK_SIZE]}
+	_put_sysfs_variable
+
+	# Select 2 target zones so that reset zone range also get tested
+	_get_blkzone_report "${TEST_DEV}" || return $?
+	zone_idx=$(_find_two_contiguous_seq_zones) || return $?
+	target_zones=( "${zone_idx}" "$((zone_idx + 1))" )
+
+	# Reset the 2 target zones and write 4KB in each zone to increment
+	# their write pointer positions
+	_reset_zones "${TEST_DEV}" "${zone_idx}" "2"
+	for i in "${target_zones[@]}"; do
+		_dd "${TEST_DEV}" "write" "${ZONE_STARTS[$i]}" \
+		    "8" "${phys_blk_size}"
+	done
+	_put_blkzone_report
+
+	# Check that the write pointers have moved by 8x512 B sectors
+	_get_blkzone_report "${TEST_DEV}" || return $?
+	for i in "${target_zones[@]}"; do
+		if [[ ${ZONE_WPTRS[$i]} -ne 8 ]]; then
+			echo -n "Unexpected write poiter position in zone ${i} "
+			echo "wp: ${ZONE_WPTRS[i]}"
+			return 1
+		fi
+	done
+
+	# Reset the 2 target zones
+	_reset_zones "${TEST_DEV}" "${zone_idx}" "2"
+	_put_blkzone_report
+
+	# Check that the write pointers have moved back to position 0
+	_get_blkzone_report "${TEST_DEV}" || return $?
+	for i in "${target_zones[@]}"; do
+		if [[ ${ZONE_WPTRS[i]} -ne 0 ]]; then
+			echo -n "Unexpected non-zero write poiter in zone ${i} "
+			echo "wp: ${ZONE_WPTRS[i]}"
+			return 1
+		fi
+	done
+	_put_blkzone_report
+
+	echo "Test complete"
+}
diff --git a/tests/zbd/003.out b/tests/zbd/003.out
new file mode 100644
index 0000000..7326ae4
--- /dev/null
+++ b/tests/zbd/003.out
@@ -0,0 +1,2 @@
+Running zbd/003
+Test complete
-- 
2.20.1

[PATCH blktests v3 12/13] zbd/004: Check write split accross sequential zones

[Shin'ichiro Kawasaki]

From: Masato Suzuki <masato.suzuki@wdc.com>

Check that write operations spanning a zone boundary are correctly
processed as 2 different write operations each fully within a single
zone.

Signed-off-by: Masato Suzuki <masato.suzuki@wdc.com>
---
 tests/zbd/004     | 90 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/zbd/004.out |  2 ++
 2 files changed, 92 insertions(+)
 create mode 100755 tests/zbd/004
 create mode 100644 tests/zbd/004.out

diff --git a/tests/zbd/004 b/tests/zbd/004
new file mode 100755
index 0000000..d8defa9
--- /dev/null
+++ b/tests/zbd/004
@@ -0,0 +1,90 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Western Digital Corporation or its affiliates.
+#
+# Check kernel splits write operations across a zone border. Select two
+# contiguous sequential write required zones and confirm write oprations
+# across the two zones succeed.
+
+. tests/zbd/rc
+
+DESCRIPTION="write split across sequential zones"
+QUICK=1
+CAN_BE_ZONED=1
+
+fallback_device() {
+	_fallback_null_blk_zoned
+}
+
+cleanup_fallback_device() {
+	_exit_null_blk
+}
+
+_check_zone_cond() {
+	local -i idx=${1}
+	local -i cond=${2}
+
+	if [[ ${ZONE_CONDS[idx]} -ne ${cond} ]]; then
+		echo -n "Zone ${idx} condition is not ${ZONE_COND_ARRAY[cond]} "
+		echo "cond: ${ZONE_COND_ARRAY[ZONE_CONDS[idx]]}"
+		return 1
+	fi
+}
+
+test_device() {
+	local -i idx
+	local -i phys_blk_size
+	local -i phys_blk_sectors
+
+	echo "Running ${TEST_NAME}"
+
+	# Get physical block size and sectors for dd.
+	_get_sysfs_variable "${TEST_DEV}" || return $?
+	phys_blk_size=${SYSFS_VARS[SV_PHYS_BLK_SIZE]}
+	phys_blk_sectors=${SYSFS_VARS[SV_PHYS_BLK_SECTORS]}
+	_put_sysfs_variable
+
+	# Find target sequential required zones and reset write pointers
+	_get_blkzone_report "${TEST_DEV}" || return $?
+	idx=$(_find_two_contiguous_seq_zones) || return $?
+	_reset_zones "${TEST_DEV}" "${idx}" "2"
+
+	# Confirm the zones are initialized
+	_put_blkzone_report
+	_get_blkzone_report "${TEST_DEV}" || return $?
+	_check_zone_cond "${idx}" "${ZONE_COND_EMPTY}" || return $?
+	_check_zone_cond "$((idx+1))" "${ZONE_COND_EMPTY}" || return $?
+
+	# Fill first target zone, remaining a physical block to write
+	if ! _dd "${TEST_DEV}" "write" "${ZONE_STARTS[idx]}" \
+	     $((ZONE_LENGTHS[idx] - phys_blk_sectors)) ${phys_blk_size} ; then
+		echo "Fill zone failed"
+		return 1
+	fi
+
+	# Write across the zone border as a single block write
+	local -i start_sector=$((ZONE_STARTS[idx+1] - phys_blk_sectors))
+	if ! _dd "${TEST_DEV}" "write" ${start_sector} \
+	     $((phys_blk_sectors * 2)) $((phys_blk_size * 2)) ; then
+		echo "Write across zone border failed"
+		return 1
+	fi
+
+	# Confirm the zone conditions are as expected
+	_put_blkzone_report
+	_get_blkzone_report "${TEST_DEV}" || return $?
+	_check_zone_cond "${idx}" "${ZONE_COND_FULL}" || return $?
+	_check_zone_cond "$((idx+1))" "${ZONE_COND_IMPLICIT_OPEN}" || return $?
+	if [[ ${ZONE_WPTRS[idx+1]} -ne ${phys_blk_sectors} ]]; then
+		echo -n "Unexpected write pointer for zone $((idx+1)) "
+		echo "wp: ${ZONE_WPTRS[idx+1]}"
+		return 1
+	fi
+
+	# Clean up
+	_reset_zones "${TEST_DEV}" "${idx}" "2"
+	_put_blkzone_report
+
+	echo "Test complete"
+}
+
diff --git a/tests/zbd/004.out b/tests/zbd/004.out
new file mode 100644
index 0000000..dd4ea94
--- /dev/null
+++ b/tests/zbd/004.out
@@ -0,0 +1,2 @@
+Running zbd/004
+Test complete
-- 
2.20.1

[PATCH blktests v3 13/13] zbd/005: Test write ordering

[Shin'ichiro Kawasaki]

From: Masato Suzuki <masato.suzuki@wdc.com>

Run a high queue depth direct sequential write fio job to check that
write requests are not being reordered when the deadline scheduler is
used. This test allowed to catch a bug fixed with commit
80e02039721 "block: mq-deadline: Fix write completion handling".

Signed-off-by: Masato Suzuki <masato.suzuki@wdc.com>
---
 tests/zbd/005     | 67 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/zbd/005.out |  2 ++
 2 files changed, 69 insertions(+)
 create mode 100755 tests/zbd/005
 create mode 100644 tests/zbd/005.out

diff --git a/tests/zbd/005 b/tests/zbd/005
new file mode 100755
index 0000000..bc9cad5
--- /dev/null
+++ b/tests/zbd/005
@@ -0,0 +1,67 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2018 Western Digital Corporation or its affiliates.
+#
+# Run a high queue depth direct sequential write fio job to check that
+# write requests are not being reordered when the deadline scheduler is
+# used. This test allowed to catch a bug fixed with commit 80e02039721
+# "block: mq-deadline: Fix write completion handling".
+
+. tests/zbd/rc
+
+DESCRIPTION="write command ordering"
+TIMED=1
+CAN_BE_ZONED=1
+
+requires() {
+	_have_fio_zbd_zonemode
+}
+
+fallback_device() {
+	_fallback_null_blk_zoned
+}
+
+cleanup_fallback_device() {
+	_exit_null_blk
+}
+
+_find_first_sequential_zone() {
+	for ((idx =  NR_CONV_ZONES; idx < REPORTED_COUNT; idx++)); do
+		if [[ ${ZONE_TYPES[idx]} -eq ${ZONE_TYPE_SEQ_WRITE_REQUIRED} ]];
+		then
+			echo "${idx}"
+			return 0
+		fi
+	done
+	echo "Sequential write required zone not found"
+
+	return 1
+}
+
+test_device() {
+	local -i zone_idx
+	local -i offset
+
+	echo "Running ${TEST_NAME}"
+
+	_get_sysfs_variable "${TEST_DEV}" || return $?
+	_get_blkzone_report "${TEST_DEV}" || return $?
+
+	zone_idx=$(_find_first_sequential_zone) || return $?
+	offset=$((ZONE_STARTS[zone_idx] * 512))
+
+	blkzone reset -o "${ZONE_STARTS[zone_idx]}" "${TEST_DEV}"
+
+	_test_dev_queue_set scheduler deadline
+
+	: "${TIMEOUT:=30}"
+	FIO_PERF_FIELDS=("write io" "write iops")
+	_fio_perf --filename="${TEST_DEV}" --name zbdwo --rw=write --direct=1 \
+		  --ioengine=libaio --iodepth=128 --bs=256k \
+		  --offset="${offset}"
+
+	_put_blkzone_report
+	_put_sysfs_variable
+
+	echo "Test complete"
+}
diff --git a/tests/zbd/005.out b/tests/zbd/005.out
new file mode 100644
index 0000000..3ff78c6
--- /dev/null
+++ b/tests/zbd/005.out
@@ -0,0 +1,2 @@
+Running zbd/005
+Test complete
-- 
2.20.1