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

[Omar Sandoval <osandov@osandov.com>]

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
>