[PATCH v3 0/3] fix failures caused by mount error msg change in util-linux v2.30

[PATCH v3 0/3] fix failures caused by mount error msg change in util-linux v2.30

[Eryu Guan]

util-linux v2.30 changed mount error message again and caused several
tests to fail (reported by Misono Tomohiro, thanks!), this is mainly due
to two util-linux commits in v2.30-rc1

6dede2f2f7c5 libmount: support MS_RDONLY on write-protected devices
ea848180dd34 libmount: add mnt_context_get_excode()

This patchset introduced _filter_ending_dot, _filter_error_mount and
_filter_busy_mount and improved _filter_ro_mount to deal with all these
output differences between old and new util-linux versions.

I've tested and confirmed generic/050, xfs/005 and overlay/03[567] all
passed with both old and new util-linux. xfs/333 is always _notrun
because there's no realtime-rmapbt support yet.

Thanks,
Eryu

v3:
- address Amir's review comments, detailed change log goes to each patch
v2:
- remove not-needed _filter_scratch from tests

Eryu Guan (3):
  fstests: filter mount error message for EUCLEAN and ESTALE
  overlay/036: filter busy mount message
  fstests: filter readonly mount error messages

 common/filter         | 86 +++++++++++++++++++++++++++++++++++++++++++++++++--
 tests/generic/050     |  8 ++---
 tests/generic/050.out |  8 ++---
 tests/overlay/035     |  4 +--
 tests/overlay/035.out |  4 +--
 tests/overlay/036     |  4 +--
 tests/overlay/036.out |  4 +--
 tests/overlay/037     |  4 +--
 tests/overlay/037.out |  4 +--
 tests/xfs/005         |  7 +----
 tests/xfs/333         |  2 +-
 tests/xfs/333.out     |  2 +-
 12 files changed, 107 insertions(+), 30 deletions(-)

-- 
2.14.3

[PATCH v3 1/3] fstests: filter mount error message for EUCLEAN and ESTALE

[Eryu Guan]

util-linux commit ea848180dd34 ("libmount: add
mnt_context_get_excode()") since v2.30 changed the error message on
EUCLEAN and ESTALE again (and maybe other errno too):

 - mount: <device> on <mountpoint> failed: Structure needs cleaning
 + mount: <mountpoint>: mount(2) system call failed: Structure needs cleaning.

and it causes xfs/005, overlay/037 to fail (and probably xfs/333 too,
but it's always _notrun for now).

And what's more, the mentioned tests would also fail when testing
with util-linux prior to v2.21, no one complained just because the
tests are usually _notrun on such old distributions that ship
util-linux < v2.21.

So let's filter out the changing parts and keep the error message
simple.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
v3:
- update commit log to mention potential test failure using
  util-linux < v2.21
- document the filtered format as comments

 common/filter         | 26 ++++++++++++++++++++++++++
 tests/overlay/037     |  4 ++--
 tests/overlay/037.out |  4 ++--
 tests/xfs/005         |  7 +------
 tests/xfs/333         |  2 +-
 tests/xfs/333.out     |  2 +-
 6 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/common/filter b/common/filter
index 0cb458b0051f..0d3875cc6cdc 100644
--- a/common/filter
+++ b/common/filter
@@ -390,6 +390,13 @@ _filter_fstrim()
 	egrep -o "[0-9]+ bytes" | $AWK_PROG '{print $1}'
 }
 
+# Remove the ending dot appended to mount error message, util-linux 2.30
+# starts to do so.
+_filter_ending_dot()
+{
+	sed -e "s/\.$//"
+}
+
 # Older mount output referred to "block device" when mounting RO devices
 # It's gone in newer versions
 _filter_ro_mount() {
@@ -397,6 +404,25 @@ _filter_ro_mount() {
 	    -e "s/mount: cannot mount block device/mount: cannot mount/g"
 }
 
+# Filter a failed mount output due to EUCLEAN and USTALE, util-linux changed
+# the message several times.
+#
+# prior to v2.21:
+# mount: Structure needs cleaning
+# v2.21 to v2.29:
+# mount: mount <device> on <mountpoint> failed: Structure needs cleaning
+# v2.30 and later:
+# mount: <mountpoint>: mount(2) system call failed: Structure needs cleaning.
+#
+# This is also true for ESTALE error. So let's remove all the changing parts
+# and keep the 'prior to v2.21' format:
+# mount: Structure needs cleaning
+# mount: Stale file handle
+_filter_error_mount()
+{
+	sed -e "s/mount:\(.*failed:\)/mount:/" | _filter_ending_dot
+}
+
 _filter_od()
 {
 	BLOCK_SIZE=$(_get_block_size $SCRATCH_MNT)
diff --git a/tests/overlay/037 b/tests/overlay/037
index 728732903358..6710ddaf4802 100755
--- a/tests/overlay/037
+++ b/tests/overlay/037
@@ -76,12 +76,12 @@ $UMOUNT_PROG $SCRATCH_MNT
 
 # Try to mount an overlay with the same upperdir and different lowerdir - expect ESTALE
 _overlay_scratch_mount_dirs $lowerdir2 $upperdir $workdir -oindex=on \
-	2>&1 | _filter_scratch
+	2>&1 | _filter_error_mount
 $UMOUNT_PROG $SCRATCH_MNT 2>/dev/null
 
 # Try to mount an overlay with the same workdir and different upperdir - expect ESTALE
 _overlay_scratch_mount_dirs $lowerdir $upperdir2 $workdir -oindex=on \
-	2>&1 | _filter_scratch
+	2>&1 | _filter_error_mount
 $UMOUNT_PROG $SCRATCH_MNT 2>/dev/null
 
 # Mount overlay with original lowerdir, upperdir, workdir and index=on - expect success
diff --git a/tests/overlay/037.out b/tests/overlay/037.out
index 786ef85c29e3..d6ad7cfad687 100644
--- a/tests/overlay/037.out
+++ b/tests/overlay/037.out
@@ -1,3 +1,3 @@
 QA output created by 037
-mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Stale file handle
-mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Stale file handle
+mount: Stale file handle
+mount: Stale file handle
diff --git a/tests/xfs/005 b/tests/xfs/005
index fade4bbf282b..ebf4b15ec9b5 100755
--- a/tests/xfs/005
+++ b/tests/xfs/005
@@ -40,11 +40,6 @@ _cleanup()
     rm -f $tmp.*
 }
 
-filter_mount()
-{
-	sed -e "s/mount .* failed: //"
-}
-
 # get standard environment, filters and checks
 . ./common/rc
 . ./common/filter
@@ -64,7 +59,7 @@ _scratch_mkfs_xfs -m crc=1 >> $seqres.full 2>&1 || _fail "mkfs failed"
 $XFS_IO_PROG -c "pwrite 224 4" -c fsync $SCRATCH_DEV | _filter_xfs_io
 
 # should FAIL, the crc is bad; golden output contains mount failure
-_scratch_mount 2>&1 | filter_mount
+_scratch_mount 2>&1 | _filter_error_mount
 
 # success, all done
 status=0
diff --git a/tests/xfs/333 b/tests/xfs/333
index bf0c811d2435..2f394feda6c9 100755
--- a/tests/xfs/333
+++ b/tests/xfs/333
@@ -64,7 +64,7 @@ _scratch_unmount
 
 echo "Corrupt fs"
 _scratch_xfs_db -x -c 'sb 0' -c "write rrmapino $ino" >> $seqres.full
-_scratch_mount 2>&1 | _filter_scratch
+_scratch_mount 2>&1 | _filter_error_mount
 
 echo "Test done, mount should have failed"
 
diff --git a/tests/xfs/333.out b/tests/xfs/333.out
index f7518f46d900..b3c698750f8f 100644
--- a/tests/xfs/333.out
+++ b/tests/xfs/333.out
@@ -2,5 +2,5 @@ QA output created by 333
 Format and mount
 Create some files
 Corrupt fs
-mount: mount SCRATCH_DEV on SCRATCH_MNT failed: Structure needs cleaning
+mount: Structure needs cleaning
 Test done, mount should have failed
-- 
2.14.3

[PATCH v3 2/3] overlay/036: filter busy mount message

[Eryu Guan]

util-linux v2.30 changed error message of a busy mount and caused
overlay/036 to fail. e.g.

 - mount: <device> is already mounted or <mountpoint> busy
 + mount: <mountpoint>: <device> already mounted or mount point busy.

Filter the mount output by a newly introduced _filter_busy_mount
into a unified format.

   mount: device already mounted or mount point busy

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
v3:
- filter out $SCRATCH_DEV/MNT too and use a consistent output
- update regexp in sed as suggested by Amir

 common/filter         | 12 ++++++++++++
 tests/overlay/036     |  4 ++--
 tests/overlay/036.out |  4 ++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/common/filter b/common/filter
index 0d3875cc6cdc..a212c09aa138 100644
--- a/common/filter
+++ b/common/filter
@@ -423,6 +423,18 @@ _filter_error_mount()
 	sed -e "s/mount:\(.*failed:\)/mount:/" | _filter_ending_dot
 }
 
+# Similar to _filter_error_mount, filter a busy mount output.
+# Turn both old (prior to util-linux v2.30) and new (v2.30 and later) format to
+# a simple one. e.g.
+# old: mount: <device> is already mounted or <mountpoint> busy
+# new: mount: <mountpoint>: <device> already mounted or mount point busy.
+# filtered: mount: device already mounted or mount point busy
+_filter_busy_mount()
+{
+	sed -e "s/.*: .* already mounted or .* busy/mount: device already mounted or mount point busy/" | \
+		_filter_ending_dot
+}
+
 _filter_od()
 {
 	BLOCK_SIZE=$(_get_block_size $SCRATCH_MNT)
diff --git a/tests/overlay/036 b/tests/overlay/036
index 544d4e4eaa87..e0c13ae88c92 100755
--- a/tests/overlay/036
+++ b/tests/overlay/036
@@ -103,12 +103,12 @@ _overlay_mount_dirs $lowerdir2 $upperdir2 $workdir \
 # Try to mount another overlay with the same upperdir
 # with index=on - expect EBUSY
 _overlay_mount_dirs $lowerdir2 $upperdir $workdir2 \
-		    overlay2 $SCRATCH_MNT -oindex=on 2>&1 | _filter_scratch
+	    overlay2 $SCRATCH_MNT -oindex=on 2>&1 | _filter_busy_mount
 
 # Try to mount another overlay with the same workdir
 # with index=on - expect EBUSY
 _overlay_mount_dirs $lowerdir2 $upperdir2 $workdir \
-		    overlay3 $SCRATCH_MNT -oindex=on 2>&1 | _filter_scratch
+	    overlay3 $SCRATCH_MNT -oindex=on 2>&1 | _filter_busy_mount
 
 
 # success, all done
diff --git a/tests/overlay/036.out b/tests/overlay/036.out
index 51746114db1c..0d266f1b216e 100644
--- a/tests/overlay/036.out
+++ b/tests/overlay/036.out
@@ -1,3 +1,3 @@
 QA output created by 036
-mount: overlay2 is already mounted or SCRATCH_MNT busy
-mount: overlay3 is already mounted or SCRATCH_MNT busy
+mount: device already mounted or mount point busy
+mount: device already mounted or mount point busy
-- 
2.14.3

[PATCH v3 3/3] fstests: filter readonly mount error messages

[Eryu Guan]

util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
write-protected devices") changed the error message on read-only
block device, and in the failure case printed one line message
instead of two (for details please see comments in common/filter),
and this change broke generic/050 and overlay/035.

Fix it by adding more filter rules to _filter_ro_mount and updating
associated .out files to unify the output from both old and new
util-linux versions.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
v3:
- document the filtered format in comments
- remove legacy sed filter, the perl filter covers the legacy case well
- filter out $SCRATCH_DEV/MNT too and use a consistent output
- remove the new filter_mount helper in overlay/035

 common/filter         | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 tests/generic/050     |  8 ++++----
 tests/generic/050.out |  8 ++++----
 tests/overlay/035     |  4 ++--
 tests/overlay/035.out |  4 ++--
 5 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/common/filter b/common/filter
index a212c09aa138..6140b331017f 100644
--- a/common/filter
+++ b/common/filter
@@ -399,9 +399,53 @@ _filter_ending_dot()
 
 # Older mount output referred to "block device" when mounting RO devices
 # It's gone in newer versions
+#
+# And util-linux v2.30 changed the output again, e.g.
+# for a successful ro mount:
+# prior to v2.30:  mount: <device> is write-protected, mounting read-only
+# v2.30 and later: mount: <mountpoint>: WARNING: device write-protected, mounted read-only.
+#
+# a failed ro mount:
+# prior to v2.30:
+# mount: <device> is write-protected, mounting read-only
+# mount: cannot mount <device> read-only
+# v2.30 and later:
+# mount: <mountpoint>: cannot mount <device> read-only.
+#
+# a failed rw remount:
+# prior to v2.30:  mount: cannot remount <device> read-write, is write-protected
+# v2.30 and later: mount: <mountpoint>: cannot remount <device> read-write, is write-protected.
+#
+# Now use _filter_ro_mount to unify all these differences across old & new
+# util-linux versions. So the filtered format would be:
+#
+# successful ro mount:
+# mount: device write-protected, mounting read-only
+#
+# failed ro mount:
+# mount: device write-protected, mounting read-only
+# mount: cannot mount read-only
+#
+# failed rw remount:
+# mount: cannot remount device read-write, is write-protected
 _filter_ro_mount() {
-	sed -e "s/mount: block device/mount:/g" \
-	    -e "s/mount: cannot mount block device/mount: cannot mount/g"
+	perl -ne '
+	if (/write-protected, mount.*read-only/) {
+		# successful ro mount
+		print "mount: device write-protected, mounting read-only\n";
+	} elsif (/mount: .*: cannot mount.*read-only/) {
+		# filter v2.30 format failed ro mount
+		print "mount: device write-protected, mounting read-only\n";
+		print "mount: cannot mount read-only\n";
+	} elsif (/(^mount: cannot mount) .* (read-only$)/) {
+		# filter prior to v2.30 format failed ro mount
+		print "$1 $2\n";
+	} elsif (/mount:.* cannot remount .* read-write.*/) {
+		# failed rw remount
+		print "mount: cannot remount device read-write, is write-protected\n";
+	} else {
+		print "$_";
+	}' | _filter_ending_dot
 }
 
 # Filter a failed mount output due to EUCLEAN and USTALE, util-linux changed
diff --git a/tests/generic/050 b/tests/generic/050
index 5fa28a7648e5..efa45f04825b 100755
--- a/tests/generic/050
+++ b/tests/generic/050
@@ -60,7 +60,7 @@ blockdev --setro $SCRATCH_DEV
 # Mount it, and make sure we can't write to it, and we can unmount it again
 #
 echo "mounting read-only block device:"
-_scratch_mount 2>&1 | _filter_scratch | _filter_ro_mount
+_scratch_mount 2>&1 | _filter_ro_mount
 
 echo "touching file on read-only filesystem (should fail)"
 touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
@@ -95,10 +95,10 @@ blockdev --setro $SCRATCH_DEV
 # -o norecovery is used.
 #
 echo "mounting filesystem that needs recovery on a read-only device:"
-_scratch_mount 2>&1 | _filter_scratch | _filter_ro_mount
+_scratch_mount 2>&1 | _filter_ro_mount
 
 echo "unmounting read-only filesystem"
-_scratch_unmount 2>&1 | _filter_scratch
+_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
 
 #
 # This is the way out if the underlying device really is read-only.
@@ -106,7 +106,7 @@ _scratch_unmount 2>&1 | _filter_scratch
 # data recovery hack.
 #
 echo "mounting filesystem with -o norecovery on a read-only device:"
-_scratch_mount -o norecovery 2>&1 | _filter_scratch | _filter_ro_mount
+_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
 
 echo "unmounting read-only filesystem"
 _scratch_unmount 2>&1 | _filter_scratch
diff --git a/tests/generic/050.out b/tests/generic/050.out
index fb90f6ea5819..2187d16fa328 100644
--- a/tests/generic/050.out
+++ b/tests/generic/050.out
@@ -1,7 +1,7 @@
 QA output created by 050
 setting device read-only
 mounting read-only block device:
-mount: SCRATCH_DEV is write-protected, mounting read-only
+mount: device write-protected, mounting read-only
 touching file on read-only filesystem (should fail)
 touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
 unmounting read-only filesystem
@@ -12,12 +12,12 @@ going down:
 unmounting shutdown filesystem:
 setting device read-only
 mounting filesystem that needs recovery on a read-only device:
-mount: SCRATCH_DEV is write-protected, mounting read-only
-mount: cannot mount SCRATCH_DEV read-only
+mount: device write-protected, mounting read-only
+mount: cannot mount read-only
 unmounting read-only filesystem
 umount: SCRATCH_DEV: not mounted
 mounting filesystem with -o norecovery on a read-only device:
-mount: SCRATCH_DEV is write-protected, mounting read-only
+mount: device write-protected, mounting read-only
 unmounting read-only filesystem
 setting device read-write
 mounting filesystem that needs recovery with -o ro:
diff --git a/tests/overlay/035 b/tests/overlay/035
index 64fcd708105e..05447741a1ba 100755
--- a/tests/overlay/035
+++ b/tests/overlay/035
@@ -69,7 +69,7 @@ mkdir -p $lowerdir1 $lowerdir2 $upperdir $workdir
 $MOUNT_PROG -t overlay -o"lowerdir=$lowerdir2:$lowerdir1" \
 			$OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
-_scratch_remount rw 2>&1 | _filter_scratch
+_scratch_remount rw 2>&1 | _filter_ro_mount
 $UMOUNT_PROG $SCRATCH_MNT
 
 # Make workdir immutable to prevent workdir re-create on mount
@@ -79,7 +79,7 @@ $CHATTR_PROG +i $workdir
 # Verify that overlay is mounted read-only and that it cannot be remounted rw.
 _overlay_scratch_mount_dirs $lowerdir2 $upperdir $workdir
 touch $SCRATCH_MNT/bar 2>&1 | _filter_scratch
-_scratch_remount rw 2>&1 | _filter_scratch
+_scratch_remount rw 2>&1 | _filter_ro_mount
 
 # success, all done
 status=0
diff --git a/tests/overlay/035.out b/tests/overlay/035.out
index 5a5f67771402..e08ba2ebc691 100644
--- a/tests/overlay/035.out
+++ b/tests/overlay/035.out
@@ -1,5 +1,5 @@
 QA output created by 035
 touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
-mount: cannot remount SCRATCH_DEV read-write, is write-protected
+mount: cannot remount device read-write, is write-protected
 touch: cannot touch 'SCRATCH_MNT/bar': Read-only file system
-mount: cannot remount SCRATCH_DEV read-write, is write-protected
+mount: cannot remount device read-write, is write-protected
-- 
2.14.3

Re: [PATCH v3 3/3] fstests: filter readonly mount error messages

[Amir Goldstein]

On Thu, Nov 23, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
> util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
> write-protected devices") changed the error message on read-only
> block device, and in the failure case printed one line message
> instead of two (for details please see comments in common/filter),
> and this change broke generic/050 and overlay/035.
>
> Fix it by adding more filter rules to _filter_ro_mount and updating
> associated .out files to unify the output from both old and new
> util-linux versions.
>
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> v3:
> - document the filtered format in comments
> - remove legacy sed filter, the perl filter covers the legacy case well
> - filter out $SCRATCH_DEV/MNT too and use a consistent output
> - remove the new filter_mount helper in overlay/035
>
>  common/filter         | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>  tests/generic/050     |  8 ++++----
>  tests/generic/050.out |  8 ++++----
>  tests/overlay/035     |  4 ++--
>  tests/overlay/035.out |  4 ++--
>  5 files changed, 58 insertions(+), 14 deletions(-)
>
> diff --git a/common/filter b/common/filter
> index a212c09aa138..6140b331017f 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -399,9 +399,53 @@ _filter_ending_dot()
>
>  # Older mount output referred to "block device" when mounting RO devices
>  # It's gone in newer versions
> +#
> +# And util-linux v2.30 changed the output again, e.g.
> +# for a successful ro mount:
> +# prior to v2.30:  mount: <device> is write-protected, mounting read-only
> +# v2.30 and later: mount: <mountpoint>: WARNING: device write-protected, mounted read-only.
> +#
> +# a failed ro mount:
> +# prior to v2.30:
> +# mount: <device> is write-protected, mounting read-only
> +# mount: cannot mount <device> read-only
> +# v2.30 and later:
> +# mount: <mountpoint>: cannot mount <device> read-only.
> +#
> +# a failed rw remount:
> +# prior to v2.30:  mount: cannot remount <device> read-write, is write-protected
> +# v2.30 and later: mount: <mountpoint>: cannot remount <device> read-write, is write-protected.
> +#
> +# Now use _filter_ro_mount to unify all these differences across old & new
> +# util-linux versions. So the filtered format would be:
> +#
> +# successful ro mount:
> +# mount: device write-protected, mounting read-only
> +#
> +# failed ro mount:
> +# mount: device write-protected, mounting read-only
> +# mount: cannot mount read-only
> +#
> +# failed rw remount:
> +# mount: cannot remount device read-write, is write-protected
>  _filter_ro_mount() {
> -       sed -e "s/mount: block device/mount:/g" \
> -           -e "s/mount: cannot mount block device/mount: cannot mount/g"
> +       perl -ne '
> +       if (/write-protected, mount.*read-only/) {
> +               # successful ro mount
> +               print "mount: device write-protected, mounting read-only\n";
> +       } elsif (/mount: .*: cannot mount.*read-only/) {
> +               # filter v2.30 format failed ro mount
> +               print "mount: device write-protected, mounting read-only\n";

Leftover copy&paste line?

> +               print "mount: cannot mount read-only\n";
> +       } elsif (/(^mount: cannot mount) .* (read-only$)/) {
> +               # filter prior to v2.30 format failed ro mount
> +               print "$1 $2\n";

Maybe I am missing something, but why are you printing arguments
when you know what the expected output should be?
Also, in what is that match expression different from the v2.30 format?
Can't you merge both cases to use the more generic match expr
(without .*:) and print the expected result?

> +       } elsif (/mount:.* cannot remount .* read-write.*/) {
> +               # failed rw remount
> +               print "mount: cannot remount device read-write, is write-protected\n";
> +       } else {
> +               print "$_";
> +       }' | _filter_ending_dot
>  }
>

Re: [PATCH v3 3/3] fstests: filter readonly mount error messages

[Amir Goldstein]

On Thu, Nov 23, 2017 at 3:28 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Nov 23, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
>> util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
>> write-protected devices") changed the error message on read-only
>> block device, and in the failure case printed one line message
>> instead of two (for details please see comments in common/filter),
>> and this change broke generic/050 and overlay/035.
>>
>> Fix it by adding more filter rules to _filter_ro_mount and updating
>> associated .out files to unify the output from both old and new
>> util-linux versions.
>>
>> Signed-off-by: Eryu Guan <eguan@redhat.com>
>> ---
>> v3:
>> - document the filtered format in comments
>> - remove legacy sed filter, the perl filter covers the legacy case well
>> - filter out $SCRATCH_DEV/MNT too and use a consistent output
>> - remove the new filter_mount helper in overlay/035
>>
>>  common/filter         | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
>>  tests/generic/050     |  8 ++++----
>>  tests/generic/050.out |  8 ++++----
>>  tests/overlay/035     |  4 ++--
>>  tests/overlay/035.out |  4 ++--
>>  5 files changed, 58 insertions(+), 14 deletions(-)
>>
>> diff --git a/common/filter b/common/filter
>> index a212c09aa138..6140b331017f 100644
>> --- a/common/filter
>> +++ b/common/filter
>> @@ -399,9 +399,53 @@ _filter_ending_dot()
>>
>>  # Older mount output referred to "block device" when mounting RO devices
>>  # It's gone in newer versions
>> +#
>> +# And util-linux v2.30 changed the output again, e.g.
>> +# for a successful ro mount:
>> +# prior to v2.30:  mount: <device> is write-protected, mounting read-only
>> +# v2.30 and later: mount: <mountpoint>: WARNING: device write-protected, mounted read-only.
>> +#
>> +# a failed ro mount:
>> +# prior to v2.30:
>> +# mount: <device> is write-protected, mounting read-only
>> +# mount: cannot mount <device> read-only
>> +# v2.30 and later:
>> +# mount: <mountpoint>: cannot mount <device> read-only.
>> +#
>> +# a failed rw remount:
>> +# prior to v2.30:  mount: cannot remount <device> read-write, is write-protected
>> +# v2.30 and later: mount: <mountpoint>: cannot remount <device> read-write, is write-protected.
>> +#
>> +# Now use _filter_ro_mount to unify all these differences across old & new
>> +# util-linux versions. So the filtered format would be:
>> +#
>> +# successful ro mount:
>> +# mount: device write-protected, mounting read-only
>> +#
>> +# failed ro mount:
>> +# mount: device write-protected, mounting read-only
>> +# mount: cannot mount read-only
>> +#
>> +# failed rw remount:
>> +# mount: cannot remount device read-write, is write-protected
>>  _filter_ro_mount() {
>> -       sed -e "s/mount: block device/mount:/g" \
>> -           -e "s/mount: cannot mount block device/mount: cannot mount/g"
>> +       perl -ne '
>> +       if (/write-protected, mount.*read-only/) {
>> +               # successful ro mount
>> +               print "mount: device write-protected, mounting read-only\n";
>> +       } elsif (/mount: .*: cannot mount.*read-only/) {
>> +               # filter v2.30 format failed ro mount
>> +               print "mount: device write-protected, mounting read-only\n";
>
> Leftover copy&paste line?
>
>> +               print "mount: cannot mount read-only\n";
>> +       } elsif (/(^mount: cannot mount) .* (read-only$)/) {
>> +               # filter prior to v2.30 format failed ro mount
>> +               print "$1 $2\n";
>
> Maybe I am missing something, but why are you printing arguments
> when you know what the expected output should be?
> Also, in what is that match expression different from the v2.30 format?
> Can't you merge both cases to use the more generic match expr
> (without .*:) and print the expected result?
>

Also, I suggest to change the expected format to:
"mount: cannot mount device read-only"

(i.e. referring to "device") to be consistent with the rest of the
_filter_ro_mount and _filter_busy_mount errors.

Thanks,
Amir.

Re: [PATCH v3 3/3] fstests: filter readonly mount error messages

[Eryu Guan]

On Thu, Nov 23, 2017 at 03:28:58PM +0200, Amir Goldstein wrote:
> On Thu, Nov 23, 2017 at 1:10 PM, Eryu Guan <eguan@redhat.com> wrote:
> > util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
> > write-protected devices") changed the error message on read-only
> > block device, and in the failure case printed one line message
> > instead of two (for details please see comments in common/filter),
> > and this change broke generic/050 and overlay/035.
> >
> > Fix it by adding more filter rules to _filter_ro_mount and updating
> > associated .out files to unify the output from both old and new
> > util-linux versions.
> >
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > v3:
> > - document the filtered format in comments
> > - remove legacy sed filter, the perl filter covers the legacy case well
> > - filter out $SCRATCH_DEV/MNT too and use a consistent output
> > - remove the new filter_mount helper in overlay/035
> >
> >  common/filter         | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
> >  tests/generic/050     |  8 ++++----
> >  tests/generic/050.out |  8 ++++----
> >  tests/overlay/035     |  4 ++--
> >  tests/overlay/035.out |  4 ++--
> >  5 files changed, 58 insertions(+), 14 deletions(-)
> >
> > diff --git a/common/filter b/common/filter
> > index a212c09aa138..6140b331017f 100644
> > --- a/common/filter
> > +++ b/common/filter
> > @@ -399,9 +399,53 @@ _filter_ending_dot()
> >
> >  # Older mount output referred to "block device" when mounting RO devices
> >  # It's gone in newer versions
> > +#
> > +# And util-linux v2.30 changed the output again, e.g.
> > +# for a successful ro mount:
> > +# prior to v2.30:  mount: <device> is write-protected, mounting read-only
> > +# v2.30 and later: mount: <mountpoint>: WARNING: device write-protected, mounted read-only.
> > +#
> > +# a failed ro mount:
> > +# prior to v2.30:
> > +# mount: <device> is write-protected, mounting read-only
> > +# mount: cannot mount <device> read-only
> > +# v2.30 and later:
> > +# mount: <mountpoint>: cannot mount <device> read-only.
> > +#
> > +# a failed rw remount:
> > +# prior to v2.30:  mount: cannot remount <device> read-write, is write-protected
> > +# v2.30 and later: mount: <mountpoint>: cannot remount <device> read-write, is write-protected.
> > +#
> > +# Now use _filter_ro_mount to unify all these differences across old & new
> > +# util-linux versions. So the filtered format would be:
> > +#
> > +# successful ro mount:
> > +# mount: device write-protected, mounting read-only
> > +#
> > +# failed ro mount:
> > +# mount: device write-protected, mounting read-only
> > +# mount: cannot mount read-only
> > +#
> > +# failed rw remount:
> > +# mount: cannot remount device read-write, is write-protected
> >  _filter_ro_mount() {
> > -       sed -e "s/mount: block device/mount:/g" \
> > -           -e "s/mount: cannot mount block device/mount: cannot mount/g"
> > +       perl -ne '
> > +       if (/write-protected, mount.*read-only/) {
> > +               # successful ro mount
> > +               print "mount: device write-protected, mounting read-only\n";
> > +       } elsif (/mount: .*: cannot mount.*read-only/) {
> > +               # filter v2.30 format failed ro mount
> > +               print "mount: device write-protected, mounting read-only\n";
> 
> Leftover copy&paste line?

No, that's intentional. Prior to v2.30, mount printed two-line error
messages, as described in the comments above:

# a failed ro mount:
# prior to v2.30:
# mount: <device> is write-protected, mounting read-only
# mount: cannot mount <device> read-only
# v2.30 and later:
# mount: <mountpoint>: cannot mount <device> read-only.

So this is matching the one-line v2.30 format and converting it to
two-line messages.

> 
> > +               print "mount: cannot mount read-only\n";
> > +       } elsif (/(^mount: cannot mount) .* (read-only$)/) {
> > +               # filter prior to v2.30 format failed ro mount
> > +               print "$1 $2\n";
> 
> Maybe I am missing something, but why are you printing arguments
> when you know what the expected output should be?

No special reason, perhaps that was easier to type :) I'll print the
expected output directly, and change the output to

mount: cannot mount device read-only

as you suggested.

> Also, in what is that match expression different from the v2.30 format?

v2.30 failed ro mount output is a single-line message, with the
"<mountpoint> :" string in between "mount: " and "cannot mount", so I'm
explicitly matching the colon with ".*:".

prior to v2.30, failed ro mount output is two-line message, with the
first line identical to the "successful ro mount" case (maybe I should
have mentioned it too along with the "successful ro mount" comment), so
I'm matching the second line here.

> Can't you merge both cases to use the more generic match expr
> (without .*:) and print the expected result?

Seems no, I need to distinguish the two cases and decide if I should
print out single-line or two-line messages.

It's a bit complicated, maybe I missed some obvious straightforward fix,
that's why I said the following in v1/v2 cover letter :)

"Perhaps there're better and cleaner ways to fix the problems, please
help review and advise"

Thanks,
Eryu

> 
> > +       } elsif (/mount:.* cannot remount .* read-write.*/) {
> > +               # failed rw remount
> > +               print "mount: cannot remount device read-write, is write-protected\n";
> > +       } else {
> > +               print "$_";
> > +       }' | _filter_ending_dot
> >  }
> >

[PATCH v4] fstests: filter readonly mount error messages

[Eryu Guan]

util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
write-protected devices") changed the error message on read-only
block device, and in the failure case printed one line message
instead of two (for details please see comments in common/filter),
and this change broke generic/050 and overlay/035.

Fix it by adding more filter rules to _filter_ro_mount and updating
associated .out files to unify the output from both old and new
util-linux versions.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
v4 (also passed with both old & new util-linux):
- add more comments to explain the output differences between util-linux
  versions
- print out message directly instead of using perl variables
- add word "device" to failed ro mount message

v3:
- document the filtered format in comments
- remove legacy sed filter, the perl filter covers the legacy case well
- filter out $SCRATCH_DEV/MNT too and use a consistent output
- remove the new filter_mount helper in overlay/035

 common/filter         | 59 +++++++++++++++++++++++++++++++++++++++++++++++----
 tests/generic/050     |  8 +++----
 tests/generic/050.out |  8 +++----
 tests/overlay/035     |  4 ++--
 tests/overlay/035.out |  4 ++--
 5 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/common/filter b/common/filter
index a212c09aa138..9c33efacea6c 100644
--- a/common/filter
+++ b/common/filter
@@ -397,11 +397,62 @@ _filter_ending_dot()
 	sed -e "s/\.$//"
 }
 
-# Older mount output referred to "block device" when mounting RO devices
-# It's gone in newer versions
+# Older mount output referred to "block device" when mounting RO devices. It's
+# gone in newer versions. v2.30 changed the output again. This filter is to
+# unify all read-only mount messages across all util-linux versions.
+#
+# for a successful ro mount:
+# ancient:	   mount: block device <device> is write-protected, mounting read-only
+# prior to v2.30:  mount: <device> is write-protected, mounting read-only
+# v2.30 and later: mount: <mountpoint>: WARNING: device write-protected, mounted read-only.
+#
+# a failed ro mount:
+# ancient (two-line message):
+# mount: block device <device> is write-protected, mounting read-only
+# mount: cannot mount block device <device> read-only
+# prior to v2.30 (two-line message):
+# mount: <device> is write-protected, mounting read-only
+# mount: cannot mount <device> read-only
+# v2.30 and later (single-line message):
+# mount: <mountpoint>: cannot mount <device> read-only.
+#
+# a failed rw remount:
+# ancient:	   mount: cannot remount block device <device> read-write, is write-protected
+# prior to v2.30:  mount: cannot remount <device> read-write, is write-protected
+# v2.30 and later: mount: <mountpoint>: cannot remount <device> read-write, is write-protected.
+#
+# Now use _filter_ro_mount to unify all these differences across old & new
+# util-linux versions. So the filtered format would be:
+#
+# successful ro mount:
+# mount: device write-protected, mounting read-only
+#
+# failed ro mount:
+# mount: device write-protected, mounting read-only
+# mount: cannot mount device read-only
+#
+# failed rw remount:
+# mount: cannot remount device read-write, is write-protected
 _filter_ro_mount() {
-	sed -e "s/mount: block device/mount:/g" \
-	    -e "s/mount: cannot mount block device/mount: cannot mount/g"
+	perl -ne '
+	if (/write-protected, mount.*read-only/) {
+		# filter successful ro mount, and first line of prior to v2.30
+		# format failed ro mount
+		print "mount: device write-protected, mounting read-only\n";
+	} elsif (/mount: .*: cannot mount.*read-only/) {
+		# filter v2.30 format failed ro mount, convert single-line
+		# message to two-line message
+		print "mount: device write-protected, mounting read-only\n";
+		print "mount: cannot mount device read-only\n";
+	} elsif (/^mount: cannot mount .* read-only$/) {
+		# filter prior to v2.30 format failed ro mount
+		print "mount: cannot mount device read-only\n";
+	} elsif (/mount:.* cannot remount .* read-write.*/) {
+		# filter failed rw remount
+		print "mount: cannot remount device read-write, is write-protected\n";
+	} else {
+		print "$_";
+	}' | _filter_ending_dot
 }
 
 # Filter a failed mount output due to EUCLEAN and USTALE, util-linux changed
diff --git a/tests/generic/050 b/tests/generic/050
index 5fa28a7648e5..efa45f04825b 100755
--- a/tests/generic/050
+++ b/tests/generic/050
@@ -60,7 +60,7 @@ blockdev --setro $SCRATCH_DEV
 # Mount it, and make sure we can't write to it, and we can unmount it again
 #
 echo "mounting read-only block device:"
-_scratch_mount 2>&1 | _filter_scratch | _filter_ro_mount
+_scratch_mount 2>&1 | _filter_ro_mount
 
 echo "touching file on read-only filesystem (should fail)"
 touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
@@ -95,10 +95,10 @@ blockdev --setro $SCRATCH_DEV
 # -o norecovery is used.
 #
 echo "mounting filesystem that needs recovery on a read-only device:"
-_scratch_mount 2>&1 | _filter_scratch | _filter_ro_mount
+_scratch_mount 2>&1 | _filter_ro_mount
 
 echo "unmounting read-only filesystem"
-_scratch_unmount 2>&1 | _filter_scratch
+_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
 
 #
 # This is the way out if the underlying device really is read-only.
@@ -106,7 +106,7 @@ _scratch_unmount 2>&1 | _filter_scratch
 # data recovery hack.
 #
 echo "mounting filesystem with -o norecovery on a read-only device:"
-_scratch_mount -o norecovery 2>&1 | _filter_scratch | _filter_ro_mount
+_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
 
 echo "unmounting read-only filesystem"
 _scratch_unmount 2>&1 | _filter_scratch
diff --git a/tests/generic/050.out b/tests/generic/050.out
index fb90f6ea5819..7d70ddee83cb 100644
--- a/tests/generic/050.out
+++ b/tests/generic/050.out
@@ -1,7 +1,7 @@
 QA output created by 050
 setting device read-only
 mounting read-only block device:
-mount: SCRATCH_DEV is write-protected, mounting read-only
+mount: device write-protected, mounting read-only
 touching file on read-only filesystem (should fail)
 touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
 unmounting read-only filesystem
@@ -12,12 +12,12 @@ going down:
 unmounting shutdown filesystem:
 setting device read-only
 mounting filesystem that needs recovery on a read-only device:
-mount: SCRATCH_DEV is write-protected, mounting read-only
-mount: cannot mount SCRATCH_DEV read-only
+mount: device write-protected, mounting read-only
+mount: cannot mount device read-only
 unmounting read-only filesystem
 umount: SCRATCH_DEV: not mounted
 mounting filesystem with -o norecovery on a read-only device:
-mount: SCRATCH_DEV is write-protected, mounting read-only
+mount: device write-protected, mounting read-only
 unmounting read-only filesystem
 setting device read-write
 mounting filesystem that needs recovery with -o ro:
diff --git a/tests/overlay/035 b/tests/overlay/035
index 64fcd708105e..05447741a1ba 100755
--- a/tests/overlay/035
+++ b/tests/overlay/035
@@ -69,7 +69,7 @@ mkdir -p $lowerdir1 $lowerdir2 $upperdir $workdir
 $MOUNT_PROG -t overlay -o"lowerdir=$lowerdir2:$lowerdir1" \
 			$OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
 touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
-_scratch_remount rw 2>&1 | _filter_scratch
+_scratch_remount rw 2>&1 | _filter_ro_mount
 $UMOUNT_PROG $SCRATCH_MNT
 
 # Make workdir immutable to prevent workdir re-create on mount
@@ -79,7 +79,7 @@ $CHATTR_PROG +i $workdir
 # Verify that overlay is mounted read-only and that it cannot be remounted rw.
 _overlay_scratch_mount_dirs $lowerdir2 $upperdir $workdir
 touch $SCRATCH_MNT/bar 2>&1 | _filter_scratch
-_scratch_remount rw 2>&1 | _filter_scratch
+_scratch_remount rw 2>&1 | _filter_ro_mount
 
 # success, all done
 status=0
diff --git a/tests/overlay/035.out b/tests/overlay/035.out
index 5a5f67771402..e08ba2ebc691 100644
--- a/tests/overlay/035.out
+++ b/tests/overlay/035.out
@@ -1,5 +1,5 @@
 QA output created by 035
 touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
-mount: cannot remount SCRATCH_DEV read-write, is write-protected
+mount: cannot remount device read-write, is write-protected
 touch: cannot touch 'SCRATCH_MNT/bar': Read-only file system
-mount: cannot remount SCRATCH_DEV read-write, is write-protected
+mount: cannot remount device read-write, is write-protected
-- 
2.14.3

Re: [PATCH v4] fstests: filter readonly mount error messages

[Amir Goldstein]

[cc: Karel Zak]

On Fri, Nov 24, 2017 at 7:01 AM, Eryu Guan <eguan@redhat.com> wrote:
> util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
> write-protected devices") changed the error message on read-only
> block device, and in the failure case printed one line message
> instead of two (for details please see comments in common/filter),
> and this change broke generic/050 and overlay/035.
>
> Fix it by adding more filter rules to _filter_ro_mount and updating
> associated .out files to unify the output from both old and new
> util-linux versions.
>
> Signed-off-by: Eryu Guan <eguan@redhat.com>
> ---
> v4 (also passed with both old & new util-linux):
> - add more comments to explain the output differences between util-linux
>   versions
> - print out message directly instead of using perl variables
> - add word "device" to failed ro mount message


Eryu,

This looks good and you can take it as an ACK on the series.

I've CC'ed Karel, so maybe he thinks about us fstests guys before making
these sort of changes again...

Thinking out loud, does xfstest even need to use mount program from
util-linux? Do we ever need anything other than the bare libc mount(2)?
We need it for -o loop, but that is the exception to the rule.

For fs that have mount helpers (cifs,nfs), we could use mount.$FSTYP
directly (what error formats are reported from the helpers??). For all other
fs we can write a simple t_mount program to wrap libc mount(2) and not be
dependent on util-linux error message formats.
Maybe we can consider it next time util-linux changes..

Another idea to through in the air in the direction of Karel -
Maybe it makes sense for util-linux to check some env variable
and then print all error messages in a unified machine format, e.g.:
fprintf("%s: errno=%d\n", progname, errno);

Cheers,
Amir.

>
> v3:
> - document the filtered format in comments
> - remove legacy sed filter, the perl filter covers the legacy case well
> - filter out $SCRATCH_DEV/MNT too and use a consistent output
> - remove the new filter_mount helper in overlay/035
>
>  common/filter         | 59 +++++++++++++++++++++++++++++++++++++++++++++++----
>  tests/generic/050     |  8 +++----
>  tests/generic/050.out |  8 +++----
>  tests/overlay/035     |  4 ++--
>  tests/overlay/035.out |  4 ++--
>  5 files changed, 67 insertions(+), 16 deletions(-)
>
> diff --git a/common/filter b/common/filter
> index a212c09aa138..9c33efacea6c 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -397,11 +397,62 @@ _filter_ending_dot()
>         sed -e "s/\.$//"
>  }
>
> -# Older mount output referred to "block device" when mounting RO devices
> -# It's gone in newer versions
> +# Older mount output referred to "block device" when mounting RO devices. It's
> +# gone in newer versions. v2.30 changed the output again. This filter is to
> +# unify all read-only mount messages across all util-linux versions.
> +#
> +# for a successful ro mount:
> +# ancient:        mount: block device <device> is write-protected, mounting read-only
> +# prior to v2.30:  mount: <device> is write-protected, mounting read-only
> +# v2.30 and later: mount: <mountpoint>: WARNING: device write-protected, mounted read-only.
> +#
> +# a failed ro mount:
> +# ancient (two-line message):
> +# mount: block device <device> is write-protected, mounting read-only
> +# mount: cannot mount block device <device> read-only
> +# prior to v2.30 (two-line message):
> +# mount: <device> is write-protected, mounting read-only
> +# mount: cannot mount <device> read-only
> +# v2.30 and later (single-line message):
> +# mount: <mountpoint>: cannot mount <device> read-only.
> +#
> +# a failed rw remount:
> +# ancient:        mount: cannot remount block device <device> read-write, is write-protected
> +# prior to v2.30:  mount: cannot remount <device> read-write, is write-protected
> +# v2.30 and later: mount: <mountpoint>: cannot remount <device> read-write, is write-protected.
> +#
> +# Now use _filter_ro_mount to unify all these differences across old & new
> +# util-linux versions. So the filtered format would be:
> +#
> +# successful ro mount:
> +# mount: device write-protected, mounting read-only
> +#
> +# failed ro mount:
> +# mount: device write-protected, mounting read-only
> +# mount: cannot mount device read-only
> +#
> +# failed rw remount:
> +# mount: cannot remount device read-write, is write-protected
>  _filter_ro_mount() {
> -       sed -e "s/mount: block device/mount:/g" \
> -           -e "s/mount: cannot mount block device/mount: cannot mount/g"
> +       perl -ne '
> +       if (/write-protected, mount.*read-only/) {
> +               # filter successful ro mount, and first line of prior to v2.30
> +               # format failed ro mount
> +               print "mount: device write-protected, mounting read-only\n";
> +       } elsif (/mount: .*: cannot mount.*read-only/) {
> +               # filter v2.30 format failed ro mount, convert single-line
> +               # message to two-line message
> +               print "mount: device write-protected, mounting read-only\n";
> +               print "mount: cannot mount device read-only\n";
> +       } elsif (/^mount: cannot mount .* read-only$/) {
> +               # filter prior to v2.30 format failed ro mount
> +               print "mount: cannot mount device read-only\n";
> +       } elsif (/mount:.* cannot remount .* read-write.*/) {
> +               # filter failed rw remount
> +               print "mount: cannot remount device read-write, is write-protected\n";
> +       } else {
> +               print "$_";
> +       }' | _filter_ending_dot
>  }
>
>  # Filter a failed mount output due to EUCLEAN and USTALE, util-linux changed
> diff --git a/tests/generic/050 b/tests/generic/050
> index 5fa28a7648e5..efa45f04825b 100755
> --- a/tests/generic/050
> +++ b/tests/generic/050
> @@ -60,7 +60,7 @@ blockdev --setro $SCRATCH_DEV
>  # Mount it, and make sure we can't write to it, and we can unmount it again
>  #
>  echo "mounting read-only block device:"
> -_scratch_mount 2>&1 | _filter_scratch | _filter_ro_mount
> +_scratch_mount 2>&1 | _filter_ro_mount
>
>  echo "touching file on read-only filesystem (should fail)"
>  touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> @@ -95,10 +95,10 @@ blockdev --setro $SCRATCH_DEV
>  # -o norecovery is used.
>  #
>  echo "mounting filesystem that needs recovery on a read-only device:"
> -_scratch_mount 2>&1 | _filter_scratch | _filter_ro_mount
> +_scratch_mount 2>&1 | _filter_ro_mount
>
>  echo "unmounting read-only filesystem"
> -_scratch_unmount 2>&1 | _filter_scratch
> +_scratch_unmount 2>&1 | _filter_scratch | _filter_ending_dot
>
>  #
>  # This is the way out if the underlying device really is read-only.
> @@ -106,7 +106,7 @@ _scratch_unmount 2>&1 | _filter_scratch
>  # data recovery hack.
>  #
>  echo "mounting filesystem with -o norecovery on a read-only device:"
> -_scratch_mount -o norecovery 2>&1 | _filter_scratch | _filter_ro_mount
> +_scratch_mount -o norecovery 2>&1 | _filter_ro_mount
>
>  echo "unmounting read-only filesystem"
>  _scratch_unmount 2>&1 | _filter_scratch
> diff --git a/tests/generic/050.out b/tests/generic/050.out
> index fb90f6ea5819..7d70ddee83cb 100644
> --- a/tests/generic/050.out
> +++ b/tests/generic/050.out
> @@ -1,7 +1,7 @@
>  QA output created by 050
>  setting device read-only
>  mounting read-only block device:
> -mount: SCRATCH_DEV is write-protected, mounting read-only
> +mount: device write-protected, mounting read-only
>  touching file on read-only filesystem (should fail)
>  touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
>  unmounting read-only filesystem
> @@ -12,12 +12,12 @@ going down:
>  unmounting shutdown filesystem:
>  setting device read-only
>  mounting filesystem that needs recovery on a read-only device:
> -mount: SCRATCH_DEV is write-protected, mounting read-only
> -mount: cannot mount SCRATCH_DEV read-only
> +mount: device write-protected, mounting read-only
> +mount: cannot mount device read-only
>  unmounting read-only filesystem
>  umount: SCRATCH_DEV: not mounted
>  mounting filesystem with -o norecovery on a read-only device:
> -mount: SCRATCH_DEV is write-protected, mounting read-only
> +mount: device write-protected, mounting read-only
>  unmounting read-only filesystem
>  setting device read-write
>  mounting filesystem that needs recovery with -o ro:
> diff --git a/tests/overlay/035 b/tests/overlay/035
> index 64fcd708105e..05447741a1ba 100755
> --- a/tests/overlay/035
> +++ b/tests/overlay/035
> @@ -69,7 +69,7 @@ mkdir -p $lowerdir1 $lowerdir2 $upperdir $workdir
>  $MOUNT_PROG -t overlay -o"lowerdir=$lowerdir2:$lowerdir1" \
>                         $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT
>  touch $SCRATCH_MNT/foo 2>&1 | _filter_scratch
> -_scratch_remount rw 2>&1 | _filter_scratch
> +_scratch_remount rw 2>&1 | _filter_ro_mount
>  $UMOUNT_PROG $SCRATCH_MNT
>
>  # Make workdir immutable to prevent workdir re-create on mount
> @@ -79,7 +79,7 @@ $CHATTR_PROG +i $workdir
>  # Verify that overlay is mounted read-only and that it cannot be remounted rw.
>  _overlay_scratch_mount_dirs $lowerdir2 $upperdir $workdir
>  touch $SCRATCH_MNT/bar 2>&1 | _filter_scratch
> -_scratch_remount rw 2>&1 | _filter_scratch
> +_scratch_remount rw 2>&1 | _filter_ro_mount
>
>  # success, all done
>  status=0
> diff --git a/tests/overlay/035.out b/tests/overlay/035.out
> index 5a5f67771402..e08ba2ebc691 100644
> --- a/tests/overlay/035.out
> +++ b/tests/overlay/035.out
> @@ -1,5 +1,5 @@
>  QA output created by 035
>  touch: cannot touch 'SCRATCH_MNT/foo': Read-only file system
> -mount: cannot remount SCRATCH_DEV read-write, is write-protected
> +mount: cannot remount device read-write, is write-protected
>  touch: cannot touch 'SCRATCH_MNT/bar': Read-only file system
> -mount: cannot remount SCRATCH_DEV read-write, is write-protected
> +mount: cannot remount device read-write, is write-protected
> --
> 2.14.3
>

Re: [PATCH v4] fstests: filter readonly mount error messages

[Karel Zak]

On Fri, Nov 24, 2017 at 10:04:33AM +0200, Amir Goldstein wrote:
> [cc: Karel Zak]
> 
> On Fri, Nov 24, 2017 at 7:01 AM, Eryu Guan <eguan@redhat.com> wrote:
> > util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
> > write-protected devices") changed the error message on read-only
> > block device, and in the failure case printed one line message
> > instead of two (for details please see comments in common/filter),
> > and this change broke generic/050 and overlay/035.
> >
> > Fix it by adding more filter rules to _filter_ro_mount and updating
> > associated .out files to unify the output from both old and new
> > util-linux versions.
> >
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > v4 (also passed with both old & new util-linux):
> > - add more comments to explain the output differences between util-linux
> >   versions
> > - print out message directly instead of using perl variables
> > - add word "device" to failed ro mount message
> 
> 
> Eryu,
> 
> This looks good and you can take it as an ACK on the series.
> 
> I've CC'ed Karel, so maybe he thinks about us fstests guys before making
> these sort of changes again...

Frankly, we have never promised that things like warning messages (or
another messages) are stable interface. It's fragile to depend on this
stuff...

> Thinking out loud, does xfstest even need to use mount program from
> util-linux? Do we ever need anything other than the bare libc mount(2)?
> We need it for -o loop, but that is the exception to the rule.

Well, I don't think that create a parallel universe is the best
solution.

> For fs that have mount helpers (cifs,nfs), we could use mount.$FSTYP
> directly (what error formats are reported from the helpers??). For all other
> fs we can write a simple t_mount program to wrap libc mount(2) and not be
> dependent on util-linux error message formats.
> Maybe we can consider it next time util-linux changes..
> 
> Another idea to through in the air in the direction of Karel -
> Maybe it makes sense for util-linux to check some env variable
> and then print all error messages in a unified machine format, e.g.:
> fprintf("%s: errno=%d\n", progname, errno);

This is good idea, I can try to implement it into libmount. 

 Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v4] fstests: filter readonly mount error messages

[Amir Goldstein]

On Fri, Nov 24, 2017 at 12:38 PM, Karel Zak <kzak@redhat.com> wrote:
> On Fri, Nov 24, 2017 at 10:04:33AM +0200, Amir Goldstein wrote:
>> [cc: Karel Zak]
>>
>> On Fri, Nov 24, 2017 at 7:01 AM, Eryu Guan <eguan@redhat.com> wrote:
>> > util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
>> > write-protected devices") changed the error message on read-only
>> > block device, and in the failure case printed one line message
>> > instead of two (for details please see comments in common/filter),
>> > and this change broke generic/050 and overlay/035.
>> >
>> > Fix it by adding more filter rules to _filter_ro_mount and updating
>> > associated .out files to unify the output from both old and new
>> > util-linux versions.
>> >
>> > Signed-off-by: Eryu Guan <eguan@redhat.com>
>> > ---
>> > v4 (also passed with both old & new util-linux):
>> > - add more comments to explain the output differences between util-linux
>> >   versions
>> > - print out message directly instead of using perl variables
>> > - add word "device" to failed ro mount message
>>
>>
>> Eryu,
>>
>> This looks good and you can take it as an ACK on the series.
>>
>> I've CC'ed Karel, so maybe he thinks about us fstests guys before making
>> these sort of changes again...
>
> Frankly, we have never promised that things like warning messages (or
> another messages) are stable interface. It's fragile to depend on this
> stuff...
>

I know. No complains. Just wanted to inform you of the downstream
ripples.

>> Thinking out loud, does xfstest even need to use mount program from
>> util-linux? Do we ever need anything other than the bare libc mount(2)?
>> We need it for -o loop, but that is the exception to the rule.
>
> Well, I don't think that create a parallel universe is the best
> solution.
>

I agree. However, xfstest uses 'mount' is a very particular way:
- User has to provide both block device and mount point
- xfstests already checks that those block device and mount point
  are not in use in /proc/mounts *before* calling 'mount'

So I prefer the 'machine output format' solution going forward, but
since xfstests will need to continue supporting old util-linux installations
it might be less clumsy to use a simple C wrapper to mount(2), then
to carry these error format filters.
xfstests can also check for availability of the new machine format in
'mount' and if it doesn't exist fall back to using its private mount helper.

>> For fs that have mount helpers (cifs,nfs), we could use mount.$FSTYP
>> directly (what error formats are reported from the helpers??). For all other
>> fs we can write a simple t_mount program to wrap libc mount(2) and not be
>> dependent on util-linux error message formats.
>> Maybe we can consider it next time util-linux changes..
>>
>> Another idea to through in the air in the direction of Karel -
>> Maybe it makes sense for util-linux to check some env variable
>> and then print all error messages in a unified machine format, e.g.:
>> fprintf("%s: errno=%d\n", progname, errno);
>
> This is good idea, I can try to implement it into libmount.
>

That would be great.

Thanks,
Amir.

Re: [PATCH v4] fstests: filter readonly mount error messages

[Karel Zak]

On Sun, Nov 26, 2017 at 10:56:40AM +0200, Amir Goldstein wrote:
> >> Thinking out loud, does xfstest even need to use mount program from
> >> util-linux? Do we ever need anything other than the bare libc mount(2)?
> >> We need it for -o loop, but that is the exception to the rule.
> >
> > Well, I don't think that create a parallel universe is the best
> > solution.
> >
> 
> I agree. However, xfstest uses 'mount' is a very particular way:
> - User has to provide both block device and mount point
> - xfstests already checks that those block device and mount point
>   are not in use in /proc/mounts *before* calling 'mount'

OK, I see, than the wrapper is not so bad idea (if you're able to keep
it simple and stupid:-)

> So I prefer the 'machine output format' solution going forward, but
> since xfstests will need to continue supporting old util-linux installations
> it might be less clumsy to use a simple C wrapper to mount(2), then
> to carry these error format filters.
> xfstests can also check for availability of the new machine format in
> 'mount' and if it doesn't exist fall back to using its private mount helper.

If you want to implement the wrapper than it's probably better use it
in all cases and then you can remove all the backward compatibility
filters. In thins case it seems better to depend on the wrapper only.
So, the unified messages maintained by unit-linux mount(8) will be
unnecessary.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH v4] fstests: filter readonly mount error messages

[Eryu Guan]

On Sun, Nov 26, 2017 at 10:56:40AM +0200, Amir Goldstein wrote:
> On Fri, Nov 24, 2017 at 12:38 PM, Karel Zak <kzak@redhat.com> wrote:
> > On Fri, Nov 24, 2017 at 10:04:33AM +0200, Amir Goldstein wrote:
> >> [cc: Karel Zak]
> >>
> >> On Fri, Nov 24, 2017 at 7:01 AM, Eryu Guan <eguan@redhat.com> wrote:
> >> > util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
> >> > write-protected devices") changed the error message on read-only
> >> > block device, and in the failure case printed one line message
> >> > instead of two (for details please see comments in common/filter),
> >> > and this change broke generic/050 and overlay/035.
> >> >
> >> > Fix it by adding more filter rules to _filter_ro_mount and updating
> >> > associated .out files to unify the output from both old and new
> >> > util-linux versions.
> >> >
> >> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> >> > ---
> >> > v4 (also passed with both old & new util-linux):
> >> > - add more comments to explain the output differences between util-linux
> >> >   versions
> >> > - print out message directly instead of using perl variables
> >> > - add word "device" to failed ro mount message
> >>
> >>
> >> Eryu,
> >>
> >> This looks good and you can take it as an ACK on the series.
> >>
> >> I've CC'ed Karel, so maybe he thinks about us fstests guys before making
> >> these sort of changes again...
> >
> > Frankly, we have never promised that things like warning messages (or
> > another messages) are stable interface. It's fragile to depend on this
> > stuff...
> >
> 
> I know. No complains. Just wanted to inform you of the downstream
> ripples.

Agreed.

> 
> >> Thinking out loud, does xfstest even need to use mount program from
> >> util-linux? Do we ever need anything other than the bare libc mount(2)?
> >> We need it for -o loop, but that is the exception to the rule.
> >
> > Well, I don't think that create a parallel universe is the best
> > solution.
> >
> 
> I agree. However, xfstest uses 'mount' is a very particular way:
> - User has to provide both block device and mount point
> - xfstests already checks that those block device and mount point
>   are not in use in /proc/mounts *before* calling 'mount'

The problem is it's not a mount-only issue, outputs from other
util-linux commands and commands from coreutils have been changing over
time too, we really can't "fork" & maintain a local wrapper all the
changing pieces. As long as fstests still go with the 'match golden
image' path, it's not likely to avoid such problems completely. OTOH,
the output changes are not so frequent (such a change per 3-4 years
maybe?), I think the maintain burden of filters is much less than
maintaining local wrappers of such commands.

And commands like mount are fs-related, it's better to get them some
(indirect) test coverage in fstests too, as we test other fs-specific
userspace tools like xfsprogs, e2fsprogs etc.

Thanks,
Eryu