[PATCH] xfs/424: add check for crc and finobt

[PATCH] xfs/424: add check for crc and finobt

[Xiao Yang]

1) When crc is not supported or disabled, this case could not indicate
   any expected error.  Running all tests without crc makes no sense.

2) When finobt is disabled, xfs_db fails to get current address of
   free_root, as below:
   xfs_db -c "agi" -c "addr free_root" -c "daddr" /dev/sda11
   Metadata CRC error detected at xfs_inobt block 0x0/0x1000
   ...
   Running related tests without finobt makes no sense.

We add check to fix it.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 tests/xfs/424 | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/xfs/424 b/tests/xfs/424
index 0a1eef9..165c390 100755
--- a/tests/xfs/424
+++ b/tests/xfs/424
@@ -55,9 +55,15 @@ rm -f $seqres.full
 _supported_os Linux
 _supported_fs xfs
 _require_scratch
+_require_xfs_mkfs_crc
 
 echo "Silence is golden."
 
+crc_finobt_enabled="crc=1,finobt=0"
+
+[ _scratch_mkfs_xfs_supported -m crc=1,finobt=1 >/dev/null 2>&1 ] && \
+	crc_finobt_enabled="crc=1,finobt=1"
+
 # real QA test starts here
 
 # for different sector sizes, ensure no CRC errors are falsely reported.
@@ -77,7 +83,7 @@ while [ $sec_sz -le 4096 ]; do
 done
 
 for SECTOR_SIZE in $sector_sizes; do
-	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV > /dev/null
+	$MKFS_XFS_PROG -f -m "$crc_finobt_enabled" -s size=$SECTOR_SIZE $SCRATCH_DEV > /dev/null
 
 	for TYPE in agf agi agfl sb; do
 		DADDR=`_scratch_xfs_db -c "$TYPE" -c "daddr" | filter_dbval`
@@ -96,9 +102,11 @@ for SECTOR_SIZE in $sector_sizes; do
 	DADDR=`_scratch_xfs_db -c "agi" -c "addr root" -c "daddr" |
 		filter_dbval`
 	_scratch_xfs_db -c "daddr $DADDR" -c "type inobt"
-	DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
-		filter_dbval`
-	_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
+	if [[ "$crc_finobt_enabled" =~ 'finobt=1' ]]; then
+		DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
+			filter_dbval`
+		_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
+	fi
 
 	_scratch_xfs_db -c "daddr $DADDR" -c "type text"
 	_scratch_xfs_db -c "daddr $DADDR" -c "type data"
-- 
1.8.3.1

Re: [PATCH] xfs/424: add check for crc and finobt

[Eric Sandeen]

On 8/3/17 7:05 AM, Xiao Yang wrote:
> 1) When crc is not supported or disabled, this case could not indicate
>    any expected error.  Running all tests without crc makes no sense.

It's still fine to run the test.  The description indicates
that errors may be seen on crc filesystems, but there's nothing
wrong with a test which iterates over "type" commands in xfs_db,
even without crcs.  It's possible that some other error could
be found in the future.

So perhaps the description should be updated; it does not "check for
that false crc error," it simply checks that setting types causes
/no/ error.
 
> 2) When finobt is disabled, xfs_db fails to get current address of
>    free_root, as below:
>    xfs_db -c "agi" -c "addr free_root" -c "daddr" /dev/sda11
>    Metadata CRC error detected at xfs_inobt block 0x0/0x1000
>    ...

I agree that the test should not be trying to set types that do
not exist on the filesystem.

But I think the right approach is to let the test run over whatever
MKFS_OPTIONS have been specified, and be sure that the test can handle
it properly, rather than forcing the test to run only with a certain
set of options enabled.

Thanks,
-Eric

>    Running related tests without finobt makes no sense.
> 
> We add check to fix it.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  tests/xfs/424 | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/xfs/424 b/tests/xfs/424
> index 0a1eef9..165c390 100755
> --- a/tests/xfs/424
> +++ b/tests/xfs/424
> @@ -55,9 +55,15 @@ rm -f $seqres.full
>  _supported_os Linux
>  _supported_fs xfs
>  _require_scratch
> +_require_xfs_mkfs_crc
>  
>  echo "Silence is golden."
>  
> +crc_finobt_enabled="crc=1,finobt=0"
> +
> +[ _scratch_mkfs_xfs_supported -m crc=1,finobt=1 >/dev/null 2>&1 ] && \
> +	crc_finobt_enabled="crc=1,finobt=1"
> +
>  # real QA test starts here
>  
>  # for different sector sizes, ensure no CRC errors are falsely reported.
> @@ -77,7 +83,7 @@ while [ $sec_sz -le 4096 ]; do
>  done
>  
>  for SECTOR_SIZE in $sector_sizes; do
> -	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV > /dev/null
> +	$MKFS_XFS_PROG -f -m "$crc_finobt_enabled" -s size=$SECTOR_SIZE $SCRATCH_DEV > /dev/null
>  
>  	for TYPE in agf agi agfl sb; do
>  		DADDR=`_scratch_xfs_db -c "$TYPE" -c "daddr" | filter_dbval`
> @@ -96,9 +102,11 @@ for SECTOR_SIZE in $sector_sizes; do
>  	DADDR=`_scratch_xfs_db -c "agi" -c "addr root" -c "daddr" |
>  		filter_dbval`
>  	_scratch_xfs_db -c "daddr $DADDR" -c "type inobt"
> -	DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
> -		filter_dbval`
> -	_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
> +	if [[ "$crc_finobt_enabled" =~ 'finobt=1' ]]; then
> +		DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
> +			filter_dbval`
> +		_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
> +	fi
>  
>  	_scratch_xfs_db -c "daddr $DADDR" -c "type text"
>  	_scratch_xfs_db -c "daddr $DADDR" -c "type data"
> 

Re: [PATCH] xfs/424: add check for crc and finobt

[Xiao Yang]

On 2017/08/03 22:57, Eric Sandeen wrote:
> On 8/3/17 7:05 AM, Xiao Yang wrote:
>> 1) When crc is not supported or disabled, this case could not indicate
>>     any expected error.  Running all tests without crc makes no sense.
> It's still fine to run the test.  The description indicates
> that errors may be seen on crc filesystems, but there's nothing
> wrong with a test which iterates over "type" commands in xfs_db,
> even without crcs.  It's possible that some other error could
> be found in the future.
>
> So perhaps the description should be updated; it does not "check for
> that false crc error," it simply checks that setting types causes
> /no/ error.
Hi Eric,

Agreed.  We can just update the description.
>
>> 2) When finobt is disabled, xfs_db fails to get current address of
>>     free_root, as below:
>>     xfs_db -c "agi" -c "addr free_root" -c "daddr" /dev/sda11
>>     Metadata CRC error detected at xfs_inobt block 0x0/0x1000
>>     ...
> I agree that the test should not be trying to set types that do
> not exist on the filesystem.
>
> But I think the right approach is to let the test run over whatever
> MKFS_OPTIONS have been specified, and be sure that the test can handle
> it properly, rather than forcing the test to run only with a certain
> set of options enabled.
OK， i will change the check for finobt as you suggested.
Thanks a lot for your comments.

Thanks,
Xiao Yang.
> Thanks,
> -Eric
>
>>     Running related tests without finobt makes no sense.
>>
>> We add check to fix it.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>>   tests/xfs/424 | 16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/xfs/424 b/tests/xfs/424
>> index 0a1eef9..165c390 100755
>> --- a/tests/xfs/424
>> +++ b/tests/xfs/424
>> @@ -55,9 +55,15 @@ rm -f $seqres.full
>>   _supported_os Linux
>>   _supported_fs xfs
>>   _require_scratch
>> +_require_xfs_mkfs_crc
>>
>>   echo "Silence is golden."
>>
>> +crc_finobt_enabled="crc=1,finobt=0"
>> +
>> +[ _scratch_mkfs_xfs_supported -m crc=1,finobt=1>/dev/null 2>&1 ]&&  \
>> +	crc_finobt_enabled="crc=1,finobt=1"
>> +
>>   # real QA test starts here
>>
>>   # for different sector sizes, ensure no CRC errors are falsely reported.
>> @@ -77,7 +83,7 @@ while [ $sec_sz -le 4096 ]; do
>>   done
>>
>>   for SECTOR_SIZE in $sector_sizes; do
>> -	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV>  /dev/null
>> +	$MKFS_XFS_PROG -f -m "$crc_finobt_enabled" -s size=$SECTOR_SIZE $SCRATCH_DEV>  /dev/null
>>
>>   	for TYPE in agf agi agfl sb; do
>>   		DADDR=`_scratch_xfs_db -c "$TYPE" -c "daddr" | filter_dbval`
>> @@ -96,9 +102,11 @@ for SECTOR_SIZE in $sector_sizes; do
>>   	DADDR=`_scratch_xfs_db -c "agi" -c "addr root" -c "daddr" |
>>   		filter_dbval`
>>   	_scratch_xfs_db -c "daddr $DADDR" -c "type inobt"
>> -	DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
>> -		filter_dbval`
>> -	_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
>> +	if [[ "$crc_finobt_enabled" =~ 'finobt=1' ]]; then
>> +		DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
>> +			filter_dbval`
>> +		_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
>> +	fi
>>
>>   	_scratch_xfs_db -c "daddr $DADDR" -c "type text"
>>   	_scratch_xfs_db -c "daddr $DADDR" -c "type data"
>>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> .
>

[PATCH v2] xfs/424: add check for finobt && update comments

[Xiao Yang]

1) This test can check if setting types causes error regardless of
   supporting crc, so we can update existed comments about it.  We
   also add new comments about known issues triggered in this test.

2) When finobt is disabled, xfs_db fails to get current address of
   free_root, as below:
   xfs_db -c "agi" -c "addr free_root" -c "daddr" /dev/sda11
   Metadata CRC error detected at xfs_inobt block 0x0/0x1000
   ...
   Running related tests without finobt makes no sense, so we add
   check for finobt.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 tests/xfs/424 | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/tests/xfs/424 b/tests/xfs/424
index 0a1eef9..41180d8 100755
--- a/tests/xfs/424
+++ b/tests/xfs/424
@@ -1,11 +1,19 @@
 #! /bin/bash
 # FS QA Test 424
 #
-# xfs_db should take type size into account when setting type.
-# If type size isn't updated whenever type is set, a false crc
-# error can occur due to the stale size. This test checks for
-# that false crc error.
+# This case checks if setting type causes error.
 #
+# On crc filesystems, xfs_db doesn't take sector size into account
+# when setting type, and this can result in an errant crc.
+# This issue has been fixed in xfsprogs-dev:
+# '55f224b ("xfs_db: update buffer size when new type is set")'
+#
+# On crc filesystems, when setting the type to "inode" the verifier
+# validates multiple inodes in the current fs block, so setting the
+# buffer size to that of just one inode is not sufficient and it'll
+# emit spurious verifier errors for all but the first.
+# This issue has been fixed in xfsprogs-dev:
+# '533d1d2 ("xfs_db: properly set inode type")'
 #-----------------------------------------------------------------------
 # Copyright (c) 2017 Red Hat, Inc.  All Rights Reserved.
 #
@@ -77,7 +85,9 @@ while [ $sec_sz -le 4096 ]; do
 done
 
 for SECTOR_SIZE in $sector_sizes; do
-	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV > /dev/null
+	finobt_enabled=0
+	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV | \
+		grep -q 'finobt=1' && finobt_enabled=1
 
 	for TYPE in agf agi agfl sb; do
 		DADDR=`_scratch_xfs_db -c "$TYPE" -c "daddr" | filter_dbval`
@@ -96,9 +106,11 @@ for SECTOR_SIZE in $sector_sizes; do
 	DADDR=`_scratch_xfs_db -c "agi" -c "addr root" -c "daddr" |
 		filter_dbval`
 	_scratch_xfs_db -c "daddr $DADDR" -c "type inobt"
-	DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
-		filter_dbval`
-	_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
+	if [ $finobt_enabled -eq 1 ]; then
+		DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
+			filter_dbval`
+		_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
+	fi
 
 	_scratch_xfs_db -c "daddr $DADDR" -c "type text"
 	_scratch_xfs_db -c "daddr $DADDR" -c "type data"
-- 
1.8.3.1

Re: [PATCH] xfs/424: add check for crc and finobt

[Xiao Yang]

Hi Eric,

When finobt is disabled, xfs_db gets double crc errors on upstream xfsporgs-dev.
as below(e.g. block size is 4096):
...
xfs_db -c "daddr 16" -c "type finobt" /dev/sda11
Metadata corruption detected at xfs_inobt block 0x10/0x1000
Metadata corruption detected at xfs_inobt block 0x10/0x1000

Is this expected behavor?  Because xfs_db gets one crc error on v4.5 xfsprogs-dev.
as below(e.g. block size is 4096):
...
xfs_db -c "daddr 16" -c "type finobt" /dev/sda11
Metadata CRC error detected at xfs_inobt block 0x10/0x200

Thanks,
Xiao Yang.
On 2017/08/03 22:57, Eric Sandeen wrote:
> On 8/3/17 7:05 AM, Xiao Yang wrote:
>> 1) When crc is not supported or disabled, this case could not indicate
>>     any expected error.  Running all tests without crc makes no sense.
> It's still fine to run the test.  The description indicates
> that errors may be seen on crc filesystems, but there's nothing
> wrong with a test which iterates over "type" commands in xfs_db,
> even without crcs.  It's possible that some other error could
> be found in the future.
>
> So perhaps the description should be updated; it does not "check for
> that false crc error," it simply checks that setting types causes
> /no/ error.
>
>> 2) When finobt is disabled, xfs_db fails to get current address of
>>     free_root, as below:
>>     xfs_db -c "agi" -c "addr free_root" -c "daddr" /dev/sda11
>>     Metadata CRC error detected at xfs_inobt block 0x0/0x1000
>>     ...
> I agree that the test should not be trying to set types that do
> not exist on the filesystem.
>
> But I think the right approach is to let the test run over whatever
> MKFS_OPTIONS have been specified, and be sure that the test can handle
> it properly, rather than forcing the test to run only with a certain
> set of options enabled.
>
> Thanks,
> -Eric
>
>>     Running related tests without finobt makes no sense.
>>
>> We add check to fix it.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
>>   tests/xfs/424 | 16 ++++++++++++----
>>   1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/xfs/424 b/tests/xfs/424
>> index 0a1eef9..165c390 100755
>> --- a/tests/xfs/424
>> +++ b/tests/xfs/424
>> @@ -55,9 +55,15 @@ rm -f $seqres.full
>>   _supported_os Linux
>>   _supported_fs xfs
>>   _require_scratch
>> +_require_xfs_mkfs_crc
>>
>>   echo "Silence is golden."
>>
>> +crc_finobt_enabled="crc=1,finobt=0"
>> +
>> +[ _scratch_mkfs_xfs_supported -m crc=1,finobt=1>/dev/null 2>&1 ]&&  \
>> +	crc_finobt_enabled="crc=1,finobt=1"
>> +
>>   # real QA test starts here
>>
>>   # for different sector sizes, ensure no CRC errors are falsely reported.
>> @@ -77,7 +83,7 @@ while [ $sec_sz -le 4096 ]; do
>>   done
>>
>>   for SECTOR_SIZE in $sector_sizes; do
>> -	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV>  /dev/null
>> +	$MKFS_XFS_PROG -f -m "$crc_finobt_enabled" -s size=$SECTOR_SIZE $SCRATCH_DEV>  /dev/null
>>
>>   	for TYPE in agf agi agfl sb; do
>>   		DADDR=`_scratch_xfs_db -c "$TYPE" -c "daddr" | filter_dbval`
>> @@ -96,9 +102,11 @@ for SECTOR_SIZE in $sector_sizes; do
>>   	DADDR=`_scratch_xfs_db -c "agi" -c "addr root" -c "daddr" |
>>   		filter_dbval`
>>   	_scratch_xfs_db -c "daddr $DADDR" -c "type inobt"
>> -	DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
>> -		filter_dbval`
>> -	_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
>> +	if [[ "$crc_finobt_enabled" =~ 'finobt=1' ]]; then
>> +		DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
>> +			filter_dbval`
>> +		_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
>> +	fi
>>
>>   	_scratch_xfs_db -c "daddr $DADDR" -c "type text"
>>   	_scratch_xfs_db -c "daddr $DADDR" -c "type data"
>>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
> .
>

Re: [PATCH] xfs/424: add check for crc and finobt

[Eric Sandeen]

On 8/4/17 2:20 AM, Xiao Yang wrote:
> Hi Eric,
> 
> When finobt is disabled, xfs_db gets double crc errors on upstream xfsporgs-dev.
> as below(e.g. block size is 4096):
> ...
> xfs_db -c "daddr 16" -c "type finobt" /dev/sda11
> Metadata corruption detected at xfs_inobt block 0x10/0x1000
> Metadata corruption detected at xfs_inobt block 0x10/0x1000
> 
> Is this expected behavor?  Because xfs_db gets one crc error on v4.5 xfsprogs-dev.
> as below(e.g. block size is 4096):
> ...
> xfs_db -c "daddr 16" -c "type finobt" /dev/sda11
> Metadata CRC error detected at xfs_inobt block 0x10/0x200

So, errors is expected because you finobt is disabled, and you are asking
it to print a not-finobt block as a finobt.

I think the double printing is because upstream the code actually sets
the cursor twice after the commit which tried to fix the type-setting,
it's on my list to fix.

Thanks,
-Eric

> Thanks,
> Xiao Yang.

Re: [PATCH] xfs/424: add check for crc and finobt

[Xiao Yang]

On 2017/08/04 22:37, Eric Sandeen wrote:
> On 8/4/17 2:20 AM, Xiao Yang wrote:
>> Hi Eric,
>>
>> When finobt is disabled, xfs_db gets double crc errors on upstream xfsporgs-dev.
>> as below(e.g. block size is 4096):
>> ...
>> xfs_db -c "daddr 16" -c "type finobt" /dev/sda11
>> Metadata corruption detected at xfs_inobt block 0x10/0x1000
>> Metadata corruption detected at xfs_inobt block 0x10/0x1000
>>
>> Is this expected behavor?  Because xfs_db gets one crc error on v4.5 xfsprogs-dev.
>> as below(e.g. block size is 4096):
>> ...
>> xfs_db -c "daddr 16" -c "type finobt" /dev/sda11
>> Metadata CRC error detected at xfs_inobt block 0x10/0x200
> So, errors is expected because you finobt is disabled, and you are asking
> it to print a not-finobt block as a finobt.
>
> I think the double printing is because upstream the code actually sets
> the cursor twice after the commit which tried to fix the type-setting,
> it's on my list to fix.
Hi Eric,

Thanks for your detailed explanation, and i got it.
Could you help me review the v2 patch.  Thanks a lot!

Thanks,
Xiao Yang
> Thanks,
> -Eric
>
>> Thanks,
>> Xiao Yang.
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

Re: [PATCH v2] xfs/424: add check for finobt && update comments

[Xiao Yang]

Hi,

Could you help me review the v2 patch. Thanks a lot!

Thanks,
Xiao Yang.
On 2017/08/04 15:08, Xiao Yang wrote:
> 1) This test can check if setting types causes error regardless of
>    supporting crc, so we can update existed comments about it.  We
>    also add new comments about known issues triggered in this test.
>
> 2) When finobt is disabled, xfs_db fails to get current address of
>    free_root, as below:
>    xfs_db -c "agi" -c "addr free_root" -c "daddr" /dev/sda11
>    Metadata CRC error detected at xfs_inobt block 0x0/0x1000
>    ...
>    Running related tests without finobt makes no sense, so we add
>    check for finobt.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  tests/xfs/424 | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/tests/xfs/424 b/tests/xfs/424
> index 0a1eef9..41180d8 100755
> --- a/tests/xfs/424
> +++ b/tests/xfs/424
> @@ -1,11 +1,19 @@
>  #! /bin/bash
>  # FS QA Test 424
>  #
> -# xfs_db should take type size into account when setting type.
> -# If type size isn't updated whenever type is set, a false crc
> -# error can occur due to the stale size. This test checks for
> -# that false crc error.
> +# This case checks if setting type causes error.
>  #
> +# On crc filesystems, xfs_db doesn't take sector size into account
> +# when setting type, and this can result in an errant crc.
> +# This issue has been fixed in xfsprogs-dev:
> +# '55f224b ("xfs_db: update buffer size when new type is set")'
> +#
> +# On crc filesystems, when setting the type to "inode" the verifier
> +# validates multiple inodes in the current fs block, so setting the
> +# buffer size to that of just one inode is not sufficient and it'll
> +# emit spurious verifier errors for all but the first.
> +# This issue has been fixed in xfsprogs-dev:
> +# '533d1d2 ("xfs_db: properly set inode type")'
>  #-----------------------------------------------------------------------
>  # Copyright (c) 2017 Red Hat, Inc.  All Rights Reserved.
>  #
> @@ -77,7 +85,9 @@ while [ $sec_sz -le 4096 ]; do
>  done
>  
>  for SECTOR_SIZE in $sector_sizes; do
> -	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV > /dev/null
> +	finobt_enabled=0
> +	$MKFS_XFS_PROG -f -s size=$SECTOR_SIZE $SCRATCH_DEV | \
> +		grep -q 'finobt=1' && finobt_enabled=1
>  
>  	for TYPE in agf agi agfl sb; do
>  		DADDR=`_scratch_xfs_db -c "$TYPE" -c "daddr" | filter_dbval`
> @@ -96,9 +106,11 @@ for SECTOR_SIZE in $sector_sizes; do
>  	DADDR=`_scratch_xfs_db -c "agi" -c "addr root" -c "daddr" |
>  		filter_dbval`
>  	_scratch_xfs_db -c "daddr $DADDR" -c "type inobt"
> -	DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
> -		filter_dbval`
> -	_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
> +	if [ $finobt_enabled -eq 1 ]; then
> +		DADDR=`_scratch_xfs_db -c "agi" -c "addr free_root" -c "daddr" |
> +			filter_dbval`
> +		_scratch_xfs_db -c "daddr $DADDR" -c "type finobt"
> +	fi
>  
>  	_scratch_xfs_db -c "daddr $DADDR" -c "type text"
>  	_scratch_xfs_db -c "daddr $DADDR" -c "type data"

Re: [PATCH v2] xfs/424: add check for finobt && update comments

[Eryu Guan]

On Wed, Aug 16, 2017 at 09:54:21AM +0800, Xiao Yang wrote:
> Hi,
> 
> Could you help me review the v2 patch. Thanks a lot!

Patch looks ok to me, I've queued it for next update, unless Eric has
more comments before Friday :)

Thanks,
Eryu