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 1/2] common: check if a given rename flag is supported in _requires_renameat2
Date: Fri, 13 Sep 2019 13:36:14 -0400 [thread overview]
Message-ID: <20190913173614.GC28512@bfoster> (raw)
In-Reply-To: <719f7bb3-96db-7563-56d8-56ed765fabc4@gmail.com>
On Wed, Sep 11, 2019 at 09:15:16PM +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>
> ---
Looks mostly reasonable...
> common/renameat2 | 41 +++++++++++++++++++++++++++++++++++++++--
> tests/generic/024 | 13 +++----------
> tests/generic/025 | 13 +++----------
> tests/generic/078 | 13 +++----------
> 4 files changed, 48 insertions(+), 32 deletions(-)
>
> diff --git a/common/renameat2 b/common/renameat2
> index f8d6d4f..7bb81e0 100644
> --- a/common/renameat2
> +++ b/common/renameat2
> @@ -103,10 +103,47 @@ _rename_tests()
> #
> _requires_renameat2()
> {
> + local flags=$1
> + local rename_dir=$TEST_DIR/$$
> +
> if test ! -x src/renameat2; then
> _notrun "renameat2 binary not found"
> fi
> - if ! src/renameat2 -t; then
> - _notrun "kernel doesn't support renameat2 syscall"
> +
> + if test -z "$flags"; then
> + src/renameat2 -t
> + [ $? -eq 0 ] || _notrun "kernel doesn't support renameat2 syscall"
> + return
> fi
> +
> + case $flags in
> + "noreplace")
> + 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
> + ;;
> + "exchange")
> + 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
> + ;;
> + "whiteout")
> + 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
> + ;;
> + *)
> + _notrun "only support noreplace,exchange,whiteout rename flags, please check."
> + ;;
> + esac
... but it seems like this could be simplified to reduce (duplicate)
code. For example:
mkdir $rename_dir
touch $rename_dir/foo
cmd=""
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"
;;
...
if ! src/renameat2 -t $cmd; then
rm -rf $rename_dir
_notrun "fs doesn't support renameat2"
fi
rm -rf $rename_dir
Hm?
Brian
> + rm -fr $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
next prev parent reply other threads:[~2019-09-13 17:36 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-11 13:15 [PATCH 1/2] common: check if a given rename flag is supported in _requires_renameat2 kaixuxia
2019-09-13 17:36 ` Brian Foster [this message]
2019-09-16 2:11 ` kaixuxia
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=20190913173614.GC28512@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).