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

["Darrick J. Wong" <djwong@kernel.org>]

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