On Thu, Jun 02, 2022 at 01:17:16PM +0800, Zorro Lang wrote:
> Due to generic/139 tests base on 512 bytes aligned, so skip this test
> if the minimum dio write size >512. This patch also change the
> common/rc::_require_dio helper, supports a minimum aligned size
> argument.
>
> Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> ---
>
> Thanks the review from Darrick on v2, this v3 remove some duplicate code
> which I forgot.
>
> Thanks,
> Zorro
>
> common/rc | 13 ++++++++++---
> tests/generic/139 | 2 +-
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 2f31ca46..9823e3a1 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2721,9 +2721,12 @@ _require_xfs_io_command()
> fi
> }
>
> -# check that kernel and filesystem support direct I/O
> +# check that kernel and filesystem support direct I/O, and check if "$1" size
> +# aligned (optional) is supported
> _require_odirect()
> {
> + local alignment=${1:+"-b $1"}
This might be a nit, but you might want to do this instead:
local blocksize=$1
local align_args=${1:+"-b $1"}
So that there's only one "$1" to change if the arguments ever get
rearranged. But that might never happen, and this feels nearly like
pointless navelgazing.
If you're happy with things the way they are, the logic looks ok so:
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> +
> if [ $FSTYP = "ext4" ] || [ $FSTYP = "f2fs" ] ; then
> if echo "$MOUNT_OPTIONS" | grep -q "test_dummy_encryption"; then
> _notrun "$FSTYP encryption doesn't support O_DIRECT"
> @@ -2735,9 +2738,13 @@ _require_odirect()
> fi
> fi
> local testfile=$TEST_DIR/$$.direct
> - $XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile > /dev/null 2>&1
> + $XFS_IO_PROG -F -f -d -c "pwrite $alignment 0 20k" $testfile > /dev/null 2>&1
> if [ $? -ne 0 ]; then
> - _notrun "O_DIRECT is not supported"
> + if [ -n "$alignment" ]; then
> + _notrun "O_DIRECT aligned to $1 bytes is not supported"
> + else
> + _notrun "O_DIRECT is not supported"
> + fi
> fi
> rm -f $testfile 2>&1 > /dev/null
> }
> diff --git a/tests/generic/139 b/tests/generic/139
> index 0bbc222c..3eb1519d 100755
> --- a/tests/generic/139
> +++ b/tests/generic/139
> @@ -26,7 +26,7 @@ _cleanup()
> # real QA test starts here
> _require_test_reflink
> _require_cp_reflink
> -_require_odirect
> +_require_odirect 512
>
> testdir=$TEST_DIR/test-$seq
> rm -rf $testdir
> --
> 2.31.1
>
