[PATCH v2 1/2] xfs/270: Fix syntax error

[PATCH v2 1/2] xfs/270: Fix syntax error

[Yang Xu]

Without xfsprogs commit
f4afdcb0a ("xfs_db: clean up the salvage read callsites in set_cur()"),
this case will fail as below:

+/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected

With this xfsprogs bug, xfs_db can't get new_ro_compat value, so report
this bash syntax error.

Fix this and add a helper output for xfsprogs bug.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 tests/xfs/270 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/xfs/270 b/tests/xfs/270
index b740c379..4954e943 100755
--- a/tests/xfs/270
+++ b/tests/xfs/270
@@ -47,9 +47,12 @@ _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \
 new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \
 						2>/dev/null)
 
-# verify the new ro_compat field is correct.
-if [ $new_ro_compat != $ro_compat ]; then
+# verify the new ro_compat field is correct. Without xfsprogs commit
+# f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()"),
+# we can't get new_ro_compat value.
+if [ "$new_ro_compat" != "$ro_compat" ]; then
 	echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat"
+	echo "May hit a known xfsprog bug"
 fi
 
 # rw mount with unknown ro-compat feature should fail
-- 
2.27.0

[PATCH v2 2/2] xfs/191: test xfs_db get corrupted value

[Yang Xu]

This is a regression test that xfs_db code doesn't clean up the
LIBXFS_READBUF_SALVAGE call sites in set_cur so that we
can't use the return value directly and scraping it out later.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 tests/xfs/191     | 33 +++++++++++++++++++++++++++++++++
 tests/xfs/191.out |  5 +++++
 2 files changed, 38 insertions(+)
 create mode 100755 tests/xfs/191
 create mode 100644 tests/xfs/191.out

diff --git a/tests/xfs/191 b/tests/xfs/191
new file mode 100755
index 00000000..98e9e186
--- /dev/null
+++ b/tests/xfs/191
@@ -0,0 +1,33 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+#
+# FS QA Test 191
+#
+# Regression test for xfsprogs commit
+# f4afdcb0ad11 ("xfs_db: clean up the salvage read callsites in set_cur()")
+#
+# This case test xfs_db whether can get the new magicnum field value even we
+# just have corrupted this field value.
+#
+
+. ./common/preamble
+_begin_fstest auto quick
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+_supported_fs xfs
+
+_scratch_mkfs_xfs >>$seqres.full 2>&1
+
+# write the bad magicnum field value(0) to the superblock
+_scratch_xfs_set_metadata_field "magicnum" "0" "sb 1"
+
+#Even magicnum filed has been corrupted, we still can read this filed value
+_scratch_xfs_get_metadata_field "magicnum" "sb 1"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/191.out b/tests/xfs/191.out
new file mode 100644
index 00000000..71826198
--- /dev/null
+++ b/tests/xfs/191.out
@@ -0,0 +1,5 @@
+QA output created by 191
+Allowing write of corrupted data with good CRC
+magicnum = 0
+bad magic number
+0
-- 
2.27.0

Re: [PATCH v2 2/2] xfs/191: test xfs_db get corrupted value

[Zorro Lang]

On Tue, Jul 05, 2022 at 05:20:08PM +0800, Yang Xu wrote:
> This is a regression test that xfs_db code doesn't clean up the
> LIBXFS_READBUF_SALVAGE call sites in set_cur so that we
> can't use the return value directly and scraping it out later.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  tests/xfs/191     | 33 +++++++++++++++++++++++++++++++++
>  tests/xfs/191.out |  5 +++++

This seqnum has been taken, please rebase to latest xfstests for-next branch.

>  2 files changed, 38 insertions(+)
>  create mode 100755 tests/xfs/191
>  create mode 100644 tests/xfs/191.out
> 
> diff --git a/tests/xfs/191 b/tests/xfs/191
> new file mode 100755
> index 00000000..98e9e186
> --- /dev/null
> +++ b/tests/xfs/191
> @@ -0,0 +1,33 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
> +#
> +# FS QA Test 191
> +#
> +# Regression test for xfsprogs commit
> +# f4afdcb0ad11 ("xfs_db: clean up the salvage read callsites in set_cur()")
> +#
> +# This case test xfs_db whether can get the new magicnum field value even we
> +# just have corrupted this field value.
> +#
> +
> +. ./common/preamble
> +_begin_fstest auto quick

This's a test for xfs_db, so add "db" test group please.

> +
> +# Import common functions.
> +. ./common/filter

Do you use any helpers in common/filter?

> +
> +# real QA test starts here
> +_supported_fs xfs

_require_scratch_nocheck ??

And ...

_fixed_by_git_commit xfsprogs f4afdcb0ad11 \
	"xfs_db: clean up the salvage read callsites in set_cur()"

Thanks,
Zorro

> +
> +_scratch_mkfs_xfs >>$seqres.full 2>&1
> +
> +# write the bad magicnum field value(0) to the superblock
> +_scratch_xfs_set_metadata_field "magicnum" "0" "sb 1"
> +
> +#Even magicnum filed has been corrupted, we still can read this filed value
> +_scratch_xfs_get_metadata_field "magicnum" "sb 1"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/191.out b/tests/xfs/191.out
> new file mode 100644
> index 00000000..71826198
> --- /dev/null
> +++ b/tests/xfs/191.out
> @@ -0,0 +1,5 @@
> +QA output created by 191
> +Allowing write of corrupted data with good CRC
> +magicnum = 0
> +bad magic number
> +0
> -- 
> 2.27.0
> 

Re: [PATCH v2 2/2] xfs/191: test xfs_db get corrupted value

[xuyang2018.jy]

on 2022/07/19 13:21, Zorro Lang wrote:
> On Tue, Jul 05, 2022 at 05:20:08PM +0800, Yang Xu wrote:
>> This is a regression test that xfs_db code doesn't clean up the
>> LIBXFS_READBUF_SALVAGE call sites in set_cur so that we
>> can't use the return value directly and scraping it out later.
>>
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>   tests/xfs/191     | 33 +++++++++++++++++++++++++++++++++
>>   tests/xfs/191.out |  5 +++++
> 
> This seqnum has been taken, please rebase to latest xfstests for-next branch.

I will use xfs/533 seqnum.
> 
>>   2 files changed, 38 insertions(+)
>>   create mode 100755 tests/xfs/191
>>   create mode 100644 tests/xfs/191.out
>>
>> diff --git a/tests/xfs/191 b/tests/xfs/191
>> new file mode 100755
>> index 00000000..98e9e186
>> --- /dev/null
>> +++ b/tests/xfs/191
>> @@ -0,0 +1,33 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
>> +#
>> +# FS QA Test 191
>> +#
>> +# Regression test for xfsprogs commit
>> +# f4afdcb0ad11 ("xfs_db: clean up the salvage read callsites in set_cur()")
>> +#
>> +# This case test xfs_db whether can get the new magicnum field value even we
>> +# just have corrupted this field value.
>> +#
>> +
>> +. ./common/preamble
>> +_begin_fstest auto quick
> 
> This's a test for xfs_db, so add "db" test group please.

YES.
> 
>> +
>> +# Import common functions.
>> +. ./common/filter
> 
> Do you use any helpers in common/filter?

No, will remove.
> 
>> +
>> +# real QA test starts here
>> +_supported_fs xfs
> 
> _require_scratch_nocheck ??
> 
> And ...
> 
> _fixed_by_git_commit xfsprogs f4afdcb0ad11 \
> 	"xfs_db: clean up the salvage read callsites in set_cur()"

Yes, will add them in v3.

Best Regards
Yang Xu
> 
> Thanks,
> Zorro
> 
>> +
>> +_scratch_mkfs_xfs >>$seqres.full 2>&1
>> +
>> +# write the bad magicnum field value(0) to the superblock
>> +_scratch_xfs_set_metadata_field "magicnum" "0" "sb 1"
>> +
>> +#Even magicnum filed has been corrupted, we still can read this filed value
>> +_scratch_xfs_get_metadata_field "magicnum" "sb 1"
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/xfs/191.out b/tests/xfs/191.out
>> new file mode 100644
>> index 00000000..71826198
>> --- /dev/null
>> +++ b/tests/xfs/191.out
>> @@ -0,0 +1,5 @@
>> +QA output created by 191
>> +Allowing write of corrupted data with good CRC
>> +magicnum = 0
>> +bad magic number
>> +0
>> -- 
>> 2.27.0
>>
> 

Re: [PATCH v2 1/2] xfs/270: Fix syntax error

[xuyang2018.jy]

on 2022/07/05 17:20, Yang Xu wrote:
> Without xfsprogs commit
> f4afdcb0a ("xfs_db: clean up the salvage read callsites in set_cur()"),
> this case will fail as below:

Ping.

> 
> +/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected
> 
> With this xfsprogs bug, xfs_db can't get new_ro_compat value, so report
> this bash syntax error.
> 
> Fix this and add a helper output for xfsprogs bug.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>   tests/xfs/270 | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/270 b/tests/xfs/270
> index b740c379..4954e943 100755
> --- a/tests/xfs/270
> +++ b/tests/xfs/270
> @@ -47,9 +47,12 @@ _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \
>   new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \
>   						2>/dev/null)
>   
> -# verify the new ro_compat field is correct.
> -if [ $new_ro_compat != $ro_compat ]; then
> +# verify the new ro_compat field is correct. Without xfsprogs commit
> +# f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()"),
> +# we can't get new_ro_compat value.
> +if [ "$new_ro_compat" != "$ro_compat" ]; then
>   	echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat"
> +	echo "May hit a known xfsprog bug"
>   fi
>   
>   # rw mount with unknown ro-compat feature should fail

Re: [PATCH v2 1/2] xfs/270: Fix syntax error

[Zorro Lang]

On Tue, Jul 05, 2022 at 05:20:07PM +0800, Yang Xu wrote:
> Without xfsprogs commit
> f4afdcb0a ("xfs_db: clean up the salvage read callsites in set_cur()"),
> this case will fail as below:
> 
> +/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected
> 
> With this xfsprogs bug, xfs_db can't get new_ro_compat value, so report
> this bash syntax error.
> 
> Fix this and add a helper output for xfsprogs bug.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  tests/xfs/270 | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/270 b/tests/xfs/270
> index b740c379..4954e943 100755
> --- a/tests/xfs/270
> +++ b/tests/xfs/270
> @@ -47,9 +47,12 @@ _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \
>  new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \
>  						2>/dev/null)
>  
> -# verify the new ro_compat field is correct.
> -if [ $new_ro_compat != $ro_compat ]; then
> +# verify the new ro_compat field is correct. Without xfsprogs commit
> +# f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()"),
> +# we can't get new_ro_compat value.
> +if [ "$new_ro_compat" != "$ro_compat" ]; then
>  	echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat"
> +	echo "May hit a known xfsprog bug"
>  fi
>  
>  # rw mount with unknown ro-compat feature should fail
> -- 
> 2.27.0
> 

[PATCH v3 1/2] xfs/270: Fix syntax error

[Yang Xu]

Without xfsprogs commit
f4afdcb0a ("xfs_db: clean up the salvage read callsites in set_cur()"),
this case will fail as below:

+/var/lib/xfstests/tests/xfs/270: line 51: [: !=: unary operator expected

With this xfsprogs bug, xfs_db can't get new_ro_compat value, so report
this bash syntax error.

Fix this and add a helper output for xfsprogs bug.

Reviewed-by: Zorro Lang <zlang@redhat.com>
Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 tests/xfs/270 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/xfs/270 b/tests/xfs/270
index b740c379..4954e943 100755
--- a/tests/xfs/270
+++ b/tests/xfs/270
@@ -47,9 +47,12 @@ _scratch_xfs_set_metadata_field "features_ro_compat" "$ro_compat" "sb 0" \
 new_ro_compat=$(_scratch_xfs_get_metadata_field "features_ro_compat" "sb 0" \
 						2>/dev/null)
 
-# verify the new ro_compat field is correct.
-if [ $new_ro_compat != $ro_compat ]; then
+# verify the new ro_compat field is correct. Without xfsprogs commit
+# f4afdcb0ad ("xfs_db: clean up the salvage read callsites in set_cur()"),
+# we can't get new_ro_compat value.
+if [ "$new_ro_compat" != "$ro_compat" ]; then
 	echo "Unable to set new features_ro_compat. Wanted $ro_compat, got $new_ro_compat"
+	echo "May hit a known xfsprog bug"
 fi
 
 # rw mount with unknown ro-compat feature should fail
-- 
2.23.0

[PATCH v3 2/2] xfs/533: test xfs_db whether still can get corrupted field value

[Yang Xu]

This is a regression test that xfs_db code doesn't clean up the
LIBXFS_READBUF_SALVAGE call sites in set_cur so that we can't use the
return value directly and scraping it out later.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 tests/xfs/533     | 34 ++++++++++++++++++++++++++++++++++
 tests/xfs/533.out |  5 +++++
 2 files changed, 39 insertions(+)
 create mode 100755 tests/xfs/533
 create mode 100644 tests/xfs/533.out

diff --git a/tests/xfs/533 b/tests/xfs/533
new file mode 100755
index 00000000..c0a4c0d2
--- /dev/null
+++ b/tests/xfs/533
@@ -0,0 +1,34 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
+#
+# FS QA Test 533
+#
+# Regression test for xfsprogs commit
+# f4afdcb0ad11 ("xfs_db: clean up the salvage read callsites in set_cur()")
+#
+# This case test xfs_db whether can get the new magicnum field value even we
+# just have corrupted this field value.
+#
+
+. ./common/preamble
+_begin_fstest auto quick db
+
+# real QA test starts here
+_supported_fs xfs
+_fixed_by_git_commit xfsprogs f4afdcb0ad11 \
+	"xfs_db: clean up the salvage read callsites in set_cur()"
+#skip fs check because invalid superblock 1
+_require_scratch_nocheck
+
+_scratch_mkfs_xfs >>$seqres.full 2>&1
+
+# write the bad magicnum field value(0) to the superblock 1
+_scratch_xfs_set_metadata_field "magicnum" "0" "sb 1"
+
+#Even magicnum filed has been corrupted, we still can read this filed value
+_scratch_xfs_get_metadata_field "magicnum" "sb 1"
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/533.out b/tests/xfs/533.out
new file mode 100644
index 00000000..7deb78a3
--- /dev/null
+++ b/tests/xfs/533.out
@@ -0,0 +1,5 @@
+QA output created by 533
+Allowing write of corrupted data with good CRC
+magicnum = 0
+bad magic number
+0
-- 
2.23.0

Re: [PATCH v3 2/2] xfs/533: test xfs_db whether still can get corrupted field value

[Darrick J. Wong]

On Tue, Jul 19, 2022 at 04:31:13PM +0800, Yang Xu wrote:
> This is a regression test that xfs_db code doesn't clean up the
> LIBXFS_READBUF_SALVAGE call sites in set_cur so that we can't use the
> return value directly and scraping it out later.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  tests/xfs/533     | 34 ++++++++++++++++++++++++++++++++++
>  tests/xfs/533.out |  5 +++++
>  2 files changed, 39 insertions(+)
>  create mode 100755 tests/xfs/533
>  create mode 100644 tests/xfs/533.out
> 
> diff --git a/tests/xfs/533 b/tests/xfs/533
> new file mode 100755
> index 00000000..c0a4c0d2
> --- /dev/null
> +++ b/tests/xfs/533
> @@ -0,0 +1,34 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
> +#
> +# FS QA Test 533
> +#
> +# Regression test for xfsprogs commit
> +# f4afdcb0ad11 ("xfs_db: clean up the salvage read callsites in set_cur()")
> +#
> +# This case test xfs_db whether can get the new magicnum field value even we
> +# just have corrupted this field value.
> +#
> +
> +. ./common/preamble
> +_begin_fstest auto quick db
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_fixed_by_git_commit xfsprogs f4afdcb0ad11 \
> +	"xfs_db: clean up the salvage read callsites in set_cur()"
> +#skip fs check because invalid superblock 1
> +_require_scratch_nocheck
> +
> +_scratch_mkfs_xfs >>$seqres.full 2>&1
> +
> +# write the bad magicnum field value(0) to the superblock 1
> +_scratch_xfs_set_metadata_field "magicnum" "0" "sb 1"
> +
> +#Even magicnum filed has been corrupted, we still can read this filed value

"field"?

With that fixed, this looks good to me...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D


> +_scratch_xfs_get_metadata_field "magicnum" "sb 1"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/533.out b/tests/xfs/533.out
> new file mode 100644
> index 00000000..7deb78a3
> --- /dev/null
> +++ b/tests/xfs/533.out
> @@ -0,0 +1,5 @@
> +QA output created by 533
> +Allowing write of corrupted data with good CRC
> +magicnum = 0
> +bad magic number
> +0
> -- 
> 2.23.0
>