All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: kaixuxia <xiakaixu1987@gmail.com>
Cc: fstests@vger.kernel.org, linux-xfs@vger.kernel.org,
	Eryu Guan <guaneryu@gmail.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	newtongao@tencent.com, jasperwang@tencent.com
Subject: Re: [PATCH v3 1/2] common: check if a given rename flag is supported in _requires_renameat2
Date: Wed, 18 Sep 2019 09:53:35 -0400	[thread overview]
Message-ID: <20190918135335.GC29377@bfoster> (raw)
In-Reply-To: <7d4b6a1d-98a1-1fcb-5ccf-991e537df77c@gmail.com>

On Wed, Sep 18, 2019 at 07:47:47PM +0800, kaixuxia wrote:
> Some testcases may require a special rename flag, such as RENAME_WHITEOUT,
> so add support check for if a given rename flag is supported in
> _requires_renameat2.
> 
> Signed-off-by: kaixuxia <kaixuxia@tencent.com>
> ---
>  common/renameat2  | 30 +++++++++++++++++++++++++++++-
>  tests/generic/024 | 13 +++----------
>  tests/generic/025 | 13 +++----------
>  tests/generic/078 | 13 +++----------
>  4 files changed, 38 insertions(+), 31 deletions(-)
> 
> diff --git a/common/renameat2 b/common/renameat2
> index f8d6d4f..9625d8c 100644
> --- a/common/renameat2
> +++ b/common/renameat2
> @@ -103,10 +103,38 @@ _rename_tests()
>  #
>  _requires_renameat2()
>  {
> +	local flags=$1
> +	local rename_dir=$TEST_DIR/$$
> +	local cmd=""
> +
>  	if test ! -x src/renameat2; then
>  		_notrun "renameat2 binary not found"
>  	fi
> -	if ! src/renameat2 -t; then
> +
> +	mkdir $rename_dir
> +	touch $rename_dir/foo
> +	case $flags in
> +	"noreplace")
> +		cmd="-n $rename_dir/foo $rename_dir/bar"
> +		;;
> +	"exchange")
> +		touch $rename_dir/bar
> +		cmd="-x $rename_dir/foo $rename_dir/bar"
> +		;;
> +	"whiteout")
> +		touch $rename_dir/bar
> +		cmd="-w $rename_dir/foo $rename_dir/bar"
> +		;;
> +	"")

I find putting the cmd="" here slightly more self documenting, but
that's just a nit. Otherwise seems fine:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> +		;;
> +	*)
> +		rm -rf $rename_dir
> +		_notrun "only support noreplace,exchange,whiteout rename flags, please check."
> +		;;
> +	esac
> +	if ! src/renameat2 -t $cmd; then
> +		rm -rf $rename_dir
>  		_notrun "kernel doesn't support renameat2 syscall"
>  	fi
> +	rm -rf $rename_dir
>  }
> diff --git a/tests/generic/024 b/tests/generic/024
> index 2888c66..9c1161a 100755
> --- a/tests/generic/024
> +++ b/tests/generic/024
> @@ -29,20 +29,13 @@ _supported_fs generic
>  _supported_os Linux
>  
>  _require_test
> -_requires_renameat2
> +_requires_renameat2 noreplace
>  _require_test_symlinks
>  
> -rename_dir=$TEST_DIR/$$
> -mkdir $rename_dir
> -touch $rename_dir/foo
> -if ! src/renameat2 -t -n $rename_dir/foo $rename_dir/bar; then
> -    rm -f $rename_dir/foo $rename_dir/bar; rmdir $rename_dir
> -    _notrun "fs doesn't support RENAME_NOREPLACE"
> -fi
> -rm -f $rename_dir/foo $rename_dir/bar
> -
>  # real QA test starts here
>  
> +rename_dir=$TEST_DIR/$$
> +mkdir $rename_dir
>  _rename_tests $rename_dir -n
>  rmdir $rename_dir
>  
> diff --git a/tests/generic/025 b/tests/generic/025
> index 0310efa..1ee9ad6 100755
> --- a/tests/generic/025
> +++ b/tests/generic/025
> @@ -29,20 +29,13 @@ _supported_fs generic
>  _supported_os Linux
>  
>  _require_test
> -_requires_renameat2
> +_requires_renameat2 exchange
>  _require_test_symlinks
>  
> -rename_dir=$TEST_DIR/$$
> -mkdir $rename_dir
> -touch $rename_dir/foo $rename_dir/bar
> -if ! src/renameat2 -t -x $rename_dir/foo $rename_dir/bar; then
> -    rm -f $rename_dir/foo $rename_dir/bar; rmdir $rename_dir
> -    _notrun "fs doesn't support RENAME_EXCHANGE"
> -fi
> -rm -f $rename_dir/foo $rename_dir/bar
> -
>  # real QA test starts here
>  
> +rename_dir=$TEST_DIR/$$
> +mkdir $rename_dir
>  _rename_tests $rename_dir -x
>  rmdir $rename_dir
>  
> diff --git a/tests/generic/078 b/tests/generic/078
> index 9608574..37f3201 100755
> --- a/tests/generic/078
> +++ b/tests/generic/078
> @@ -29,20 +29,13 @@ _supported_fs generic
>  _supported_os Linux
>  
>  _require_test
> -_requires_renameat2
> +_requires_renameat2 whiteout
>  _require_test_symlinks
>  
> -rename_dir=$TEST_DIR/$$
> -mkdir $rename_dir
> -touch $rename_dir/foo $rename_dir/bar
> -if ! src/renameat2 -t -w $rename_dir/foo $rename_dir/bar; then
> -    rm -f $rename_dir/foo $rename_dir/bar; rmdir $rename_dir
> -    _notrun "fs doesn't support RENAME_WHITEOUT"
> -fi
> -rm -f $rename_dir/foo $rename_dir/bar
> -
>  # real QA test starts here
>  
> +rename_dir=$TEST_DIR/$$
> +mkdir $rename_dir
>  _rename_tests $rename_dir -w
>  rmdir $rename_dir
>  
> -- 
> 1.8.3.1
> 
> -- 
> kaixuxia

  reply	other threads:[~2019-09-18 13:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-18 11:47 [PATCH v3 1/2] common: check if a given rename flag is supported in _requires_renameat2 kaixuxia
2019-09-18 13:53 ` Brian Foster [this message]
2019-09-21 14:34 ` Eryu Guan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190918135335.GC29377@bfoster \
    --to=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=fstests@vger.kernel.org \
    --cc=guaneryu@gmail.com \
    --cc=jasperwang@tencent.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=newtongao@tencent.com \
    --cc=xiakaixu1987@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.