linux-kernel - Re: [PATCH REPOST blktests v2 1/9] nvme-rc: Auto convert test device size info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 28 Apr 2023 02:39:19 +0000
From:   Shinichiro Kawasaki <shinichiro.kawasaki@....com>
To:     Daniel Wagner <dwagner@...e.de>
CC:     "linux-nvme@...ts.infradead.org" <linux-nvme@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        Chaitanya Kulkarni <kch@...dia.com>,
        Shin'ichiro Kawasaki <shinichiro@...tmail.com>
Subject: Re: [PATCH REPOST blktests v2 1/9] nvme-rc: Auto convert test device
 size info

This patch looks good to me. One nit in the commit title is the prefix
"nvme-rc" which is new. How about "nvme" or "nvme/rc"?

Please find one more nit comment in line.

On Apr 21, 2023 / 08:04, Daniel Wagner wrote:
> Introduce a convert_to_mb() helper which converts the size argument
> to MBytes and use in test device require function. This makes it
> possible to use user input strings in future.
> 
> Signed-off-by: Daniel Wagner <dwagner@...e.de>
> ---
>  common/rc      | 30 +++++++++++++++++++++++++++---
>  tests/nvme/035 |  2 +-
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index af4c0b1cab22..dd0afda3d821 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -324,9 +324,12 @@ _get_pci_parent_from_blkdev() {
>  		tail -2 | head -1
>  }
>  
> -_require_test_dev_size_mb() {
> -	local require_sz_mb=$1
> -	local test_dev_sz_mb=$(($(blockdev --getsize64 "$TEST_DEV")/1024/1024))
> +_require_test_dev_size() {
> +	local require_sz_mb
> +	local test_dev_sz_mb
> +
> +	require_sz_mb="$(convert_to_mb "$1")"
> +	test_dev_sz_mb="$(($(blockdev --getsize64 "$TEST_DEV")/1024/1024))"
>  
>  	if (( "$test_dev_sz_mb" < "$require_sz_mb" )); then
>  		SKIP_REASONS+=("${TEST_DEV} required at least ${require_sz_mb}m")
> @@ -422,3 +425,24 @@ _have_writeable_kmsg() {
>  _run_user() {
>  	su "$NORMAL_USER" -c "$1"
>  }
> +
> +convert_to_mb()
> +{
> +    local str=$1
> +    local res
> +
> +    res=$(echo "${str}" | sed -n 's/^\([0-9]\+\)$/\1/p')
> +    if [[ -n "${res}" ]]; then
> +        echo "$((res / 1024 / 1024))"
> +    fi
> +
> +    res=$(echo "${str}" | sed -n 's/^\([0-9]\+\)[mM]$/\1/p')
> +    if [[ -n "${res}" ]]; then
> +        echo "$((res))"
> +    fi
> +
> +    res=$(echo "${str}" | sed -n 's/^\([0-9]\+\)[gG]$/\1/p')
> +    if [[ -n "${res}" ]]; then
> +        echo "$((res * 1024))"
> +    fi
> +}

Nit: this function uses not tabs but spaces for indent.

> diff --git a/tests/nvme/035 b/tests/nvme/035
> index d169e351e3d0..eb1024edddbf 100755
> --- a/tests/nvme/035
> +++ b/tests/nvme/035
> @@ -17,7 +17,7 @@ requires() {
>  }
>  
>  device_requires() {
> -	_require_test_dev_size_mb 1024
> +	_require_test_dev_size 1024m
>  }
>  
>  test_device() {
> -- 
> 2.40.0
>