Re: [PATCH blktests 3/3] common/xfs: update _xfs_run_fio_verify_io to accept the size parameter

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

On Oct 24, 2022 / 14:13, Yi Zhang wrote:
> This commit alo updated nvme/012 nvme/013 nvme/035 to pass the size

Let's note why we do this. How about this descprtion?

  Change fio I/O size of nvme/012,013,035 from 950m to 900m, since recent
  change increased the xfs log size and it caused fio failure with I/O
  size 950m.

  Also add size parameter to _run_fio_verify_io. This allows to move the
  fio I/O size definition from common/xfs to the test case, so that device
  size and fio I/O size are both defined at single place.

I think the commit title needs to reflect the motivations above, like:

  nvme/012,013,035: change fio I/O size and move size definition place

> parameter to _xfs_run_fio_verify_io
> 
> Signed-off-by: Yi Zhang <yi.zhang@redhat.com>

How about to add "Link:" tag to refer our discussion?

  Link: https://lore.kernel.org/linux-block/20221019051244.810755-1-yi.zhang@redhat.com/


> ---
>  common/xfs     | 3 ++-
>  tests/nvme/012 | 2 +-
>  tests/nvme/013 | 2 +-
>  tests/nvme/035 | 9 ++++++++-
>  4 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/common/xfs b/common/xfs
> index 846a5ef..2c5d961 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -23,10 +23,11 @@ _xfs_mkfs_and_mount() {
>  _xfs_run_fio_verify_io() {
>  	local mount_dir="/mnt/blktests"
>  	local bdev=$1
> +	local sz=$2
>  
>  	_xfs_mkfs_and_mount "${bdev}" "${mount_dir}" >> "${FULL}" 2>&1
>  
> -	_run_fio_verify_io --size=950m --directory="${mount_dir}/"
> +	_run_fio_verify_io --size="$sz" --directory="${mount_dir}/"
>  
>  	umount "${mount_dir}" >> "${FULL}" 2>&1
>  	rm -fr "${mount_dir}"
> diff --git a/tests/nvme/012 b/tests/nvme/012
> index c9d2438..e60082c 100755
> --- a/tests/nvme/012
> +++ b/tests/nvme/012
> @@ -44,7 +44,7 @@ test() {
>  	cat "/sys/block/${nvmedev}n1/uuid"
>  	cat "/sys/block/${nvmedev}n1/wwid"
>  
> -	_xfs_run_fio_verify_io "/dev/${nvmedev}n1"
> +	_xfs_run_fio_verify_io "/dev/${nvmedev}n1" "900m"
>  
>  	_nvme_disconnect_subsys "${subsys_name}"
>  
> diff --git a/tests/nvme/013 b/tests/nvme/013
> index 265b696..9d60a7d 100755
> --- a/tests/nvme/013
> +++ b/tests/nvme/013
> @@ -41,7 +41,7 @@ test() {
>  	cat "/sys/block/${nvmedev}n1/uuid"
>  	cat "/sys/block/${nvmedev}n1/wwid"
>  
> -	_xfs_run_fio_verify_io "/dev/${nvmedev}n1"
> +	_xfs_run_fio_verify_io "/dev/${nvmedev}n1" "900m"
>  
>  	_nvme_disconnect_subsys "${subsys_name}"
>  
> diff --git a/tests/nvme/035 b/tests/nvme/035
> index ee78a75..31de0d1 100755
> --- a/tests/nvme/035
> +++ b/tests/nvme/035
> @@ -21,14 +21,21 @@ test_device() {
>  	local ctrldev
>  	local nsdev
>  	local port
> +	local test_dev_sz
>  
>  	echo "Running ${TEST_NAME}"
>  
>  	_setup_nvmet
>  	port=$(_nvmet_passthru_target_setup "${subsys}")
>  	nsdev=$(_nvmet_passthru_target_connect "${nvme_trtype}" "${subsys}")
> +	test_dev_sz=$(_get_test_dev_size_mb)
>  
> -	_xfs_run_fio_verify_io "${nsdev}"
> +	if (( "$test_dev_sz" < 1024 )); then
> +		echo "Test dev: $TEST_DEV should at leat 1024m"
> +		return 1
> +
> +	fi

As I commented on the second patch, I suggest to move this device size check
part to the second patch.

Other changes looks good tome.

> +	_xfs_run_fio_verify_io "${nsdev}" "900m"
>  
>  	_nvme_disconnect_subsys "${subsys}"
>  	_nvmet_passthru_target_cleanup "${port}" "${subsys}"
> -- 
> 2.34.1
> 

-- 
Shin'ichiro Kawasaki