Re: [PATCH blktests 2/3] common/rc: add one function to get test dev size in mb

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

On Oct 24, 2022 / 14:13, Yi Zhang wrote:

Short explanation will help to understand why we do this: something like,

  nvme/035 has minimum TEST_DEV size requirement. Add a helper
  function to check it.

> Signed-off-by: Yi Zhang <yi.zhang@redhat.com>
> ---
>  common/rc | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index e490041..847be1b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -324,6 +324,14 @@ _get_pci_parent_from_blkdev() {
>  		tail -2 | head -1
>  }
>  
> +_get_test_dev_size_mb() {
> +	local test_dev_sz

Nit: one empty line will make it easier to read.

> +	test_dev_sz=$(blockdev --getsize64 "$TEST_DEV")
> +
> +	echo $((test_dev_sz / 1024 / 1024))
> +

Nit: an empty line not needed.

> +}
> +

I suggest to improve this new function to _require_test_dev_size_mb(). It takes
1st argument as the minimum size size in MB, and if TEST_DEV size is smaller
than that, it set SKIP_REASON and return 1. We can add device_requires() to
nvme/035 to call _require_test_dev_size_mb(). This will skip the test case when
the TEST_DEV is small, and do not report it as a failure.

I also suggest to include nvme/035 change for the size check in this patch. I
think one shot change for function addition and function call will be simpler
for this charge.

>  _require_test_dev_in_hotplug_slot() {
>  	local parent
>  	parent="$(_get_pci_parent_from_blkdev)"
> -- 
> 2.34.1
> 

-- 
Shin'ichiro Kawasaki