[PATCH 0/2] add read-only logdev/rtdev mount-remount tests

[PATCH 0/2] add read-only logdev/rtdev mount-remount tests

[Hans Holmberg]

These two tests checks different mount scenarios when logdev/rtdevs
are marked up up as read-only.

Currently these fail (mount fails if rtdevs/logdevs are ro), but
I've submitted a patch to adress this:

https://marc.info/?l=linux-xfs&m=174557094404857&w=2

Hans Holmberg (2):
  xfs: add mount test for read only rt devices
  xfs: add mount test for read only log devices

 tests/xfs/837     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/837.out | 10 +++++++++
 tests/xfs/838     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/838.out | 10 +++++++++
 4 files changed, 130 insertions(+)
 create mode 100755 tests/xfs/837
 create mode 100644 tests/xfs/837.out
 create mode 100755 tests/xfs/838
 create mode 100644 tests/xfs/838.out

-- 
2.34.1

[PATCH 1/2] xfs: add mount test for read only rt devices

[Hans Holmberg]

Make sure that we can mount rt devices read-only if them themselves
are marked as read-only.

Also make sure that rw re-mounts are not allowed if the device is
marked as read-only.

Based on generic/050.

Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---
 tests/xfs/837     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/837.out | 10 +++++++++
 2 files changed, 65 insertions(+)
 create mode 100755 tests/xfs/837
 create mode 100644 tests/xfs/837.out

diff --git a/tests/xfs/837 b/tests/xfs/837
new file mode 100755
index 000000000000..b20e9c5f33f2
--- /dev/null
+++ b/tests/xfs/837
@@ -0,0 +1,55 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2009 Christoph Hellwig.
+# Copyright (c) 2025 Western Digital Corporation
+#
+# FS QA Test No. 837
+#
+# Check out various mount/remount/unmount scenarious on a read-only rtdev
+# Based on generic/050
+#
+. ./common/preamble
+_begin_fstest mount auto quick
+
+_cleanup_setrw()
+{
+	cd /
+	blockdev --setrw $SCRATCH_RTDEV
+}
+
+# Import common functions.
+. ./common/filter
+
+_require_realtime
+_require_local_device $SCRATCH_RTDEV
+_register_cleanup "_cleanup_setrw"
+
+_scratch_mkfs "-d rtinherit" > /dev/null 2>&1
+
+#
+# Mark the rt device read-only.
+#
+echo "setting device read-only"
+blockdev --setro $SCRATCH_RTDEV
+
+#
+# Mount it and make sure it can't be written to.
+#
+echo "mounting read-only rt block device:"
+_scratch_mount 2>&1 | _filter_ro_mount | _filter_scratch
+if [ "${PIPESTATUS[0]}" -eq 0 ]; then
+	echo "writing to file on read-only filesystem:"
+	dd if=/dev/zero of=$SCRATCH_MNT/foo bs=1M count=1 oflag=direct 2>&1 | _filter_scratch
+else
+	_fail "failed to mount"
+fi
+
+echo "remounting read-write:"
+_scratch_remount rw 2>&1 | _filter_scratch | _filter_ro_mount
+
+echo "unmounting read-only filesystem"
+_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
+
+# success, all done
+echo "*** done"
+status=0
diff --git a/tests/xfs/837.out b/tests/xfs/837.out
new file mode 100644
index 000000000000..0a843a0ba398
--- /dev/null
+++ b/tests/xfs/837.out
@@ -0,0 +1,10 @@
+QA output created by 837
+setting device read-only
+mounting read-only rt block device:
+mount: device write-protected, mounting read-only
+writing to file on read-only filesystem:
+dd: failed to open 'SCRATCH_MNT/foo': Read-only file system
+remounting read-write:
+mount: cannot remount device read-write, is write-protected
+unmounting read-only filesystem
+*** done
-- 
2.34.1

Re: [PATCH 1/2] xfs: add mount test for read only rt devices

[Darrick J. Wong]

On Fri, Apr 25, 2025 at 09:03:22AM +0000, Hans Holmberg wrote:
> Make sure that we can mount rt devices read-only if them themselves
> are marked as read-only.
> 
> Also make sure that rw re-mounts are not allowed if the device is
> marked as read-only.
> 
> Based on generic/050.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> ---
>  tests/xfs/837     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/837.out | 10 +++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100755 tests/xfs/837
>  create mode 100644 tests/xfs/837.out
> 
> diff --git a/tests/xfs/837 b/tests/xfs/837
> new file mode 100755
> index 000000000000..b20e9c5f33f2
> --- /dev/null
> +++ b/tests/xfs/837
> @@ -0,0 +1,55 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2009 Christoph Hellwig.
> +# Copyright (c) 2025 Western Digital Corporation
> +#
> +# FS QA Test No. 837
> +#
> +# Check out various mount/remount/unmount scenarious on a read-only rtdev
> +# Based on generic/050
> +#
> +. ./common/preamble
> +_begin_fstest mount auto quick
> +
> +_cleanup_setrw()
> +{
> +	cd /
> +	blockdev --setrw $SCRATCH_RTDEV
> +}
> +
> +# Import common functions.
> +. ./common/filter
> +
> +_require_realtime
> +_require_local_device $SCRATCH_RTDEV

I suspect this is copy-pasted from generic/050, but I wonder when
SCRATCH_RTDEV could be a character device, but maybe that's a relic of
Irix (and Solaris too, IIRC)?

The rest of the test looks fine to me though.

--D

> +_register_cleanup "_cleanup_setrw"
> +
> +_scratch_mkfs "-d rtinherit" > /dev/null 2>&1
> +
> +#
> +# Mark the rt device read-only.
> +#
> +echo "setting device read-only"
> +blockdev --setro $SCRATCH_RTDEV
> +
> +#
> +# Mount it and make sure it can't be written to.
> +#
> +echo "mounting read-only rt block device:"
> +_scratch_mount 2>&1 | _filter_ro_mount | _filter_scratch
> +if [ "${PIPESTATUS[0]}" -eq 0 ]; then
> +	echo "writing to file on read-only filesystem:"
> +	dd if=/dev/zero of=$SCRATCH_MNT/foo bs=1M count=1 oflag=direct 2>&1 | _filter_scratch
> +else
> +	_fail "failed to mount"
> +fi
> +
> +echo "remounting read-write:"
> +_scratch_remount rw 2>&1 | _filter_scratch | _filter_ro_mount
> +
> +echo "unmounting read-only filesystem"
> +_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
> +
> +# success, all done
> +echo "*** done"
> +status=0
> diff --git a/tests/xfs/837.out b/tests/xfs/837.out
> new file mode 100644
> index 000000000000..0a843a0ba398
> --- /dev/null
> +++ b/tests/xfs/837.out
> @@ -0,0 +1,10 @@
> +QA output created by 837
> +setting device read-only
> +mounting read-only rt block device:
> +mount: device write-protected, mounting read-only
> +writing to file on read-only filesystem:
> +dd: failed to open 'SCRATCH_MNT/foo': Read-only file system
> +remounting read-write:
> +mount: cannot remount device read-write, is write-protected
> +unmounting read-only filesystem
> +*** done
> -- 
> 2.34.1
> 

Re: [PATCH 1/2] xfs: add mount test for read only rt devices

[Hans Holmberg]

On 25/04/2025 17:04, Darrick J. Wong wrote:
> On Fri, Apr 25, 2025 at 09:03:22AM +0000, Hans Holmberg wrote:
>> Make sure that we can mount rt devices read-only if them themselves
>> are marked as read-only.
>>
>> Also make sure that rw re-mounts are not allowed if the device is
>> marked as read-only.
>>
>> Based on generic/050.
>>
>> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
>> ---
>>  tests/xfs/837     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/837.out | 10 +++++++++
>>  2 files changed, 65 insertions(+)
>>  create mode 100755 tests/xfs/837
>>  create mode 100644 tests/xfs/837.out
>>
>> diff --git a/tests/xfs/837 b/tests/xfs/837
>> new file mode 100755
>> index 000000000000..b20e9c5f33f2
>> --- /dev/null
>> +++ b/tests/xfs/837
>> @@ -0,0 +1,55 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2009 Christoph Hellwig.
>> +# Copyright (c) 2025 Western Digital Corporation
>> +#
>> +# FS QA Test No. 837
>> +#
>> +# Check out various mount/remount/unmount scenarious on a read-only rtdev
>> +# Based on generic/050
>> +#
>> +. ./common/preamble
>> +_begin_fstest mount auto quick
>> +
>> +_cleanup_setrw()
>> +{
>> +	cd /
>> +	blockdev --setrw $SCRATCH_RTDEV
>> +}
>> +
>> +# Import common functions.
>> +. ./common/filter
>> +
>> +_require_realtime
>> +_require_local_device $SCRATCH_RTDEV
> 
> I suspect this is copy-pasted from generic/050, but I wonder when
> SCRATCH_RTDEV could be a character device, but maybe that's a relic of
> Irix (and Solaris too, IIRC)?

Yeah, this was carried over from generic/050, and I can just drop it
unless there is a good reason for keeping it?

<pardon replying again Darrick, forgot to reply-all yesterday>

> 
> The rest of the test looks fine to me though.
> 
> --D
> 
>> +_register_cleanup "_cleanup_setrw"
>> +
>> +_scratch_mkfs "-d rtinherit" > /dev/null 2>&1
>> +
>> +#
>> +# Mark the rt device read-only.
>> +#
>> +echo "setting device read-only"
>> +blockdev --setro $SCRATCH_RTDEV
>> +
>> +#
>> +# Mount it and make sure it can't be written to.
>> +#
>> +echo "mounting read-only rt block device:"
>> +_scratch_mount 2>&1 | _filter_ro_mount | _filter_scratch
>> +if [ "${PIPESTATUS[0]}" -eq 0 ]; then
>> +	echo "writing to file on read-only filesystem:"
>> +	dd if=/dev/zero of=$SCRATCH_MNT/foo bs=1M count=1 oflag=direct 2>&1 | _filter_scratch
>> +else
>> +	_fail "failed to mount"
>> +fi
>> +
>> +echo "remounting read-write:"
>> +_scratch_remount rw 2>&1 | _filter_scratch | _filter_ro_mount
>> +
>> +echo "unmounting read-only filesystem"
>> +_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
>> +
>> +# success, all done
>> +echo "*** done"
>> +status=0
>> diff --git a/tests/xfs/837.out b/tests/xfs/837.out
>> new file mode 100644
>> index 000000000000..0a843a0ba398
>> --- /dev/null
>> +++ b/tests/xfs/837.out
>> @@ -0,0 +1,10 @@
>> +QA output created by 837
>> +setting device read-only
>> +mounting read-only rt block device:
>> +mount: device write-protected, mounting read-only
>> +writing to file on read-only filesystem:
>> +dd: failed to open 'SCRATCH_MNT/foo': Read-only file system
>> +remounting read-write:
>> +mount: cannot remount device read-write, is write-protected
>> +unmounting read-only filesystem
>> +*** done
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH 1/2] xfs: add mount test for read only rt devices

[Darrick J. Wong]

On Tue, Apr 29, 2025 at 12:03:20PM +0000, Hans Holmberg wrote:
> On 25/04/2025 17:04, Darrick J. Wong wrote:
> > On Fri, Apr 25, 2025 at 09:03:22AM +0000, Hans Holmberg wrote:
> >> Make sure that we can mount rt devices read-only if them themselves
> >> are marked as read-only.
> >>
> >> Also make sure that rw re-mounts are not allowed if the device is
> >> marked as read-only.
> >>
> >> Based on generic/050.
> >>
> >> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> >> ---
> >>  tests/xfs/837     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/xfs/837.out | 10 +++++++++
> >>  2 files changed, 65 insertions(+)
> >>  create mode 100755 tests/xfs/837
> >>  create mode 100644 tests/xfs/837.out
> >>
> >> diff --git a/tests/xfs/837 b/tests/xfs/837
> >> new file mode 100755
> >> index 000000000000..b20e9c5f33f2
> >> --- /dev/null
> >> +++ b/tests/xfs/837
> >> @@ -0,0 +1,55 @@
> >> +#! /bin/bash
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (c) 2009 Christoph Hellwig.
> >> +# Copyright (c) 2025 Western Digital Corporation
> >> +#
> >> +# FS QA Test No. 837
> >> +#
> >> +# Check out various mount/remount/unmount scenarious on a read-only rtdev
> >> +# Based on generic/050
> >> +#
> >> +. ./common/preamble
> >> +_begin_fstest mount auto quick
> >> +
> >> +_cleanup_setrw()
> >> +{
> >> +	cd /
> >> +	blockdev --setrw $SCRATCH_RTDEV
> >> +}
> >> +
> >> +# Import common functions.
> >> +. ./common/filter
> >> +
> >> +_require_realtime
> >> +_require_local_device $SCRATCH_RTDEV
> > 
> > I suspect this is copy-pasted from generic/050, but I wonder when
> > SCRATCH_RTDEV could be a character device, but maybe that's a relic of
> > Irix (and Solaris too, IIRC)?
> 
> Yeah, this was carried over from generic/050, and I can just drop it
> unless there is a good reason for keeping it?

I think you still need the _require_local_device call itself.

And come to think of it, weren't there supposed to be pmem filesystems
which would run entirely off a pmem character device and not have a
block interface available at all?

Though for an xfs-specific test we'll blow up pretty fast if
SCRATCH_RTDEV isn't a bdev so <shrug>.

> <pardon replying again Darrick, forgot to reply-all yesterday>

(I seriously didn't even notice.)

--D

> > 
> > The rest of the test looks fine to me though.
> > 
> > --D
> > 
> >> +_register_cleanup "_cleanup_setrw"
> >> +
> >> +_scratch_mkfs "-d rtinherit" > /dev/null 2>&1
> >> +
> >> +#
> >> +# Mark the rt device read-only.
> >> +#
> >> +echo "setting device read-only"
> >> +blockdev --setro $SCRATCH_RTDEV
> >> +
> >> +#
> >> +# Mount it and make sure it can't be written to.
> >> +#
> >> +echo "mounting read-only rt block device:"
> >> +_scratch_mount 2>&1 | _filter_ro_mount | _filter_scratch
> >> +if [ "${PIPESTATUS[0]}" -eq 0 ]; then
> >> +	echo "writing to file on read-only filesystem:"
> >> +	dd if=/dev/zero of=$SCRATCH_MNT/foo bs=1M count=1 oflag=direct 2>&1 | _filter_scratch
> >> +else
> >> +	_fail "failed to mount"
> >> +fi
> >> +
> >> +echo "remounting read-write:"
> >> +_scratch_remount rw 2>&1 | _filter_scratch | _filter_ro_mount
> >> +
> >> +echo "unmounting read-only filesystem"
> >> +_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
> >> +
> >> +# success, all done
> >> +echo "*** done"
> >> +status=0
> >> diff --git a/tests/xfs/837.out b/tests/xfs/837.out
> >> new file mode 100644
> >> index 000000000000..0a843a0ba398
> >> --- /dev/null
> >> +++ b/tests/xfs/837.out
> >> @@ -0,0 +1,10 @@
> >> +QA output created by 837
> >> +setting device read-only
> >> +mounting read-only rt block device:
> >> +mount: device write-protected, mounting read-only
> >> +writing to file on read-only filesystem:
> >> +dd: failed to open 'SCRATCH_MNT/foo': Read-only file system
> >> +remounting read-write:
> >> +mount: cannot remount device read-write, is write-protected
> >> +unmounting read-only filesystem
> >> +*** done
> >> -- 
> >> 2.34.1
> >>
> > 
> 

Re: [PATCH 1/2] xfs: add mount test for read only rt devices

[hch]

On Tue, Apr 29, 2025 at 07:52:21AM -0700, Darrick J. Wong wrote:
> And come to think of it, weren't there supposed to be pmem filesystems
> which would run entirely off a pmem character device and not have a
> block interface available at all?

That support never materialized, and if it at some does it will need a
few changes all over xfstets, so no need to worry now.

Re: [PATCH 1/2] xfs: add mount test for read only rt devices

[Darrick J. Wong]

On Wed, Apr 30, 2025 at 02:56:18PM +0200, hch wrote:
> On Tue, Apr 29, 2025 at 07:52:21AM -0700, Darrick J. Wong wrote:
> > And come to think of it, weren't there supposed to be pmem filesystems
> > which would run entirely off a pmem character device and not have a
> > block interface available at all?
> 
> That support never materialized, and if it at some does it will need a
> few changes all over xfstets, so no need to worry now.

Heh, ok.  I think I'm ok with this test (which specifically pokes at rt
devices) now, so

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

Re: [PATCH 1/2] xfs: add mount test for read only rt devices

[Hans Holmberg]

On 30/04/2025 16:49, Darrick J. Wong wrote:
> On Wed, Apr 30, 2025 at 02:56:18PM +0200, hch wrote:
>> On Tue, Apr 29, 2025 at 07:52:21AM -0700, Darrick J. Wong wrote:
>>> And come to think of it, weren't there supposed to be pmem filesystems
>>> which would run entirely off a pmem character device and not have a
>>> block interface available at all?
>>
>> That support never materialized, and if it at some does it will need a
>> few changes all over xfstets, so no need to worry now.
> 
> Heh, ok.  I think I'm ok with this test (which specifically pokes at rt
> devices) now, so
> 
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
> 

I just realized that we need to _notrun this test when we have internal
rt devices, so i'll send a v2 separately.

Re: [PATCH 1/2] xfs: add mount test for read only rt devices

[Hans Holmberg]

On 07/05/2025 10:57, Hans Holmberg wrote:
> On 30/04/2025 16:49, Darrick J. Wong wrote:
>> On Wed, Apr 30, 2025 at 02:56:18PM +0200, hch wrote:
>>> On Tue, Apr 29, 2025 at 07:52:21AM -0700, Darrick J. Wong wrote:
>>>> And come to think of it, weren't there supposed to be pmem filesystems
>>>> which would run entirely off a pmem character device and not have a
>>>> block interface available at all?
>>>
>>> That support never materialized, and if it at some does it will need a
>>> few changes all over xfstets, so no need to worry now.
>>
>> Heh, ok.  I think I'm ok with this test (which specifically pokes at rt
>> devices) now, so
>>
>> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
>>
>> --D
>>
>>
> 
> I just realized that we need to _notrun this test when we have internal
> rt devices, so i'll send a v2 separately.
> 

Please ignore my confusion here. We have _require_local_device for the
scratch rt dev already in the test so we're good.

[PATCH 2/2] xfs: add mount test for read only log devices

[Hans Holmberg]

Make sure that we can mount log devices read-only if them themselves
are marked as read-only.

Also make sure that rw re-mounts are not allowed if the device is
marked as read-only.

Based on generic/050.

Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
---
 tests/xfs/838     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/838.out | 10 +++++++++
 2 files changed, 65 insertions(+)
 create mode 100755 tests/xfs/838
 create mode 100644 tests/xfs/838.out

diff --git a/tests/xfs/838 b/tests/xfs/838
new file mode 100755
index 000000000000..93a39a7ec8e9
--- /dev/null
+++ b/tests/xfs/838
@@ -0,0 +1,55 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2009 Christoph Hellwig.
+# Copyright (c) 2025 Western Digital Corporation.
+#
+# FS QA Test No. 838
+#
+# Check out various mount/remount/unmount scenarious on a read-only logdev
+# Based on generic/050
+#
+. ./common/preamble
+_begin_fstest mount auto quick
+
+_cleanup_setrw()
+{
+	cd /
+	blockdev --setrw $SCRATCH_LOGDEV
+}
+
+# Import common functions.
+. ./common/filter
+
+_require_logdev
+_require_local_device $SCRATCH_LOGDEV
+_register_cleanup "_cleanup_setrw"
+
+_scratch_mkfs >/dev/null 2>&1
+
+#
+# Mark the log device read-only
+#
+echo "setting device read-only"
+blockdev --setro $SCRATCH_LOGDEV
+
+#
+# Mount and make sure it can't be written to.
+#
+echo "mounting read-only log block device:"
+_scratch_mount 2>&1 | _filter_ro_mount | _filter_scratch
+if [ "${PIPESTATUS[0]}" -eq 0 ]; then
+	echo "writing to file on read-only filesystem:"
+	dd if=/dev/zero of=$SCRATCH_MNT/foo bs=1M count=1 oflag=direct 2>&1 | _filter_scratch
+else
+	_fail "failed to mount"
+fi
+
+echo "remounting read-write:"
+_scratch_remount rw 2>&1 | _filter_scratch | _filter_ro_mount
+
+echo "unmounting read-only filesystem"
+_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
+
+# success, all done
+echo "*** done"
+status=0
diff --git a/tests/xfs/838.out b/tests/xfs/838.out
new file mode 100644
index 000000000000..673b48f42a4e
--- /dev/null
+++ b/tests/xfs/838.out
@@ -0,0 +1,10 @@
+QA output created by 838
+setting device read-only
+mounting read-only log block device:
+mount: device write-protected, mounting read-only
+writing to file on read-only filesystem:
+dd: failed to open 'SCRATCH_MNT/foo': Read-only file system
+remounting read-write:
+mount: cannot remount device read-write, is write-protected
+unmounting read-only filesystem
+*** done
-- 
2.34.1

Re: [PATCH 2/2] xfs: add mount test for read only log devices

[Darrick J. Wong]

On Fri, Apr 25, 2025 at 09:03:23AM +0000, Hans Holmberg wrote:
> Make sure that we can mount log devices read-only if them themselves
> are marked as read-only.
> 
> Also make sure that rw re-mounts are not allowed if the device is
> marked as read-only.
> 
> Based on generic/050.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@wdc.com>
> ---
>  tests/xfs/838     | 55 +++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/838.out | 10 +++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100755 tests/xfs/838
>  create mode 100644 tests/xfs/838.out
> 
> diff --git a/tests/xfs/838 b/tests/xfs/838
> new file mode 100755
> index 000000000000..93a39a7ec8e9
> --- /dev/null
> +++ b/tests/xfs/838
> @@ -0,0 +1,55 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2009 Christoph Hellwig.
> +# Copyright (c) 2025 Western Digital Corporation.
> +#
> +# FS QA Test No. 838
> +#
> +# Check out various mount/remount/unmount scenarious on a read-only logdev
> +# Based on generic/050
> +#
> +. ./common/preamble
> +_begin_fstest mount auto quick
> +
> +_cleanup_setrw()
> +{
> +	cd /
> +	blockdev --setrw $SCRATCH_LOGDEV
> +}
> +
> +# Import common functions.
> +. ./common/filter
> +
> +_require_logdev
> +_require_local_device $SCRATCH_LOGDEV
> +_register_cleanup "_cleanup_setrw"
> +
> +_scratch_mkfs >/dev/null 2>&1
> +
> +#
> +# Mark the log device read-only
> +#
> +echo "setting device read-only"
> +blockdev --setro $SCRATCH_LOGDEV
> +
> +#
> +# Mount and make sure it can't be written to.
> +#
> +echo "mounting read-only log block device:"
> +_scratch_mount 2>&1 | _filter_ro_mount | _filter_scratch
> +if [ "${PIPESTATUS[0]}" -eq 0 ]; then
> +	echo "writing to file on read-only filesystem:"
> +	dd if=/dev/zero of=$SCRATCH_MNT/foo bs=1M count=1 oflag=direct 2>&1 | _filter_scratch

As I mentioned over in the other thread, perhaps this test should check
that a readonly log device results in a norecovery mount and that
pending changes don't show up if the mount succeeds?

Also, ext4 supports external log devices, should this be in
tests/generic?

--D

> +else
> +	_fail "failed to mount"
> +fi
> +
> +echo "remounting read-write:"
> +_scratch_remount rw 2>&1 | _filter_scratch | _filter_ro_mount
> +
> +echo "unmounting read-only filesystem"
> +_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
> +
> +# success, all done
> +echo "*** done"
> +status=0
> diff --git a/tests/xfs/838.out b/tests/xfs/838.out
> new file mode 100644
> index 000000000000..673b48f42a4e
> --- /dev/null
> +++ b/tests/xfs/838.out
> @@ -0,0 +1,10 @@
> +QA output created by 838
> +setting device read-only
> +mounting read-only log block device:
> +mount: device write-protected, mounting read-only
> +writing to file on read-only filesystem:
> +dd: failed to open 'SCRATCH_MNT/foo': Read-only file system
> +remounting read-write:
> +mount: cannot remount device read-write, is write-protected
> +unmounting read-only filesystem
> +*** done
> -- 
> 2.34.1
> 

Re: [PATCH 2/2] xfs: add mount test for read only log devices

[Hans Holmberg]

On 25/04/2025 17:05, Darrick J. Wong wrote:
> ps this test should check
> that a readonly log device results in a norecovery mount and that
> pending changes don't show up if the mount succeeds?
> 
> Also, ext4 supports external log devices, should this be in
> tests/generic?

Doh!, actually ext4 has a test for this already, ext4/002
(also based on generic/050)

With my fix, ext4/002 passes for xfs Should/can we turn that into a
generic test?

The test makes sure that a filesystem will mount ro,norecovery if
the log device is ro but does not do any real checks if recovery is
prevented (or done once the log device is rw again).
That could be added though.

Re: [PATCH 2/2] xfs: add mount test for read only log devices

[Darrick J. Wong]

On Mon, Apr 28, 2025 at 12:16:34PM +0000, Hans Holmberg wrote:
> On 25/04/2025 17:05, Darrick J. Wong wrote:
> > ps this test should check
> > that a readonly log device results in a norecovery mount and that
> > pending changes don't show up if the mount succeeds?
> > 
> > Also, ext4 supports external log devices, should this be in
> > tests/generic?
> 
> Doh!, actually ext4 has a test for this already, ext4/002
> (also based on generic/050)
> 
> With my fix, ext4/002 passes for xfs Should/can we turn that into a
> generic test?

Yeah, it looks like ext4/002 already does most of what you want.  Though
I'd amend it to check that SCRATCH_MNT/00-99 aren't visible in the
norecovery mounts and only appear after recovery actually runs.

> The test makes sure that a filesystem will mount ro,norecovery if
> the log device is ro but does not do any real checks if recovery is
> prevented (or done once the log device is rw again).
> That could be added though.

<nod>

--D

Re: [PATCH 2/2] xfs: add mount test for read only log devices

[Hans Holmberg]

On 28/04/2025 17:58, Darrick J. Wong wrote:
> On Mon, Apr 28, 2025 at 12:16:34PM +0000, Hans Holmberg wrote:
>> On 25/04/2025 17:05, Darrick J. Wong wrote:
>>> ps this test should check
>>> that a readonly log device results in a norecovery mount and that
>>> pending changes don't show up if the mount succeeds?
>>>
>>> Also, ext4 supports external log devices, should this be in
>>> tests/generic?
>>
>> Doh!, actually ext4 has a test for this already, ext4/002
>> (also based on generic/050)
>>
>> With my fix, ext4/002 passes for xfs Should/can we turn that into a
>> generic test?
> 
> Yeah, it looks like ext4/002 already does most of what you want.  Though
> I'd amend it to check that SCRATCH_MNT/00-99 aren't visible in the
> norecovery mounts and only appear after recovery actually runs.
> 

So I added this check to ext4/002 and while this works for xfs - the
touched files are not visible until log recovery has completed, it does
not for ext3/4.

For ext3/4 the files are visible after the first successful (norecovery)
mount, so even though we did a shutdown, a log recovery does not seem
required (dmesg tells me that the log recovery is done in the end after
the log device is set back to rw)

..and I presume this is fine - for a generic test can we really assume
that a log recovery is required to see the files?

Re: [PATCH 2/2] xfs: add mount test for read only log devices

[Darrick J. Wong]

On Wed, Apr 30, 2025 at 08:26:00AM +0000, Hans Holmberg wrote:
> On 28/04/2025 17:58, Darrick J. Wong wrote:
> > On Mon, Apr 28, 2025 at 12:16:34PM +0000, Hans Holmberg wrote:
> >> On 25/04/2025 17:05, Darrick J. Wong wrote:
> >>> ps this test should check
> >>> that a readonly log device results in a norecovery mount and that
> >>> pending changes don't show up if the mount succeeds?
> >>>
> >>> Also, ext4 supports external log devices, should this be in
> >>> tests/generic?
> >>
> >> Doh!, actually ext4 has a test for this already, ext4/002
> >> (also based on generic/050)
> >>
> >> With my fix, ext4/002 passes for xfs Should/can we turn that into a
> >> generic test?
> > 
> > Yeah, it looks like ext4/002 already does most of what you want.  Though
> > I'd amend it to check that SCRATCH_MNT/00-99 aren't visible in the
> > norecovery mounts and only appear after recovery actually runs.
> > 
> 
> So I added this check to ext4/002 and while this works for xfs - the
> touched files are not visible until log recovery has completed, it does
> not for ext3/4.
> 
> For ext3/4 the files are visible after the first successful (norecovery)
> mount, so even though we did a shutdown, a log recovery does not seem
> required (dmesg tells me that the log recovery is done in the end after
> the log device is set back to rw)
> 
> ..and I presume this is fine - for a generic test can we really assume
> that a log recovery is required to see the files?

Nope.  I guess that's an implementation dependent behavior.  TBH I'm not
even sure we can 100% rely on it for xfs, since it's theoretically
possible for the log to flush and checkpoint in the very small window
between the creat and the shutdown call.

If hoisting ext4/002 to generic works for the three main filesystems
then I'm fine with just doing that without the extra tests.

--D

Re: [PATCH 2/2] xfs: add mount test for read only log devices

[Hans Holmberg]

On 30/04/2025 16:52, Darrick J. Wong wrote:
> On Wed, Apr 30, 2025 at 08:26:00AM +0000, Hans Holmberg wrote:
>> On 28/04/2025 17:58, Darrick J. Wong wrote:
>>> On Mon, Apr 28, 2025 at 12:16:34PM +0000, Hans Holmberg wrote:
>>>> On 25/04/2025 17:05, Darrick J. Wong wrote:
>>>>> ps this test should check
>>>>> that a readonly log device results in a norecovery mount and that
>>>>> pending changes don't show up if the mount succeeds?
>>>>>
>>>>> Also, ext4 supports external log devices, should this be in
>>>>> tests/generic?
>>>>
>>>> Doh!, actually ext4 has a test for this already, ext4/002
>>>> (also based on generic/050)
>>>>
>>>> With my fix, ext4/002 passes for xfs Should/can we turn that into a
>>>> generic test?
>>>
>>> Yeah, it looks like ext4/002 already does most of what you want.  Though
>>> I'd amend it to check that SCRATCH_MNT/00-99 aren't visible in the
>>> norecovery mounts and only appear after recovery actually runs.
>>>
>>
>> So I added this check to ext4/002 and while this works for xfs - the
>> touched files are not visible until log recovery has completed, it does
>> not for ext3/4.
>>
>> For ext3/4 the files are visible after the first successful (norecovery)
>> mount, so even though we did a shutdown, a log recovery does not seem
>> required (dmesg tells me that the log recovery is done in the end after
>> the log device is set back to rw)
>>
>> ..and I presume this is fine - for a generic test can we really assume
>> that a log recovery is required to see the files?
> 
> Nope.  I guess that's an implementation dependent behavior.  TBH I'm not
> even sure we can 100% rely on it for xfs, since it's theoretically
> possible for the log to flush and checkpoint in the very small window
> between the creat and the shutdown call.
> 
> If hoisting ext4/002 to generic works for the three main filesystems
> then I'm fine with just doing that without the extra tests.

Cool, I've submitted a patch to turn ext4 generic.