Re: [PATCH] xfs/097: Remove wrong broken assignment operation

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Tue, Oct 08, 2019 at 10:39:59AM +0800, Yang Xu wrote:
> 
> 
> on 2019/10/07 23:12, Darrick J. Wong wrote:
> > On Mon, Oct 07, 2019 at 03:15:15PM +0800, Yang Xu wrote:
> > > On old kernel, since commit ded188b8609 ("xfs: Fix the situation that mount
> > > operation rejects corrupted XFS") running this case got the mismatched output,
> > > as below:
> > 
> > But why did the output mismatch?  Did the fs heal itself?  Did
> > allocating 5 more files somehow avoid touching the finobt?  Is the
> > assignment logic in the loop broken?
> 
> The output mismatch because on old kernel, we can mount the corrupted xfs
> and touch action will be refused. so broken is equal to 0.
> The fs doesn't heal ifself.
> allocating 5 more file will touch the finobt.
> 
> You can see this url
> https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=ded188b86096e2845e59dedae6050c7f254a96b
> 
> eg xfs/087, they all delete "broken=0" before allocationg 5 more file.
> commit ded188b86 compatibled old kernel(permit mount and refuse touch) and
> new kernel(refuse mount) behavior on corrupted xfs.  Or, I misunderstand
> this case?

How old is the kernel?  At some point (4.10, I think?) we added a patch
to reserve metadata blocks for future free inode btree expansion.  That
required us to count the blocks in the finobt, at which point xfs/097's
behavior changed such that the fs doesn't mount after the test corrupts
the finobt.

--D

> > 
> > --D
> > 
> > > -----------------------------------
> > >   + check fs
> > >   + corrupt image
> > >   + mount image && modify files
> > > -broken: 1
> > > +broken: 0
> > >   + repair fs
> > >   + mount image (2)
> > > ------------------------------------
> > > 
> > > It fails because the broken is always equal to 0 when _try_scratch_mount
> > > succeed. So remove this wrong assignment operation.
> > > 
> > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> > > ---
> > >   tests/xfs/097 | 2 --
> > >   1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/tests/xfs/097 b/tests/xfs/097
> > > index 1cb7d69c..20791738 100755
> > > --- a/tests/xfs/097
> > > +++ b/tests/xfs/097
> > > @@ -81,8 +81,6 @@ done
> > >   echo "+ mount image && modify files"
> > >   broken=1
> > >   if _try_scratch_mount >> $seqres.full 2>&1; then
> > > -
> > > -	broken=0
> > >   	for x in `seq 65 70`; do
> > >   		touch "${TESTFILE}.${x}" 2> /dev/null && broken=0
> > >   	done
> > > -- 
> > > 2.18.1
> > > 
> > > 
> > > 
> > 
> > 
> 
>