linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] common: check if a given rename flag is supported in _requires_renameat2
@ 2019-09-11 13:15 kaixuxia
  2019-09-13 17:36 ` Brian Foster
  0 siblings, 1 reply; 3+ messages in thread
From: kaixuxia @ 2019-09-11 13:15 UTC (permalink / raw)
  To: fstests; +Cc: linux-xfs, Eryu Guan, Darrick J. Wong, newtongao, jasperwang

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  | 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
+	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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] common: check if a given rename flag is supported in _requires_renameat2
  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
  2019-09-16  2:11   ` kaixuxia
  0 siblings, 1 reply; 3+ messages in thread
From: Brian Foster @ 2019-09-13 17:36 UTC (permalink / raw)
  To: kaixuxia
  Cc: fstests, linux-xfs, Eryu Guan, Darrick J. Wong, newtongao, jasperwang

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] common: check if a given rename flag is supported in _requires_renameat2
  2019-09-13 17:36 ` Brian Foster
@ 2019-09-16  2:11   ` kaixuxia
  0 siblings, 0 replies; 3+ messages in thread
From: kaixuxia @ 2019-09-16  2:11 UTC (permalink / raw)
  To: Brian Foster
  Cc: fstests, linux-xfs, Eryu Guan, Darrick J. Wong, newtongao, jasperwang

On 2019/9/14 1:36, Brian Foster wrote:
> 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?
> 
Yeah, make more sense.

> 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

-- 
kaixuxia

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-16  2:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2019-09-16  2:11   ` kaixuxia

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).