diff mbox series

[v3] tests/xfs: check available blocks after log recovery on ro mount

Message ID 20210824050436.3l5jodgjhwt7wqzl@xzhoux.usersys.redhat.com (mailing list archive)
State New, archived
Headers show
Series [v3] tests/xfs: check available blocks after log recovery on ro mount | expand

Commit Message

Murphy Zhou Aug. 24, 2021, 5:04 a.m. UTC
And followed by a rw mount.

Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---

Thanks Darrick very much for reviewing!

v2:
   Add explaination of the issue
   add xfs_force_bdev data $SCRATCH_MNT
   use DF_PROG
   Re numbered this test
v3:
   Add _require_scratch_shutdown
   Use _get_available_space
   Explain why does not use _scratch_mount

 tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/999.out |  2 ++
 2 files changed, 67 insertions(+)
 create mode 100755 tests/xfs/999
 create mode 100644 tests/xfs/999.out

Comments

Eryu Guan Aug. 24, 2021, 5:42 a.m. UTC | #1
On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> And followed by a rw mount.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
> 
> Thanks Darrick very much for reviewing!
> 
> v2:
>    Add explaination of the issue
>    add xfs_force_bdev data $SCRATCH_MNT
>    use DF_PROG
>    Re numbered this test
> v3:
>    Add _require_scratch_shutdown
>    Use _get_available_space
>    Explain why does not use _scratch_mount
> 
>  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 67 insertions(+)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> diff --git a/tests/xfs/999 b/tests/xfs/999
> new file mode 100755
> index 00000000..0ce9989b
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,65 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 RedHat All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# Testcase for kernel commit:
> +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> +#
> +# After shutdown and readonly mount, a following read-write mount would
> +# get wrong number of available blocks. This is caused by unmounting the log
> +# on a readonly filesystem doesn't log the sb counters.
> +#
> +. ./common/preamble
> +_begin_fstest shutdown auto quick

Is this also a 'recoveryloop' test that excrises shutdown & log
recovery?

> +
> +# real QA test starts here
> +
> +_supported_fs xfs
> +_require_scratch
> +_require_scratch_shutdown
> +
> +_scratch_mkfs > $seqres.full 2>&1
> +# Don't use _scratch_mount because we need to mount without SELinux context
> +# to reproduce this issue. If we mount with SELinux context, this testcase
> +# is not reproducing the original issue.
> +mount $SCRATCH_DEV $SCRATCH_MNT

$MOUNT_PROG

> +_xfs_force_bdev data $SCRATCH_MNT
> +
> +# Write test file
> +ls > $SCRATCH_MNT/testfile

I'd like write something into file explicitly, the output of ls command
may change in subtle way, e.g. iif we decide to run fstests with an
empty dir as $PWD in the future, then we write nothing into the file.

> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> +
> +# Shutdown
> +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT

Does '_scratch_shutdown -f' work here?

> +
> +# Mount ReadOnly
> +_scratch_unmount
> +_scratch_mount -oro

_scratch_cycle_mount ro

> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> +# Umount and mount rw
> +_scratch_unmount
> +_scratch_mount

_scratch_cycle_mount

> +
> +# Get fdblocks before repair
> +fdb1=$(_get_available_space $SCRATCH_MNT)
> +_scratch_unmount
> +
> +# Repair

I find these "Mount ReadOnly" "Umount and mount rw" "Repair" comments
don't really help, they just describe what we're doing (which are
obvious in this test) not why we're doing it. It's better to describe
the reason of doing this repair.

Thanks,
Eryu

> +_repair_scratch_fs >> $seqres.full 2>&1
> +
> +# Re-mount
> +_scratch_mount
> +
> +# Get fdblocks after repair
> +fdb2=$(_get_available_space $SCRATCH_MNT)
> +
> +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> +
> +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.20.1
Zorro Lang Aug. 24, 2021, 6:23 a.m. UTC | #2
On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> And followed by a rw mount.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
> 
> Thanks Darrick very much for reviewing!
> 
> v2:
>    Add explaination of the issue
>    add xfs_force_bdev data $SCRATCH_MNT
>    use DF_PROG
>    Re numbered this test
> v3:
>    Add _require_scratch_shutdown
>    Use _get_available_space
>    Explain why does not use _scratch_mount
> 
>  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 67 insertions(+)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> diff --git a/tests/xfs/999 b/tests/xfs/999
> new file mode 100755
> index 00000000..0ce9989b
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,65 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 RedHat All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# Testcase for kernel commit:
> +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> +#
> +# After shutdown and readonly mount, a following read-write mount would
> +# get wrong number of available blocks. This is caused by unmounting the log
> +# on a readonly filesystem doesn't log the sb counters.
> +#
> +. ./common/preamble
> +_begin_fstest shutdown auto quick
> +
> +# real QA test starts here
> +
> +_supported_fs xfs

I'm wondering what limits this test to be a xfs only test? The test steps looks
common, right?

Thanks,
Zorro

> +_require_scratch
> +_require_scratch_shutdown
> +
> +_scratch_mkfs > $seqres.full 2>&1
> +# Don't use _scratch_mount because we need to mount without SELinux context
> +# to reproduce this issue. If we mount with SELinux context, this testcase
> +# is not reproducing the original issue.
> +mount $SCRATCH_DEV $SCRATCH_MNT
> +_xfs_force_bdev data $SCRATCH_MNT
> +
> +# Write test file
> +ls > $SCRATCH_MNT/testfile
> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> +
> +# Shutdown
> +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> +
> +# Mount ReadOnly
> +_scratch_unmount
> +_scratch_mount -oro
> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> +# Umount and mount rw
> +_scratch_unmount
> +_scratch_mount
> +
> +# Get fdblocks before repair
> +fdb1=$(_get_available_space $SCRATCH_MNT)
> +_scratch_unmount
> +
> +# Repair
> +_repair_scratch_fs >> $seqres.full 2>&1
> +
> +# Re-mount
> +_scratch_mount
> +
> +# Get fdblocks after repair
> +fdb2=$(_get_available_space $SCRATCH_MNT)
> +
> +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> +
> +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.20.1
>
Darrick J. Wong Aug. 24, 2021, 3:14 p.m. UTC | #3
On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> And followed by a rw mount.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
> 
> Thanks Darrick very much for reviewing!
> 
> v2:
>    Add explaination of the issue
>    add xfs_force_bdev data $SCRATCH_MNT
>    use DF_PROG
>    Re numbered this test
> v3:
>    Add _require_scratch_shutdown
>    Use _get_available_space
>    Explain why does not use _scratch_mount
> 
>  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 67 insertions(+)
>  create mode 100755 tests/xfs/999
>  create mode 100644 tests/xfs/999.out
> 
> diff --git a/tests/xfs/999 b/tests/xfs/999
> new file mode 100755
> index 00000000..0ce9989b
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,65 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 RedHat All Rights Reserved.
> +#
> +# FS QA Test 999
> +#
> +# Testcase for kernel commit:
> +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> +#
> +# After shutdown and readonly mount, a following read-write mount would
> +# get wrong number of available blocks. This is caused by unmounting the log
> +# on a readonly filesystem doesn't log the sb counters.
> +#
> +. ./common/preamble
> +_begin_fstest shutdown auto quick
> +
> +# real QA test starts here
> +
> +_supported_fs xfs
> +_require_scratch
> +_require_scratch_shutdown
> +
> +_scratch_mkfs > $seqres.full 2>&1
> +# Don't use _scratch_mount because we need to mount without SELinux context
> +# to reproduce this issue. If we mount with SELinux context, this testcase
> +# is not reproducing the original issue.
> +mount $SCRATCH_DEV $SCRATCH_MNT

This mount will fail if the test runner configured an external log or a
realtime device, because you didn't specify those devices to the mount
call.  Either this needs _require_nonexternal or $(_scratch_options
mount) needs to be injected into the command line.

--D

> +_xfs_force_bdev data $SCRATCH_MNT
> +
> +# Write test file
> +ls > $SCRATCH_MNT/testfile
> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> +
> +# Shutdown
> +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> +
> +# Mount ReadOnly
> +_scratch_unmount
> +_scratch_mount -oro
> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> +# Umount and mount rw
> +_scratch_unmount
> +_scratch_mount
> +
> +# Get fdblocks before repair
> +fdb1=$(_get_available_space $SCRATCH_MNT)
> +_scratch_unmount
> +
> +# Repair
> +_repair_scratch_fs >> $seqres.full 2>&1
> +
> +# Re-mount
> +_scratch_mount
> +
> +# Get fdblocks after repair
> +fdb2=$(_get_available_space $SCRATCH_MNT)
> +
> +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> +
> +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/xfs/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.20.1
>
Zorro Lang Aug. 24, 2021, 4:53 p.m. UTC | #4
On Tue, Aug 24, 2021 at 08:14:28AM -0700, Darrick J. Wong wrote:
> On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> > And followed by a rw mount.
> > 
> > Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> > ---
> > 
> > Thanks Darrick very much for reviewing!
> > 
> > v2:
> >    Add explaination of the issue
> >    add xfs_force_bdev data $SCRATCH_MNT
> >    use DF_PROG
> >    Re numbered this test
> > v3:
> >    Add _require_scratch_shutdown
> >    Use _get_available_space
> >    Explain why does not use _scratch_mount
> > 
> >  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/xfs/999.out |  2 ++
> >  2 files changed, 67 insertions(+)
> >  create mode 100755 tests/xfs/999
> >  create mode 100644 tests/xfs/999.out
> > 
> > diff --git a/tests/xfs/999 b/tests/xfs/999
> > new file mode 100755
> > index 00000000..0ce9989b
> > --- /dev/null
> > +++ b/tests/xfs/999
> > @@ -0,0 +1,65 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2021 RedHat All Rights Reserved.
> > +#
> > +# FS QA Test 999
> > +#
> > +# Testcase for kernel commit:
> > +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> > +#
> > +# After shutdown and readonly mount, a following read-write mount would
> > +# get wrong number of available blocks. This is caused by unmounting the log
> > +# on a readonly filesystem doesn't log the sb counters.
> > +#
> > +. ./common/preamble
> > +_begin_fstest shutdown auto quick
> > +
> > +# real QA test starts here
> > +
> > +_supported_fs xfs
> > +_require_scratch
> > +_require_scratch_shutdown
> > +
> > +_scratch_mkfs > $seqres.full 2>&1
> > +# Don't use _scratch_mount because we need to mount without SELinux context
> > +# to reproduce this issue. If we mount with SELinux context, this testcase
> > +# is not reproducing the original issue.
> > +mount $SCRATCH_DEV $SCRATCH_MNT
> 
> This mount will fail if the test runner configured an external log or a
> realtime device, because you didn't specify those devices to the mount
> call.  Either this needs _require_nonexternal or $(_scratch_options
> mount) needs to be injected into the command line.

I think I know why Xiong said _scratch_mount can't reproduce this bug.

Before this patch v3, he tried to find the xfs corruption by using $DF_PROG $SCRATCH_MNT
before&after xfs_repair.

If we use _scratch_mount, new files we create won't contains SELinux label
in xfs inode attrfork. The corruption comes from sb_ifree (free inodes):
  Phase 2 - using internal log
        - zero log...
        - scan filesystem freespace and inode maps...
  sb_ifree 61, counted 60

But if we don't use _scratch_mount, the corruption comes from sb_fdblocks(free blocks):
  Phase 2 - using internal log
        - zero log...
        - scan filesystem freespace and inode maps...
  sb_fdblocks 130086290, counted 131007896

So if he use _scratch_mount, he can't get different available blocks by
$DF_PROG $SCRATCH_MNT, before&after xfs_repair. Due to the corrupted thing
is sb_ifree.

But I still can't understand one thing, even if do *not* use _scratch_mount, the attr(selinux label)
is local(short) format[1], didn't take more blocks. What takes more blocks, cause later
sb_fdblocks corruption?

Thanks,
Zorro

[1]
...
core.aformat = 1 (local)
...
u3 = (empty)
a.sfattr.hdr.totsize = 51
a.sfattr.hdr.count = 1
a.sfattr.list[0].namelen = 7
a.sfattr.list[0].valuelen = 37
a.sfattr.list[0].root = 0
a.sfattr.list[0].secure = 1
a.sfattr.list[0].name = "selinux"
a.sfattr.list[0].value = "unconfined_u:object_r:unlabeled_t:s0\000"


> 
> --D
> 
> > +_xfs_force_bdev data $SCRATCH_MNT
> > +
> > +# Write test file
> > +ls > $SCRATCH_MNT/testfile
> > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > +
> > +# Shutdown
> > +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> > +
> > +# Mount ReadOnly
> > +_scratch_unmount
> > +_scratch_mount -oro
> > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > +# Umount and mount rw
> > +_scratch_unmount
> > +_scratch_mount
> > +
> > +# Get fdblocks before repair
> > +fdb1=$(_get_available_space $SCRATCH_MNT)
> > +_scratch_unmount
> > +
> > +# Repair
> > +_repair_scratch_fs >> $seqres.full 2>&1
> > +
> > +# Re-mount
> > +_scratch_mount
> > +
> > +# Get fdblocks after repair
> > +fdb2=$(_get_available_space $SCRATCH_MNT)
> > +
> > +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> > +
> > +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> > +
> > +# success, all done
> > +echo "Silence is golden"
> > +status=0
> > +exit
> > diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> > new file mode 100644
> > index 00000000..3b276ca8
> > --- /dev/null
> > +++ b/tests/xfs/999.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 999
> > +Silence is golden
> > -- 
> > 2.20.1
> > 
>
Donald Douwsma Aug. 24, 2021, 11:22 p.m. UTC | #5
On 24/08/2021 16:23, Zorro Lang wrote:
> On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
>> And followed by a rw mount.
>>
>> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
>> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
>> ---
>>
>> Thanks Darrick very much for reviewing!
>>
>> v2:
>>    Add explaination of the issue
>>    add xfs_force_bdev data $SCRATCH_MNT
>>    use DF_PROG
>>    Re numbered this test
>> v3:
>>    Add _require_scratch_shutdown
>>    Use _get_available_space
>>    Explain why does not use _scratch_mount
>>
>>  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/999.out |  2 ++
>>  2 files changed, 67 insertions(+)
>>  create mode 100755 tests/xfs/999
>>  create mode 100644 tests/xfs/999.out
>>
>> diff --git a/tests/xfs/999 b/tests/xfs/999
>> new file mode 100755
>> index 00000000..0ce9989b
>> --- /dev/null
>> +++ b/tests/xfs/999
>> @@ -0,0 +1,65 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2021 RedHat All Rights Reserved.
>> +#
>> +# FS QA Test 999
>> +#
>> +# Testcase for kernel commit:
>> +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
>> +#
>> +# After shutdown and readonly mount, a following read-write mount would
>> +# get wrong number of available blocks. This is caused by unmounting the log
>> +# on a readonly filesystem doesn't log the sb counters.
>> +#
>> +. ./common/preamble
>> +_begin_fstest shutdown auto quick
>> +
>> +# real QA test starts here
>> +
>> +_supported_fs xfs
> 
> I'm wondering what limits this test to be a xfs only test? The test steps looks
> common, right?

It needs `shutdown -f` to force the log to disk, if there was a way to do that 
reliably for other filesystems then yes we could shutdown in a different way
and make it generic. 

Don

> 
>> +_require_scratch
>> +_require_scratch_shutdown
>> +
>> +_scratch_mkfs > $seqres.full 2>&1
>> +# Don't use _scratch_mount because we need to mount without SELinux context
>> +# to reproduce this issue. If we mount with SELinux context, this testcase
>> +# is not reproducing the original issue.
>> +mount $SCRATCH_DEV $SCRATCH_MNT
>> +_xfs_force_bdev data $SCRATCH_MNT
>> +
>> +# Write test file
>> +ls > $SCRATCH_MNT/testfile
>> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
>> +
>> +# Shutdown
>> +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
>> +
>> +# Mount ReadOnly
>> +_scratch_unmount
>> +_scratch_mount -oro
>> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
>> +# Umount and mount rw
>> +_scratch_unmount
>> +_scratch_mount
>> +
>> +# Get fdblocks before repair
>> +fdb1=$(_get_available_space $SCRATCH_MNT)
>> +_scratch_unmount
>> +
>> +# Repair
>> +_repair_scratch_fs >> $seqres.full 2>&1
>> +
>> +# Re-mount
>> +_scratch_mount
>> +
>> +# Get fdblocks after repair
>> +fdb2=$(_get_available_space $SCRATCH_MNT)
>> +
>> +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
>> +
>> +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
>> +
>> +# success, all done
>> +echo "Silence is golden"
>> +status=0
>> +exit
>> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
>> new file mode 100644
>> index 00000000..3b276ca8
>> --- /dev/null
>> +++ b/tests/xfs/999.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 999
>> +Silence is golden
>> -- 
>> 2.20.1
>>
>
Donald Douwsma Aug. 25, 2021, 1:06 a.m. UTC | #6
On 25/08/2021 09:22, Donald Douwsma wrote:
> 
> 
> On 24/08/2021 16:23, Zorro Lang wrote:
>> On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
>>> And followed by a rw mount.
>>>
>>> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
>>> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>>> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
>>> ---
>>>
>>> Thanks Darrick very much for reviewing!
>>>
>>> v2:
>>>    Add explaination of the issue
>>>    add xfs_force_bdev data $SCRATCH_MNT
>>>    use DF_PROG
>>>    Re numbered this test
>>> v3:
>>>    Add _require_scratch_shutdown
>>>    Use _get_available_space
>>>    Explain why does not use _scratch_mount
>>>
>>>  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  tests/xfs/999.out |  2 ++
>>>  2 files changed, 67 insertions(+)
>>>  create mode 100755 tests/xfs/999
>>>  create mode 100644 tests/xfs/999.out
>>>
>>> diff --git a/tests/xfs/999 b/tests/xfs/999
>>> new file mode 100755
>>> index 00000000..0ce9989b
>>> --- /dev/null
>>> +++ b/tests/xfs/999
>>> @@ -0,0 +1,65 @@
>>> +#! /bin/bash
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# Copyright (c) 2021 RedHat All Rights Reserved.
>>> +#
>>> +# FS QA Test 999
>>> +#
>>> +# Testcase for kernel commit:
>>> +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
>>> +#
>>> +# After shutdown and readonly mount, a following read-write mount would
>>> +# get wrong number of available blocks. This is caused by unmounting the log
>>> +# on a readonly filesystem doesn't log the sb counters.
>>> +#
>>> +. ./common/preamble
>>> +_begin_fstest shutdown auto quick
>>> +
>>> +# real QA test starts here
>>> +
>>> +_supported_fs xfs
>>
>> I'm wondering what limits this test to be a xfs only test? The test steps looks
>> common, right?
> 
> It needs `shutdown -f` to force the log to disk, if there was a way to do that 
> reliably for other filesystems then yes we could shutdown in a different way
> and make it generic. 

I didn't realize that Ext implements the XFS IOCTL to shutdown the filesystem
in the same way (src/godown.c uses XFS in the IOCTL name),so it will work. 

Sweet!

> 
> Don
> 
>>
>>> +_require_scratch
>>> +_require_scratch_shutdown
>>> +
>>> +_scratch_mkfs > $seqres.full 2>&1
>>> +# Don't use _scratch_mount because we need to mount without SELinux context
>>> +# to reproduce this issue. If we mount with SELinux context, this testcase
>>> +# is not reproducing the original issue.
>>> +mount $SCRATCH_DEV $SCRATCH_MNT
>>> +_xfs_force_bdev data $SCRATCH_MNT
>>> +
>>> +# Write test file
>>> +ls > $SCRATCH_MNT/testfile
>>> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
>>> +
>>> +# Shutdown
>>> +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
>>> +
>>> +# Mount ReadOnly
>>> +_scratch_unmount
>>> +_scratch_mount -oro
>>> +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
>>> +# Umount and mount rw
>>> +_scratch_unmount
>>> +_scratch_mount
>>> +
>>> +# Get fdblocks before repair
>>> +fdb1=$(_get_available_space $SCRATCH_MNT)
>>> +_scratch_unmount
>>> +
>>> +# Repair
>>> +_repair_scratch_fs >> $seqres.full 2>&1
>>> +
>>> +# Re-mount
>>> +_scratch_mount
>>> +
>>> +# Get fdblocks after repair
>>> +fdb2=$(_get_available_space $SCRATCH_MNT)
>>> +
>>> +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
>>> +
>>> +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
>>> +
>>> +# success, all done
>>> +echo "Silence is golden"
>>> +status=0
>>> +exit
>>> diff --git a/tests/xfs/999.out b/tests/xfs/999.out
>>> new file mode 100644
>>> index 00000000..3b276ca8
>>> --- /dev/null
>>> +++ b/tests/xfs/999.out
>>> @@ -0,0 +1,2 @@
>>> +QA output created by 999
>>> +Silence is golden
>>> -- 
>>> 2.20.1
>>>
>>
Darrick J. Wong Aug. 25, 2021, 11:43 p.m. UTC | #7
On Wed, Aug 25, 2021 at 12:53:41AM +0800, Zorro Lang wrote:
> On Tue, Aug 24, 2021 at 08:14:28AM -0700, Darrick J. Wong wrote:
> > On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> > > And followed by a rw mount.
> > > 
> > > Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> > > ---
> > > 
> > > Thanks Darrick very much for reviewing!
> > > 
> > > v2:
> > >    Add explaination of the issue
> > >    add xfs_force_bdev data $SCRATCH_MNT
> > >    use DF_PROG
> > >    Re numbered this test
> > > v3:
> > >    Add _require_scratch_shutdown
> > >    Use _get_available_space
> > >    Explain why does not use _scratch_mount
> > > 
> > >  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/xfs/999.out |  2 ++
> > >  2 files changed, 67 insertions(+)
> > >  create mode 100755 tests/xfs/999
> > >  create mode 100644 tests/xfs/999.out
> > > 
> > > diff --git a/tests/xfs/999 b/tests/xfs/999
> > > new file mode 100755
> > > index 00000000..0ce9989b
> > > --- /dev/null
> > > +++ b/tests/xfs/999
> > > @@ -0,0 +1,65 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2021 RedHat All Rights Reserved.
> > > +#
> > > +# FS QA Test 999
> > > +#
> > > +# Testcase for kernel commit:
> > > +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> > > +#
> > > +# After shutdown and readonly mount, a following read-write mount would
> > > +# get wrong number of available blocks. This is caused by unmounting the log
> > > +# on a readonly filesystem doesn't log the sb counters.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest shutdown auto quick
> > > +
> > > +# real QA test starts here
> > > +
> > > +_supported_fs xfs
> > > +_require_scratch
> > > +_require_scratch_shutdown
> > > +
> > > +_scratch_mkfs > $seqres.full 2>&1
> > > +# Don't use _scratch_mount because we need to mount without SELinux context
> > > +# to reproduce this issue. If we mount with SELinux context, this testcase
> > > +# is not reproducing the original issue.
> > > +mount $SCRATCH_DEV $SCRATCH_MNT
> > 
> > This mount will fail if the test runner configured an external log or a
> > realtime device, because you didn't specify those devices to the mount
> > call.  Either this needs _require_nonexternal or $(_scratch_options
> > mount) needs to be injected into the command line.
> 
> I think I know why Xiong said _scratch_mount can't reproduce this bug.
> 
> Before this patch v3, he tried to find the xfs corruption by using $DF_PROG $SCRATCH_MNT
> before&after xfs_repair.
> 
> If we use _scratch_mount, new files we create won't contains SELinux label
> in xfs inode attrfork. The corruption comes from sb_ifree (free inodes):
>   Phase 2 - using internal log
>         - zero log...
>         - scan filesystem freespace and inode maps...
>   sb_ifree 61, counted 60

<shrug> Faulty logging of the superblock can result in any of the
summary fields being wrong, so I don't see why this is a problem?
As long as at least one of the sb fields has to be corrected, we've
successfully found a broken kernel, right?

> But if we don't use _scratch_mount, the corruption comes from sb_fdblocks(free blocks):
>   Phase 2 - using internal log
>         - zero log...
>         - scan filesystem freespace and inode maps...
>   sb_fdblocks 130086290, counted 131007896
> 
> So if he use _scratch_mount, he can't get different available blocks by
> $DF_PROG $SCRATCH_MNT, before&after xfs_repair. Due to the corrupted thing
> is sb_ifree.
> 
> But I still can't understand one thing, even if do *not* use _scratch_mount, the attr(selinux label)
> is local(short) format[1], didn't take more blocks. What takes more blocks, cause later
> sb_fdblocks corruption?

/me has no idea either, other than to suggest

SELINUX_MOUNT_OPTIONS= _scratch_mount

if you don't want the selinux labels to be applied?

--D

> 
> Thanks,
> Zorro
> 
> [1]
> ...
> core.aformat = 1 (local)
> ...
> u3 = (empty)
> a.sfattr.hdr.totsize = 51
> a.sfattr.hdr.count = 1
> a.sfattr.list[0].namelen = 7
> a.sfattr.list[0].valuelen = 37
> a.sfattr.list[0].root = 0
> a.sfattr.list[0].secure = 1
> a.sfattr.list[0].name = "selinux"
> a.sfattr.list[0].value = "unconfined_u:object_r:unlabeled_t:s0\000"
> 
> 
> > 
> > --D
> > 
> > > +_xfs_force_bdev data $SCRATCH_MNT
> > > +
> > > +# Write test file
> > > +ls > $SCRATCH_MNT/testfile
> > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > +
> > > +# Shutdown
> > > +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> > > +
> > > +# Mount ReadOnly
> > > +_scratch_unmount
> > > +_scratch_mount -oro
> > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > +# Umount and mount rw
> > > +_scratch_unmount
> > > +_scratch_mount
> > > +
> > > +# Get fdblocks before repair
> > > +fdb1=$(_get_available_space $SCRATCH_MNT)
> > > +_scratch_unmount
> > > +
> > > +# Repair
> > > +_repair_scratch_fs >> $seqres.full 2>&1
> > > +
> > > +# Re-mount
> > > +_scratch_mount
> > > +
> > > +# Get fdblocks after repair
> > > +fdb2=$(_get_available_space $SCRATCH_MNT)
> > > +
> > > +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> > > +
> > > +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> > > +
> > > +# success, all done
> > > +echo "Silence is golden"
> > > +status=0
> > > +exit
> > > diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> > > new file mode 100644
> > > index 00000000..3b276ca8
> > > --- /dev/null
> > > +++ b/tests/xfs/999.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 999
> > > +Silence is golden
> > > -- 
> > > 2.20.1
> > > 
> > 
>
Murphy Zhou Aug. 26, 2021, midnight UTC | #8
On Thu, Aug 26, 2021 at 7:43 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Wed, Aug 25, 2021 at 12:53:41AM +0800, Zorro Lang wrote:
> > On Tue, Aug 24, 2021 at 08:14:28AM -0700, Darrick J. Wong wrote:
> > > On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> > > > And followed by a rw mount.
> > > >
> > > > Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > > Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> > > > ---
> > > >
> > > > Thanks Darrick very much for reviewing!
> > > >
> > > > v2:
> > > >    Add explaination of the issue
> > > >    add xfs_force_bdev data $SCRATCH_MNT
> > > >    use DF_PROG
> > > >    Re numbered this test
> > > > v3:
> > > >    Add _require_scratch_shutdown
> > > >    Use _get_available_space
> > > >    Explain why does not use _scratch_mount
> > > >
> > > >  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/xfs/999.out |  2 ++
> > > >  2 files changed, 67 insertions(+)
> > > >  create mode 100755 tests/xfs/999
> > > >  create mode 100644 tests/xfs/999.out
> > > >
> > > > diff --git a/tests/xfs/999 b/tests/xfs/999
> > > > new file mode 100755
> > > > index 00000000..0ce9989b
> > > > --- /dev/null
> > > > +++ b/tests/xfs/999
> > > > @@ -0,0 +1,65 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2021 RedHat All Rights Reserved.
> > > > +#
> > > > +# FS QA Test 999
> > > > +#
> > > > +# Testcase for kernel commit:
> > > > +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> > > > +#
> > > > +# After shutdown and readonly mount, a following read-write mount would
> > > > +# get wrong number of available blocks. This is caused by unmounting the log
> > > > +# on a readonly filesystem doesn't log the sb counters.
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest shutdown auto quick
> > > > +
> > > > +# real QA test starts here
> > > > +
> > > > +_supported_fs xfs
> > > > +_require_scratch
> > > > +_require_scratch_shutdown
> > > > +
> > > > +_scratch_mkfs > $seqres.full 2>&1
> > > > +# Don't use _scratch_mount because we need to mount without SELinux context
> > > > +# to reproduce this issue. If we mount with SELinux context, this testcase
> > > > +# is not reproducing the original issue.
> > > > +mount $SCRATCH_DEV $SCRATCH_MNT
> > >
> > > This mount will fail if the test runner configured an external log or a
> > > realtime device, because you didn't specify those devices to the mount
> > > call.  Either this needs _require_nonexternal or $(_scratch_options
> > > mount) needs to be injected into the command line.
> >
> > I think I know why Xiong said _scratch_mount can't reproduce this bug.
> >
> > Before this patch v3, he tried to find the xfs corruption by using $DF_PROG $SCRATCH_MNT
> > before&after xfs_repair.
> >
> > If we use _scratch_mount, new files we create won't contains SELinux label
> > in xfs inode attrfork. The corruption comes from sb_ifree (free inodes):
> >   Phase 2 - using internal log
> >         - zero log...
> >         - scan filesystem freespace and inode maps...
> >   sb_ifree 61, counted 60
>
> <shrug> Faulty logging of the superblock can result in any of the
> summary fields being wrong, so I don't see why this is a problem?
> As long as at least one of the sb fields has to be corrected, we've
> successfully found a broken kernel, right?
>
> > But if we don't use _scratch_mount, the corruption comes from sb_fdblocks(free blocks):
> >   Phase 2 - using internal log
> >         - zero log...
> >         - scan filesystem freespace and inode maps...
> >   sb_fdblocks 130086290, counted 131007896
> >
> > So if he use _scratch_mount, he can't get different available blocks by
> > $DF_PROG $SCRATCH_MNT, before&after xfs_repair. Due to the corrupted thing
> > is sb_ifree.
> >
> > But I still can't understand one thing, even if do *not* use _scratch_mount, the attr(selinux label)
> > is local(short) format[1], didn't take more blocks. What takes more blocks, cause later
> > sb_fdblocks corruption?
>
> /me has no idea either, other than to suggest
>
> SELINUX_MOUNT_OPTIONS= _scratch_mount
>
> if you don't want the selinux labels to be applied?

In v5 patch, it's not a problem any more. With _scratch_mount we still
can reproduce the issue.

Thanks!

>
> --D
>
> >
> > Thanks,
> > Zorro
> >
> > [1]
> > ...
> > core.aformat = 1 (local)
> > ...
> > u3 = (empty)
> > a.sfattr.hdr.totsize = 51
> > a.sfattr.hdr.count = 1
> > a.sfattr.list[0].namelen = 7
> > a.sfattr.list[0].valuelen = 37
> > a.sfattr.list[0].root = 0
> > a.sfattr.list[0].secure = 1
> > a.sfattr.list[0].name = "selinux"
> > a.sfattr.list[0].value = "unconfined_u:object_r:unlabeled_t:s0\000"
> >
> >
> > >
> > > --D
> > >
> > > > +_xfs_force_bdev data $SCRATCH_MNT
> > > > +
> > > > +# Write test file
> > > > +ls > $SCRATCH_MNT/testfile
> > > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > > +
> > > > +# Shutdown
> > > > +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> > > > +
> > > > +# Mount ReadOnly
> > > > +_scratch_unmount
> > > > +_scratch_mount -oro
> > > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > > +# Umount and mount rw
> > > > +_scratch_unmount
> > > > +_scratch_mount
> > > > +
> > > > +# Get fdblocks before repair
> > > > +fdb1=$(_get_available_space $SCRATCH_MNT)
> > > > +_scratch_unmount
> > > > +
> > > > +# Repair
> > > > +_repair_scratch_fs >> $seqres.full 2>&1
> > > > +
> > > > +# Re-mount
> > > > +_scratch_mount
> > > > +
> > > > +# Get fdblocks after repair
> > > > +fdb2=$(_get_available_space $SCRATCH_MNT)
> > > > +
> > > > +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> > > > +
> > > > +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> > > > +
> > > > +# success, all done
> > > > +echo "Silence is golden"
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> > > > new file mode 100644
> > > > index 00000000..3b276ca8
> > > > --- /dev/null
> > > > +++ b/tests/xfs/999.out
> > > > @@ -0,0 +1,2 @@
> > > > +QA output created by 999
> > > > +Silence is golden
> > > > --
> > > > 2.20.1
> > > >
> > >
> >
>
Darrick J. Wong Aug. 26, 2021, 12:15 a.m. UTC | #9
On Thu, Aug 26, 2021 at 08:00:33AM +0800, Murphy Zhou wrote:
> On Thu, Aug 26, 2021 at 7:43 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Wed, Aug 25, 2021 at 12:53:41AM +0800, Zorro Lang wrote:
> > > On Tue, Aug 24, 2021 at 08:14:28AM -0700, Darrick J. Wong wrote:
> > > > On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> > > > > And followed by a rw mount.
> > > > >
> > > > > Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> > > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > > > Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> > > > > ---
> > > > >
> > > > > Thanks Darrick very much for reviewing!
> > > > >
> > > > > v2:
> > > > >    Add explaination of the issue
> > > > >    add xfs_force_bdev data $SCRATCH_MNT
> > > > >    use DF_PROG
> > > > >    Re numbered this test
> > > > > v3:
> > > > >    Add _require_scratch_shutdown
> > > > >    Use _get_available_space
> > > > >    Explain why does not use _scratch_mount
> > > > >
> > > > >  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  tests/xfs/999.out |  2 ++
> > > > >  2 files changed, 67 insertions(+)
> > > > >  create mode 100755 tests/xfs/999
> > > > >  create mode 100644 tests/xfs/999.out
> > > > >
> > > > > diff --git a/tests/xfs/999 b/tests/xfs/999
> > > > > new file mode 100755
> > > > > index 00000000..0ce9989b
> > > > > --- /dev/null
> > > > > +++ b/tests/xfs/999
> > > > > @@ -0,0 +1,65 @@
> > > > > +#! /bin/bash
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +# Copyright (c) 2021 RedHat All Rights Reserved.
> > > > > +#
> > > > > +# FS QA Test 999
> > > > > +#
> > > > > +# Testcase for kernel commit:
> > > > > +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> > > > > +#
> > > > > +# After shutdown and readonly mount, a following read-write mount would
> > > > > +# get wrong number of available blocks. This is caused by unmounting the log
> > > > > +# on a readonly filesystem doesn't log the sb counters.
> > > > > +#
> > > > > +. ./common/preamble
> > > > > +_begin_fstest shutdown auto quick
> > > > > +
> > > > > +# real QA test starts here
> > > > > +
> > > > > +_supported_fs xfs
> > > > > +_require_scratch
> > > > > +_require_scratch_shutdown
> > > > > +
> > > > > +_scratch_mkfs > $seqres.full 2>&1
> > > > > +# Don't use _scratch_mount because we need to mount without SELinux context
> > > > > +# to reproduce this issue. If we mount with SELinux context, this testcase
> > > > > +# is not reproducing the original issue.
> > > > > +mount $SCRATCH_DEV $SCRATCH_MNT
> > > >
> > > > This mount will fail if the test runner configured an external log or a
> > > > realtime device, because you didn't specify those devices to the mount
> > > > call.  Either this needs _require_nonexternal or $(_scratch_options
> > > > mount) needs to be injected into the command line.
> > >
> > > I think I know why Xiong said _scratch_mount can't reproduce this bug.
> > >
> > > Before this patch v3, he tried to find the xfs corruption by using $DF_PROG $SCRATCH_MNT
> > > before&after xfs_repair.
> > >
> > > If we use _scratch_mount, new files we create won't contains SELinux label
> > > in xfs inode attrfork. The corruption comes from sb_ifree (free inodes):
> > >   Phase 2 - using internal log
> > >         - zero log...
> > >         - scan filesystem freespace and inode maps...
> > >   sb_ifree 61, counted 60
> >
> > <shrug> Faulty logging of the superblock can result in any of the
> > summary fields being wrong, so I don't see why this is a problem?
> > As long as at least one of the sb fields has to be corrected, we've
> > successfully found a broken kernel, right?
> >
> > > But if we don't use _scratch_mount, the corruption comes from sb_fdblocks(free blocks):
> > >   Phase 2 - using internal log
> > >         - zero log...
> > >         - scan filesystem freespace and inode maps...
> > >   sb_fdblocks 130086290, counted 131007896
> > >
> > > So if he use _scratch_mount, he can't get different available blocks by
> > > $DF_PROG $SCRATCH_MNT, before&after xfs_repair. Due to the corrupted thing
> > > is sb_ifree.
> > >
> > > But I still can't understand one thing, even if do *not* use _scratch_mount, the attr(selinux label)
> > > is local(short) format[1], didn't take more blocks. What takes more blocks, cause later
> > > sb_fdblocks corruption?
> >
> > /me has no idea either, other than to suggest
> >
> > SELINUX_MOUNT_OPTIONS= _scratch_mount
> >
> > if you don't want the selinux labels to be applied?
> 
> In v5 patch, it's not a problem any more. With _scratch_mount we still
> can reproduce the issue.

Heh, ok, I'll go look at that.  Didn't realize there was a v5 already.

--D

> Thanks!
> 
> >
> > --D
> >
> > >
> > > Thanks,
> > > Zorro
> > >
> > > [1]
> > > ...
> > > core.aformat = 1 (local)
> > > ...
> > > u3 = (empty)
> > > a.sfattr.hdr.totsize = 51
> > > a.sfattr.hdr.count = 1
> > > a.sfattr.list[0].namelen = 7
> > > a.sfattr.list[0].valuelen = 37
> > > a.sfattr.list[0].root = 0
> > > a.sfattr.list[0].secure = 1
> > > a.sfattr.list[0].name = "selinux"
> > > a.sfattr.list[0].value = "unconfined_u:object_r:unlabeled_t:s0\000"
> > >
> > >
> > > >
> > > > --D
> > > >
> > > > > +_xfs_force_bdev data $SCRATCH_MNT
> > > > > +
> > > > > +# Write test file
> > > > > +ls > $SCRATCH_MNT/testfile
> > > > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > > > +
> > > > > +# Shutdown
> > > > > +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> > > > > +
> > > > > +# Mount ReadOnly
> > > > > +_scratch_unmount
> > > > > +_scratch_mount -oro
> > > > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > > > +# Umount and mount rw
> > > > > +_scratch_unmount
> > > > > +_scratch_mount
> > > > > +
> > > > > +# Get fdblocks before repair
> > > > > +fdb1=$(_get_available_space $SCRATCH_MNT)
> > > > > +_scratch_unmount
> > > > > +
> > > > > +# Repair
> > > > > +_repair_scratch_fs >> $seqres.full 2>&1
> > > > > +
> > > > > +# Re-mount
> > > > > +_scratch_mount
> > > > > +
> > > > > +# Get fdblocks after repair
> > > > > +fdb2=$(_get_available_space $SCRATCH_MNT)
> > > > > +
> > > > > +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> > > > > +
> > > > > +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> > > > > +
> > > > > +# success, all done
> > > > > +echo "Silence is golden"
> > > > > +status=0
> > > > > +exit
> > > > > diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> > > > > new file mode 100644
> > > > > index 00000000..3b276ca8
> > > > > --- /dev/null
> > > > > +++ b/tests/xfs/999.out
> > > > > @@ -0,0 +1,2 @@
> > > > > +QA output created by 999
> > > > > +Silence is golden
> > > > > --
> > > > > 2.20.1
> > > > >
> > > >
> > >
> >
>
Zorro Lang Aug. 26, 2021, 6:59 a.m. UTC | #10
On Wed, Aug 25, 2021 at 04:43:37PM -0700, Darrick J. Wong wrote:
> On Wed, Aug 25, 2021 at 12:53:41AM +0800, Zorro Lang wrote:
> > On Tue, Aug 24, 2021 at 08:14:28AM -0700, Darrick J. Wong wrote:
> > > On Tue, Aug 24, 2021 at 01:04:36PM +0800, Murphy Zhou wrote:
> > > > And followed by a rw mount.
> > > > 
> > > > Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > > > Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> > > > ---
> > > > 
> > > > Thanks Darrick very much for reviewing!
> > > > 
> > > > v2:
> > > >    Add explaination of the issue
> > > >    add xfs_force_bdev data $SCRATCH_MNT
> > > >    use DF_PROG
> > > >    Re numbered this test
> > > > v3:
> > > >    Add _require_scratch_shutdown
> > > >    Use _get_available_space
> > > >    Explain why does not use _scratch_mount
> > > > 
> > > >  tests/xfs/999     | 65 +++++++++++++++++++++++++++++++++++++++++++++++
> > > >  tests/xfs/999.out |  2 ++
> > > >  2 files changed, 67 insertions(+)
> > > >  create mode 100755 tests/xfs/999
> > > >  create mode 100644 tests/xfs/999.out
> > > > 
> > > > diff --git a/tests/xfs/999 b/tests/xfs/999
> > > > new file mode 100755
> > > > index 00000000..0ce9989b
> > > > --- /dev/null
> > > > +++ b/tests/xfs/999
> > > > @@ -0,0 +1,65 @@
> > > > +#! /bin/bash
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +# Copyright (c) 2021 RedHat All Rights Reserved.
> > > > +#
> > > > +# FS QA Test 999
> > > > +#
> > > > +# Testcase for kernel commit:
> > > > +#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
> > > > +#
> > > > +# After shutdown and readonly mount, a following read-write mount would
> > > > +# get wrong number of available blocks. This is caused by unmounting the log
> > > > +# on a readonly filesystem doesn't log the sb counters.
> > > > +#
> > > > +. ./common/preamble
> > > > +_begin_fstest shutdown auto quick
> > > > +
> > > > +# real QA test starts here
> > > > +
> > > > +_supported_fs xfs
> > > > +_require_scratch
> > > > +_require_scratch_shutdown
> > > > +
> > > > +_scratch_mkfs > $seqres.full 2>&1
> > > > +# Don't use _scratch_mount because we need to mount without SELinux context
> > > > +# to reproduce this issue. If we mount with SELinux context, this testcase
> > > > +# is not reproducing the original issue.
> > > > +mount $SCRATCH_DEV $SCRATCH_MNT
> > > 
> > > This mount will fail if the test runner configured an external log or a
> > > realtime device, because you didn't specify those devices to the mount
> > > call.  Either this needs _require_nonexternal or $(_scratch_options
> > > mount) needs to be injected into the command line.
> > 
> > I think I know why Xiong said _scratch_mount can't reproduce this bug.
> > 
> > Before this patch v3, he tried to find the xfs corruption by using $DF_PROG $SCRATCH_MNT
> > before&after xfs_repair.
> > 
> > If we use _scratch_mount, new files we create won't contains SELinux label
> > in xfs inode attrfork. The corruption comes from sb_ifree (free inodes):
> >   Phase 2 - using internal log
> >         - zero log...
> >         - scan filesystem freespace and inode maps...
> >   sb_ifree 61, counted 60
> 
> <shrug> Faulty logging of the superblock can result in any of the
> summary fields being wrong, so I don't see why this is a problem?
> As long as at least one of the sb fields has to be corrected, we've
> successfully found a broken kernel, right?
> 
> > But if we don't use _scratch_mount, the corruption comes from sb_fdblocks(free blocks):
> >   Phase 2 - using internal log
> >         - zero log...
> >         - scan filesystem freespace and inode maps...
> >   sb_fdblocks 130086290, counted 131007896
> > 
> > So if he use _scratch_mount, he can't get different available blocks by
> > $DF_PROG $SCRATCH_MNT, before&after xfs_repair. Due to the corrupted thing
> > is sb_ifree.
> > 
> > But I still can't understand one thing, even if do *not* use _scratch_mount, the attr(selinux label)
> > is local(short) format[1], didn't take more blocks. What takes more blocks, cause later
> > sb_fdblocks corruption?
> 
> /me has no idea either, other than to suggest
> 
> SELINUX_MOUNT_OPTIONS= _scratch_mount
> 
> if you don't want the selinux labels to be applied?

Jusk asking, to make sure if you know why with/without SELINUX_MOUNT_OPTIONS
cause this difference (just curious:)
As the single _scratch_mount already can reproduce this bug now, I don't think
we need give it more specific conditions to confuse this simple test :-D

Thanks,
Zorro

> 
> --D
> 
> > 
> > Thanks,
> > Zorro
> > 
> > [1]
> > ...
> > core.aformat = 1 (local)
> > ...
> > u3 = (empty)
> > a.sfattr.hdr.totsize = 51
> > a.sfattr.hdr.count = 1
> > a.sfattr.list[0].namelen = 7
> > a.sfattr.list[0].valuelen = 37
> > a.sfattr.list[0].root = 0
> > a.sfattr.list[0].secure = 1
> > a.sfattr.list[0].name = "selinux"
> > a.sfattr.list[0].value = "unconfined_u:object_r:unlabeled_t:s0\000"
> > 
> > 
> > > 
> > > --D
> > > 
> > > > +_xfs_force_bdev data $SCRATCH_MNT
> > > > +
> > > > +# Write test file
> > > > +ls > $SCRATCH_MNT/testfile
> > > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > > +
> > > > +# Shutdown
> > > > +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> > > > +
> > > > +# Mount ReadOnly
> > > > +_scratch_unmount
> > > > +_scratch_mount -oro
> > > > +$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
> > > > +# Umount and mount rw
> > > > +_scratch_unmount
> > > > +_scratch_mount
> > > > +
> > > > +# Get fdblocks before repair
> > > > +fdb1=$(_get_available_space $SCRATCH_MNT)
> > > > +_scratch_unmount
> > > > +
> > > > +# Repair
> > > > +_repair_scratch_fs >> $seqres.full 2>&1
> > > > +
> > > > +# Re-mount
> > > > +_scratch_mount
> > > > +
> > > > +# Get fdblocks after repair
> > > > +fdb2=$(_get_available_space $SCRATCH_MNT)
> > > > +
> > > > +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> > > > +
> > > > +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
> > > > +
> > > > +# success, all done
> > > > +echo "Silence is golden"
> > > > +status=0
> > > > +exit
> > > > diff --git a/tests/xfs/999.out b/tests/xfs/999.out
> > > > new file mode 100644
> > > > index 00000000..3b276ca8
> > > > --- /dev/null
> > > > +++ b/tests/xfs/999.out
> > > > @@ -0,0 +1,2 @@
> > > > +QA output created by 999
> > > > +Silence is golden
> > > > -- 
> > > > 2.20.1
> > > > 
> > > 
> > 
>
diff mbox series

Patch

diff --git a/tests/xfs/999 b/tests/xfs/999
new file mode 100755
index 00000000..0ce9989b
--- /dev/null
+++ b/tests/xfs/999
@@ -0,0 +1,65 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 RedHat All Rights Reserved.
+#
+# FS QA Test 999
+#
+# Testcase for kernel commit:
+#   50d25484bebe xfs: sync lazy sb accounting on quiesce of read-only mounts
+#
+# After shutdown and readonly mount, a following read-write mount would
+# get wrong number of available blocks. This is caused by unmounting the log
+# on a readonly filesystem doesn't log the sb counters.
+#
+. ./common/preamble
+_begin_fstest shutdown auto quick
+
+# real QA test starts here
+
+_supported_fs xfs
+_require_scratch
+_require_scratch_shutdown
+
+_scratch_mkfs > $seqres.full 2>&1
+# Don't use _scratch_mount because we need to mount without SELinux context
+# to reproduce this issue. If we mount with SELinux context, this testcase
+# is not reproducing the original issue.
+mount $SCRATCH_DEV $SCRATCH_MNT
+_xfs_force_bdev data $SCRATCH_MNT
+
+# Write test file
+ls > $SCRATCH_MNT/testfile
+$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
+
+# Shutdown
+$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
+
+# Mount ReadOnly
+_scratch_unmount
+_scratch_mount -oro
+$DF_PROG $SCRATCH_MNT >> $seqres.full 2>&1
+# Umount and mount rw
+_scratch_unmount
+_scratch_mount
+
+# Get fdblocks before repair
+fdb1=$(_get_available_space $SCRATCH_MNT)
+_scratch_unmount
+
+# Repair
+_repair_scratch_fs >> $seqres.full 2>&1
+
+# Re-mount
+_scratch_mount
+
+# Get fdblocks after repair
+fdb2=$(_get_available_space $SCRATCH_MNT)
+
+echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
+
+[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/xfs/999.out b/tests/xfs/999.out
new file mode 100644
index 00000000..3b276ca8
--- /dev/null
+++ b/tests/xfs/999.out
@@ -0,0 +1,2 @@ 
+QA output created by 999
+Silence is golden