xfs/111: Filter a "Structure needs cleaning" error from xfs_fsr
diff mbox series

Message ID 1543298175-5405-1-git-send-email-yangx.jy@cn.fujitsu.com
State New
Headers show
Series
  • xfs/111: Filter a "Structure needs cleaning" error from xfs_fsr
Related show

Commit Message

Xiao Yang Nov. 27, 2018, 5:56 a.m. UTC
on upstream kernel, running xfs/111 got the following error:
-------------------------------------------------
+xfs_fsr: xfs_bulkstat: Structure needs cleaning
-------------------------------------------------

Since kernel commit 29cad0b, we don't convert a -EFSCORRUPTED error
to -EINVAL in xfs_imap_to_bp() if we are doing an untrusted lookup.
So xfs_bulkstat() called by xfs_fsr command fails and returns
EFSCORRUPTED.  We think it is reasonable for xfs_fsr to report
EFSCORRUPTED because some inodes are actually corrupted.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 tests/xfs/111 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Xiao Yang Nov. 27, 2018, 6:43 a.m. UTC | #1
Hi Dave,

I am not familiar with the bulkstat code in kernel, so i am not sure
if the fix patch is OK. Could you help me review it? Thanks a lot.

Best Regards,
Xiao Yang
On 2018/11/27 13:56, Xiao Yang wrote:
> on upstream kernel, running xfs/111 got the following error:
> -------------------------------------------------
> +xfs_fsr: xfs_bulkstat: Structure needs cleaning
> -------------------------------------------------
>
> Since kernel commit 29cad0b, we don't convert a -EFSCORRUPTED error
> to -EINVAL in xfs_imap_to_bp() if we are doing an untrusted lookup.
> So xfs_bulkstat() called by xfs_fsr command fails and returns
> EFSCORRUPTED.  We think it is reasonable for xfs_fsr to report
> EFSCORRUPTED because some inodes are actually corrupted.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  tests/xfs/111 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tests/xfs/111 b/tests/xfs/111
> index 2ef69f0..2bcf002 100755
> --- a/tests/xfs/111
> +++ b/tests/xfs/111
> @@ -58,7 +58,8 @@ _try_scratch_mount || _fail "Couldn't mount after itrash"
>  
>  echo Attempting bulkstat
>  #src/bstat -q $SCRATCH_MNT
> -$XFS_FSR_PROG $SCRATCH_MNT | _filter_scratch
> +#Since kernel commit 29cad0b, xfs_fsr reports a "Structure needs cleaning" error
> +$XFS_FSR_PROG $SCRATCH_MNT | _filter_scratch | grep -v "Structure needs cleaning"
>  
>  # Fix up intentional corruption so test can pass
>  _scratch_unmount
Dave Chinner Nov. 29, 2018, 2 a.m. UTC | #2
On Tue, Nov 27, 2018 at 01:56:15PM +0800, Xiao Yang wrote:
> on upstream kernel, running xfs/111 got the following error:
> -------------------------------------------------
> +xfs_fsr: xfs_bulkstat: Structure needs cleaning
> -------------------------------------------------
> 
> Since kernel commit 29cad0b, we don't convert a -EFSCORRUPTED error
> to -EINVAL in xfs_imap_to_bp() if we are doing an untrusted lookup.
> So xfs_bulkstat() called by xfs_fsr command fails and returns
> EFSCORRUPTED.  We think it is reasonable for xfs_fsr to report
> EFSCORRUPTED because some inodes are actually corrupted.
> 
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---
>  tests/xfs/111 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/xfs/111 b/tests/xfs/111
> index 2ef69f0..2bcf002 100755
> --- a/tests/xfs/111
> +++ b/tests/xfs/111
> @@ -58,7 +58,8 @@ _try_scratch_mount || _fail "Couldn't mount after itrash"
>  
>  echo Attempting bulkstat
>  #src/bstat -q $SCRATCH_MNT
> -$XFS_FSR_PROG $SCRATCH_MNT | _filter_scratch
> +#Since kernel commit 29cad0b, xfs_fsr reports a "Structure needs cleaning" error
> +$XFS_FSR_PROG $SCRATCH_MNT | _filter_scratch | grep -v "Structure needs cleaning"

So the test tripped over a filesystem corruption and it didn't
before? Regressions like this should be reported in full to the xfs
list when they are discovered, not silenced by the test.

We may decide that the test needs changing, but reporting the
regression to the upstream developers should be done first.

Cheers,

Dave.

Patch
diff mbox series

diff --git a/tests/xfs/111 b/tests/xfs/111
index 2ef69f0..2bcf002 100755
--- a/tests/xfs/111
+++ b/tests/xfs/111
@@ -58,7 +58,8 @@  _try_scratch_mount || _fail "Couldn't mount after itrash"
 
 echo Attempting bulkstat
 #src/bstat -q $SCRATCH_MNT
-$XFS_FSR_PROG $SCRATCH_MNT | _filter_scratch
+#Since kernel commit 29cad0b, xfs_fsr reports a "Structure needs cleaning" error
+$XFS_FSR_PROG $SCRATCH_MNT | _filter_scratch | grep -v "Structure needs cleaning"
 
 # Fix up intentional corruption so test can pass
 _scratch_unmount