[PATCH] tests/xfs: check avail blocks after log recovery on ro mount

[PATCH] tests/xfs: check avail blocks after log recovery on ro mount

[Murphy Zhou]

And followed by a rw mount.

Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
 tests/xfs/175     | 59 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/175.out |  2 ++
 2 files changed, 61 insertions(+)
 create mode 100755 tests/xfs/175
 create mode 100644 tests/xfs/175.out

diff --git a/tests/xfs/175 b/tests/xfs/175
new file mode 100755
index 00000000..2a79a3d4
--- /dev/null
+++ b/tests/xfs/175
@@ -0,0 +1,59 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2021 RedHat All Rights Reserved.
+#
+# FS QA Test 175
+#
+# 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.
+#
+. ./common/preamble
+_begin_fstest shutdown auto quick
+
+# real QA test starts here
+
+_supported_fs xfs
+_require_scratch
+
+_scratch_mkfs > $seqres.full 2>&1
+mount $SCRATCH_DEV $SCRATCH_MNT
+
+# Write test file
+ls > $SCRATCH_MNT/testfile
+df --output=avail $SCRATCH_MNT >> $seqres.full 2>&1
+
+# Shutdown
+$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
+
+# Mount ReadOnly
+_scratch_unmount
+_scratch_mount -oro
+df --output=avail $SCRATCH_MNT >> $seqres.full 2>&1
+# Umount and mount rw
+_scratch_unmount
+_scratch_mount
+
+# Get fdblocks before repair
+fdb1=$(df --output=avail $SCRATCH_MNT | tail -1)
+_scratch_unmount
+
+# Repair
+_repair_scratch_fs >> $seqres.full 2>&1
+
+# Re-mount
+_scratch_mount
+
+# Get fdblocks after repair
+fdb2=$(df --output=avail $SCRATCH_MNT | tail -1)
+
+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/175.out b/tests/xfs/175.out
new file mode 100644
index 00000000..3b4af34b
--- /dev/null
+++ b/tests/xfs/175.out
@@ -0,0 +1,2 @@
+QA output created by 175
+Silence is golden
-- 
2.20.1

Re: [PATCH] tests/xfs: check avail blocks after log recovery on ro mount

[Darrick J. Wong]

On Fri, Aug 06, 2021 at 09:49:38AM +0800, Murphy Zhou wrote:
> And followed by a rw mount.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
>  tests/xfs/175     | 59 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/175.out |  2 ++
>  2 files changed, 61 insertions(+)
>  create mode 100755 tests/xfs/175
>  create mode 100644 tests/xfs/175.out
> 
> diff --git a/tests/xfs/175 b/tests/xfs/175
> new file mode 100755
> index 00000000..2a79a3d4
> --- /dev/null
> +++ b/tests/xfs/175
> @@ -0,0 +1,59 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2021 RedHat All Rights Reserved.
> +#
> +# FS QA Test 175
> +#
> +# 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.

That's a description of the reproduction strategy, but the fault was
that unmounting the log on a readonly filesystem doesn't log the sb
counters.  I think it's worth mentioning that in the description.

Thanks for putting together a test case though. :)

> +#
> +. ./common/preamble
> +_begin_fstest shutdown auto quick
> +
> +# real QA test starts here
> +
> +_supported_fs xfs
> +_require_scratch
> +
> +_scratch_mkfs > $seqres.full 2>&1
> +mount $SCRATCH_DEV $SCRATCH_MNT

_scratch_mount ?

Also, you probably want

_xfs_force_bdev data $SCRATCH_MNT

to force testfile to be created on the data device if the test runner
passed -d rtinherit=1 in MKFS_OPTS.

> +
> +# Write test file
> +ls > $SCRATCH_MNT/testfile
> +df --output=avail $SCRATCH_MNT >> $seqres.full 2>&1

$DF_PROG

> +
> +# Shutdown
> +$XFS_IO_PROG -x -c "shutdown -f" $SCRATCH_MNT
> +
> +# Mount ReadOnly
> +_scratch_unmount
> +_scratch_mount -oro
> +df --output=avail $SCRATCH_MNT >> $seqres.full 2>&1
> +# Umount and mount rw
> +_scratch_unmount
> +_scratch_mount
> +
> +# Get fdblocks before repair
> +fdb1=$(df --output=avail $SCRATCH_MNT | tail -1)
> +_scratch_unmount
> +
> +# Repair
> +_repair_scratch_fs >> $seqres.full 2>&1
> +
> +# Re-mount
> +_scratch_mount
> +
> +# Get fdblocks after repair
> +fdb2=$(df --output=avail $SCRATCH_MNT | tail -1)
> +
> +echo fdb1 $fdb1 fdb2 $fdb2 >> $seqres.full 2>&1
> +
> +[ $fdb1 -ne $fdb2 ] && echo Wrong fdblocks: $fdb1 and $fdb2

Looks reasonable enough to me.

--D

> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/xfs/175.out b/tests/xfs/175.out
> new file mode 100644
> index 00000000..3b4af34b
> --- /dev/null
> +++ b/tests/xfs/175.out
> @@ -0,0 +1,2 @@
> +QA output created by 175
> +Silence is golden
> -- 
> 2.20.1
> 

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

[Murphy Zhou]

And followed by a rw mount.

Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
Signed-off-by: Murphy Zhou <xzhou@redhat.com>
---
v2:
   Add explaination of the issue
   add xfs_force_bdev data $SCRATCH_MNT
   use DF_PROG
   Re numbered this test

 tests/xfs/999     | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/999.out |  2 ++
 2 files changed, 63 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..c93ed770
--- /dev/null
+++ b/tests/xfs/999
@@ -0,0 +1,61 @@
+#! /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
+
+_scratch_mkfs > $seqres.full 2>&1
+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=$($DF_PROG $SCRATCH_MNT | tail -1 | $AWK_PROG '{print $4}')
+_scratch_unmount
+
+# Repair
+_repair_scratch_fs >> $seqres.full 2>&1
+
+# Re-mount
+_scratch_mount
+
+# Get fdblocks after repair
+fdb2=$($DF_PROG $SCRATCH_MNT | tail -1 | $AWK_PROG '{print $4}')
+
+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

Re: [PATCH v2] tests/xfs: check available blocks after log recovery on ro mount

[Darrick J. Wong]

On Mon, Aug 23, 2021 at 03:05:41PM +0800, Murphy Zhou wrote:
> And followed by a rw mount.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
> v2:
>    Add explaination of the issue
>    add xfs_force_bdev data $SCRATCH_MNT
>    use DF_PROG
>    Re numbered this test
> 
>  tests/xfs/999     | 61 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/999.out |  2 ++
>  2 files changed, 63 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..c93ed770
> --- /dev/null
> +++ b/tests/xfs/999
> @@ -0,0 +1,61 @@
> +#! /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

Also needs
_require_scratch_shutdown

> +
> +_scratch_mkfs > $seqres.full 2>&1
> +mount $SCRATCH_DEV $SCRATCH_MNT

_scratch_mount?

> +_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=$($DF_PROG $SCRATCH_MNT | tail -1 | $AWK_PROG '{print $4}')

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=$($DF_PROG $SCRATCH_MNT | tail -1 | $AWK_PROG '{print $4}')

Here too.

With those bits fixed, I think this is in good shape for:
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> +
> +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
> 

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

[Murphy Zhou]

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
+_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

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

[Eryu Guan]

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

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

[Zorro Lang]

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
> 

[PATCH v4] tests/generic: check log recovery with readonly mount

[Murphy Zhou]

And followed by a rw mount. After log recovery by these 2 mount, the
filesystem should be in a consistent state.

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

Thanks Eryu and Zorro!

v2:
   Add explanation 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
v4:
   Add to recoveryloop group
   Move to generic as there are no xfs specific operations
   Remove all operations after 2 cycle mounts, let the fsck in fstests
to check the filesystem consistency
   Use _scratch_shutdown, MOUNT_PROG
   Remove unnecessary comments

 tests/generic/999     | 45 +++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  2 ++
 2 files changed, 47 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..9685488b
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,45 @@
+#! /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 auto quick recoveryloop shutdown
+
+# real QA test starts here
+
+_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 _scratch_mount, SELinux context
+# maybe applied and this testcase is not reproducing the original issue.
+$MOUNT_PROG $SCRATCH_DEV $SCRATCH_MNT
+_xfs_force_bdev data $SCRATCH_MNT
+
+echo Testing > $SCRATCH_MNT/testfile
+
+# -f is required to reproduce
+_scratch_shutdown -f
+
+_scratch_cycle_mount ro
+_scratch_cycle_mount
+
+# These 2 mount should have the log fully recovered. Exit here and let the
+# fsck operation to check the consistence of the tested filesystem. On the
+# buggy kernel, this testcase reports filesystem is in inconsistent state.
+# On the fixed kernel, testcase pass.
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..3b276ca8
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,2 @@
+QA output created by 999
+Silence is golden
-- 
2.20.1

Re: [PATCH v4] tests/generic: check log recovery with readonly mount

[Zorro Lang]

On Tue, Aug 24, 2021 at 05:06:38PM +0800, Murphy Zhou wrote:
> And followed by a rw mount. After log recovery by these 2 mount, the
> filesystem should be in a consistent state.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
> 
> Thanks Eryu and Zorro!
> 
> v2:
>    Add explanation 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
> v4:
>    Add to recoveryloop group
>    Move to generic as there are no xfs specific operations
>    Remove all operations after 2 cycle mounts, let the fsck in fstests
> to check the filesystem consistency
>    Use _scratch_shutdown, MOUNT_PROG
>    Remove unnecessary comments
> 
>  tests/generic/999     | 45 +++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  2 ++
>  2 files changed, 47 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
> 
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..9685488b
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,45 @@
> +#! /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 auto quick recoveryloop shutdown
> +
> +# real QA test starts here
> +
> +_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 _scratch_mount, SELinux context
> +# maybe applied and this testcase is not reproducing the original issue.
> +$MOUNT_PROG $SCRATCH_DEV $SCRATCH_MNT

This's weird, I think the SELinux mount option only can make below testfile
have no selinux label (xattr). But I don't think it's related with this bug.
So I tried to use "_scratch_mount", then still can reproduce this bug. May
you try it again?

# git diff
diff --git a/tests/generic/999 b/tests/generic/999
index 9685488b..a196e65d 100755
--- a/tests/generic/999
+++ b/tests/generic/999
@@ -23,8 +23,11 @@ _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 _scratch_mount, SELinux context
 # maybe applied and this testcase is not reproducing the original issue.
-$MOUNT_PROG $SCRATCH_DEV $SCRATCH_MNT
-_xfs_force_bdev data $SCRATCH_MNT
+#$MOUNT_PROG $SCRATCH_DEV $SCRATCH_MNT
+_scratch_mount
+if [ "$FSTYP" = "xfs" ];then
+       _xfs_force_bdev data $SCRATCH_MNT
+fi

# ./check generic/999
FSTYP         -- xfs (non-debug)
PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 4.18.0-xxx.el8.x86_64+debug #1 SMP Tue Jun 15 10:43:04 EDT 2021
MKFS_OPTIONS  -- -f /dev/mapper/rhel_hp--dl380pg8--01-xfscratch
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /dev/mapper/rhel_hp--dl380pg8--01-xfscratch /mnt/scratch

generic/999 16s ... _check_xfs_filesystem: filesystem on /dev/mapper/rhel_hp--dl380pg8--01-xfscratch is inconsistent (r)
(see /home/xfstests-dev/results//generic/999.full for details)

Ran: generic/999
Failures: generic/999
Failed 1 of 1 tests

> +_xfs_force_bdev data $SCRATCH_MNT

This's a xfs only helper, for a generic case, better to:

if [ "$FSTYP" = "xfs" ];then
	_xfs_force_bdev data $SCRATCH_MNT
fi

> +
> +echo Testing > $SCRATCH_MNT/testfile
> +
> +# -f is required to reproduce
> +_scratch_shutdown -f
> +
> +_scratch_cycle_mount ro
> +_scratch_cycle_mount
> +
> +# These 2 mount should have the log fully recovered. Exit here and let the
> +# fsck operation to check the consistence of the tested filesystem. On the
> +# buggy kernel, this testcase reports filesystem is in inconsistent state.
> +# On the fixed kernel, testcase pass.
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.20.1
> 

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

[Darrick J. Wong]

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
> 

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

[Zorro Lang]

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
> > 
> 

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

[Donald Douwsma]

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
>>
> 

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

[Donald Douwsma]

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
>>>
>>

[PATCH v5] tests/generic: check log recovery with readonly mount

[Murphy Zhou]

And followed by a rw mount. After log recovery by these 2 mounts, the
filesystem should be in a consistent state.

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

Thanks for reviewing! It seems my last several replies from gmail got
rejected by the list.

v2:
   Add explanation 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
v4:
   Add to recoveryloop group
   Move to generic as there is no xfs specific operations
   Remove all operations after 2 cycle mounts, let the fsck in fstests to check the filesystem consistency
   Use _scratch_shutdown, MOUNT_PROG
   Remove unnecessary comments
v5:
   Make _xfs_force_bdev xfs only
   Use _scratch_mount, it's still reproducible


 tests/generic/999     | 42 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/999.out |  2 ++
 2 files changed, 44 insertions(+)
 create mode 100755 tests/generic/999
 create mode 100644 tests/generic/999.out

diff --git a/tests/generic/999 b/tests/generic/999
new file mode 100755
index 00000000..60e0ce59
--- /dev/null
+++ b/tests/generic/999
@@ -0,0 +1,42 @@
+#! /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 auto quick recoveryloop shutdown
+
+# real QA test starts here
+
+_require_scratch
+_require_scratch_shutdown
+_scratch_mkfs > $seqres.full 2>&1
+
+_scratch_mount
+[ "$FSTYP" == "xfs" ] && _xfs_force_bdev data $SCRATCH_MNT
+
+echo Testing > $SCRATCH_MNT/testfile
+
+# -f is required to reproduce the original issue
+_scratch_shutdown -f
+
+_scratch_cycle_mount ro
+_scratch_cycle_mount
+
+# These two mounts should have the log fully recovered. Exit here and let the
+# fsck operation of xfstests to check the consistence of the tested filesystem.
+# On the buggy kernel, this testcase reports filesystem is inconsistent.
+# On the fixed kernel, testcase pass.
+
+# success, all done
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/generic/999.out b/tests/generic/999.out
new file mode 100644
index 00000000..3b276ca8
--- /dev/null
+++ b/tests/generic/999.out
@@ -0,0 +1,2 @@
+QA output created by 999
+Silence is golden
-- 
2.20.1

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

[Darrick J. Wong]

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
> > > 
> > 
> 

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

[Murphy Zhou]

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
> > > >
> > >
> >
>

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

[Darrick J. Wong]

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
> > > > >
> > > >
> > >
> >
> 

Re: [PATCH v5] tests/generic: check log recovery with readonly mount

[Darrick J. Wong]

On Wed, Aug 25, 2021 at 11:26:30AM +0800, Murphy Zhou wrote:
> And followed by a rw mount. After log recovery by these 2 mounts, the
> filesystem should be in a consistent state.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
> 
> Thanks for reviewing! It seems my last several replies from gmail got
> rejected by the list.
> 
> v2:
>    Add explanation 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
> v4:
>    Add to recoveryloop group
>    Move to generic as there is no xfs specific operations
>    Remove all operations after 2 cycle mounts, let the fsck in fstests to check the filesystem consistency
>    Use _scratch_shutdown, MOUNT_PROG
>    Remove unnecessary comments
> v5:
>    Make _xfs_force_bdev xfs only
>    Use _scratch_mount, it's still reproducible

Ok, seems fine now.

--D

> 
> 
>  tests/generic/999     | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  2 ++
>  2 files changed, 44 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
> 
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..60e0ce59
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,42 @@
> +#! /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 auto quick recoveryloop shutdown
> +
> +# real QA test starts here
> +
> +_require_scratch
> +_require_scratch_shutdown
> +_scratch_mkfs > $seqres.full 2>&1
> +
> +_scratch_mount
> +[ "$FSTYP" == "xfs" ] && _xfs_force_bdev data $SCRATCH_MNT
> +
> +echo Testing > $SCRATCH_MNT/testfile
> +
> +# -f is required to reproduce the original issue
> +_scratch_shutdown -f
> +
> +_scratch_cycle_mount ro
> +_scratch_cycle_mount
> +
> +# These two mounts should have the log fully recovered. Exit here and let the
> +# fsck operation of xfstests to check the consistence of the tested filesystem.
> +# On the buggy kernel, this testcase reports filesystem is inconsistent.
> +# On the fixed kernel, testcase pass.
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.20.1
> 

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

[Zorro Lang]

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
> > > > 
> > > 
> > 
> 

Re: [PATCH v5] tests/generic: check log recovery with readonly mount

[Zorro Lang]

On Wed, Aug 25, 2021 at 11:26:30AM +0800, Murphy Zhou wrote:
> And followed by a rw mount. After log recovery by these 2 mounts, the
> filesystem should be in a consistent state.
> 
> Suggested-by:  Donald Douwsma <ddouwsma@redhat.com>
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> Signed-off-by: Murphy Zhou <xzhou@redhat.com>
> ---
> 
> Thanks for reviewing! It seems my last several replies from gmail got
> rejected by the list.
> 
> v2:
>    Add explanation 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
> v4:
>    Add to recoveryloop group
>    Move to generic as there is no xfs specific operations
>    Remove all operations after 2 cycle mounts, let the fsck in fstests to check the filesystem consistency
>    Use _scratch_shutdown, MOUNT_PROG
>    Remove unnecessary comments
> v5:
>    Make _xfs_force_bdev xfs only
>    Use _scratch_mount, it's still reproducible

Good to me now,
Reviewed-by: Zorro Lang <zlang@redhat.com>

> 
> 
>  tests/generic/999     | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/999.out |  2 ++
>  2 files changed, 44 insertions(+)
>  create mode 100755 tests/generic/999
>  create mode 100644 tests/generic/999.out
> 
> diff --git a/tests/generic/999 b/tests/generic/999
> new file mode 100755
> index 00000000..60e0ce59
> --- /dev/null
> +++ b/tests/generic/999
> @@ -0,0 +1,42 @@
> +#! /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 auto quick recoveryloop shutdown
> +
> +# real QA test starts here
> +
> +_require_scratch
> +_require_scratch_shutdown
> +_scratch_mkfs > $seqres.full 2>&1
> +
> +_scratch_mount
> +[ "$FSTYP" == "xfs" ] && _xfs_force_bdev data $SCRATCH_MNT
> +
> +echo Testing > $SCRATCH_MNT/testfile
> +
> +# -f is required to reproduce the original issue
> +_scratch_shutdown -f
> +
> +_scratch_cycle_mount ro
> +_scratch_cycle_mount
> +
> +# These two mounts should have the log fully recovered. Exit here and let the
> +# fsck operation of xfstests to check the consistence of the tested filesystem.
> +# On the buggy kernel, this testcase reports filesystem is inconsistent.
> +# On the fixed kernel, testcase pass.
> +
> +# success, all done
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/generic/999.out b/tests/generic/999.out
> new file mode 100644
> index 00000000..3b276ca8
> --- /dev/null
> +++ b/tests/generic/999.out
> @@ -0,0 +1,2 @@
> +QA output created by 999
> +Silence is golden
> -- 
> 2.20.1
>