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

[Eryu Guan <eguan@redhat.com>]

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