| Message ID | 20220526095704.3772355-1-zlang@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | common/rc: filter out extra mount error | expand |
On Thu, May 26, 2022 at 05:57:04PM +0800, Zorro Lang wrote: > The lastest mount command (from util-linux) merged below commit: > 79534c0d7e0f ("mount: add hint about dmesg(8) to error messages") > which brought in a new error output when mount fails. > > That cause some cases (e.g. xfs/005) fail as: > mount: Structure needs cleaning > dmesg(1) may have more information after failed mount system call > > More failed cases like generic/050, ext4/002, xfs/154, etc ... > > Signed-off-by: Zorro Lang <zlang@kernel.org> > --- > > Due to _try_scratch_mount is call in many places, so I'm not sure if it's good > to filter this message in _try_scratch_mount. Maybe better to filter it in each > failed cases? Or write another function (e.g. _try_scratch_mount_fail) ? > Welcome review points. > > Thanks, > Zorro > > common/rc | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index 70a15f9c..67342760 100644 > --- a/common/rc > +++ b/common/rc > @@ -335,7 +335,8 @@ _try_scratch_mount() > _overlay_scratch_mount $* > return $? > fi > - _mount -t $FSTYP `_scratch_mount_options $*` > + _mount -t $FSTYP `_scratch_mount_options $*` 2> \ > + >(grep -v "dmesg(1) may have more information after failed mount" >&2) _filter_error_mount()? Cheers, Dave.
On Fri, May 27, 2022 at 09:45:06AM +1000, Dave Chinner wrote: > On Thu, May 26, 2022 at 05:57:04PM +0800, Zorro Lang wrote: > > The lastest mount command (from util-linux) merged below commit: > > 79534c0d7e0f ("mount: add hint about dmesg(8) to error messages") > > which brought in a new error output when mount fails. > > > > That cause some cases (e.g. xfs/005) fail as: > > mount: Structure needs cleaning > > dmesg(1) may have more information after failed mount system call > > > > More failed cases like generic/050, ext4/002, xfs/154, etc ... > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > --- > > > > Due to _try_scratch_mount is call in many places, so I'm not sure if it's good > > to filter this message in _try_scratch_mount. Maybe better to filter it in each > > failed cases? Or write another function (e.g. _try_scratch_mount_fail) ? > > Welcome review points. > > > > Thanks, > > Zorro > > > > common/rc | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/common/rc b/common/rc > > index 70a15f9c..67342760 100644 > > --- a/common/rc > > +++ b/common/rc > > @@ -335,7 +335,8 @@ _try_scratch_mount() > > _overlay_scratch_mount $* > > return $? > > fi > > - _mount -t $FSTYP `_scratch_mount_options $*` > > + _mount -t $FSTYP `_scratch_mount_options $*` 2> \ > > + >(grep -v "dmesg(1) may have more information after failed mount" >&2) > > _filter_error_mount()? OK, how about this change [1] ? Do you think doing this "filter out" in _try_scratch_mount is better, or in each case which expects mount failures in .out file (e.g. xfs/005) ? Or a new _try_scratch_mount_fail() ? Thanks, Zorro [1] diff --git a/common/filter b/common/filter index a6a42b7a..54fb8d68 100644 --- a/common/filter +++ b/common/filter @@ -655,5 +655,10 @@ _filter_blkzone_report() sed -e 's/len/cap/2' } +_filter_mount_error() +{ + grep -v "dmesg(1) may have more information after failed mount" +} + # make sure this script returns success /bin/true diff --git a/common/rc b/common/rc index 2f31ca46..c16bd405 100644 --- a/common/rc +++ b/common/rc @@ -288,7 +288,8 @@ _try_scratch_mount() _overlay_scratch_mount $* return $? fi - _mount -t $FSTYP `_scratch_mount_options $*` + _mount -t $FSTYP `_scratch_mount_options $*` 2> \ + >(_filter_mount_error >&2) mount_ret=$? [ $mount_ret -ne 0 ] && return $mount_ret _idmapped_mount $SCRATCH_DEV $SCRATCH_MNT Thanks, Zorro > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com >
On Fri, May 27, 2022 at 12:21:29PM +0800, Zorro Lang wrote: > On Fri, May 27, 2022 at 09:45:06AM +1000, Dave Chinner wrote: > > On Thu, May 26, 2022 at 05:57:04PM +0800, Zorro Lang wrote: > > > The lastest mount command (from util-linux) merged below commit: > > > 79534c0d7e0f ("mount: add hint about dmesg(8) to error messages") > > > which brought in a new error output when mount fails. > > > > > > That cause some cases (e.g. xfs/005) fail as: > > > mount: Structure needs cleaning > > > dmesg(1) may have more information after failed mount system call > > > > > > More failed cases like generic/050, ext4/002, xfs/154, etc ... > > > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > > --- > > > > > > Due to _try_scratch_mount is call in many places, so I'm not sure if it's good > > > to filter this message in _try_scratch_mount. Maybe better to filter it in each > > > failed cases? Or write another function (e.g. _try_scratch_mount_fail) ? > > > Welcome review points. > > > > > > Thanks, > > > Zorro > > > > > > common/rc | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/common/rc b/common/rc > > > index 70a15f9c..67342760 100644 > > > --- a/common/rc > > > +++ b/common/rc > > > @@ -335,7 +335,8 @@ _try_scratch_mount() > > > _overlay_scratch_mount $* > > > return $? > > > fi > > > - _mount -t $FSTYP `_scratch_mount_options $*` > > > + _mount -t $FSTYP `_scratch_mount_options $*` 2> \ > > > + >(grep -v "dmesg(1) may have more information after failed mount" >&2) > > > > _filter_error_mount()? > > OK, how about this change [1] ? > > Do you think doing this "filter out" in _try_scratch_mount is better, or in > each case which expects mount failures in .out file (e.g. xfs/005) ? Or a new > _try_scratch_mount_fail() ? I think you missed my point entirely. We already have a filter for this in common/filter because util-linux has changed it's failure message for EFSCORRUPTED several times: # 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 } So, capture and supress the new output via _filter_error_mount() and use that in the specific tests that are trying to mount a corrupt filesystem and capture that specific error message in their golden output.... Cheers, Dave.
On Fri, May 27, 2022 at 07:13:30PM +1000, Dave Chinner wrote: > On Fri, May 27, 2022 at 12:21:29PM +0800, Zorro Lang wrote: > > On Fri, May 27, 2022 at 09:45:06AM +1000, Dave Chinner wrote: > > > On Thu, May 26, 2022 at 05:57:04PM +0800, Zorro Lang wrote: > > > > The lastest mount command (from util-linux) merged below commit: > > > > 79534c0d7e0f ("mount: add hint about dmesg(8) to error messages") > > > > which brought in a new error output when mount fails. > > > > > > > > That cause some cases (e.g. xfs/005) fail as: > > > > mount: Structure needs cleaning > > > > dmesg(1) may have more information after failed mount system call > > > > > > > > More failed cases like generic/050, ext4/002, xfs/154, etc ... > > > > > > > > Signed-off-by: Zorro Lang <zlang@kernel.org> > > > > --- > > > > > > > > Due to _try_scratch_mount is call in many places, so I'm not sure if it's good > > > > to filter this message in _try_scratch_mount. Maybe better to filter it in each > > > > failed cases? Or write another function (e.g. _try_scratch_mount_fail) ? > > > > Welcome review points. > > > > > > > > Thanks, > > > > Zorro > > > > > > > > common/rc | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/common/rc b/common/rc > > > > index 70a15f9c..67342760 100644 > > > > --- a/common/rc > > > > +++ b/common/rc > > > > @@ -335,7 +335,8 @@ _try_scratch_mount() > > > > _overlay_scratch_mount $* > > > > return $? > > > > fi > > > > - _mount -t $FSTYP `_scratch_mount_options $*` > > > > + _mount -t $FSTYP `_scratch_mount_options $*` 2> \ > > > > + >(grep -v "dmesg(1) may have more information after failed mount" >&2) > > > > > > _filter_error_mount()? > > > > OK, how about this change [1] ? > > > > Do you think doing this "filter out" in _try_scratch_mount is better, or in > > each case which expects mount failures in .out file (e.g. xfs/005) ? Or a new > > _try_scratch_mount_fail() ? > > I think you missed my point entirely. We already have a filter for this in > common/filter because util-linux has changed it's failure message for > EFSCORRUPTED several times: Oh, sorry, I didn't notice that there's a _filter_error_mount. I thought you hope to have one. Thanks, I'll change that function and try to find out all test cases which use mount errors as golden image. Thanks, Zorro > > # 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 > } > > > So, capture and supress the new output via _filter_error_mount() and > use that in the specific tests that are trying to mount a corrupt > filesystem and capture that specific error message in their golden > output.... > > Cheers, > > Dave. > > > -- > Dave Chinner > david@fromorbit.com >
diff --git a/common/rc b/common/rc index 70a15f9c..67342760 100644 --- a/common/rc +++ b/common/rc @@ -335,7 +335,8 @@ _try_scratch_mount() _overlay_scratch_mount $* return $? fi - _mount -t $FSTYP `_scratch_mount_options $*` + _mount -t $FSTYP `_scratch_mount_options $*` 2> \ + >(grep -v "dmesg(1) may have more information after failed mount" >&2) mount_ret=$? [ $mount_ret -ne 0 ] && return $mount_ret _idmapped_mount $SCRATCH_DEV $SCRATCH_MNT
The lastest mount command (from util-linux) merged below commit: 79534c0d7e0f ("mount: add hint about dmesg(8) to error messages") which brought in a new error output when mount fails. That cause some cases (e.g. xfs/005) fail as: mount: Structure needs cleaning dmesg(1) may have more information after failed mount system call More failed cases like generic/050, ext4/002, xfs/154, etc ... Signed-off-by: Zorro Lang <zlang@kernel.org> --- Due to _try_scratch_mount is call in many places, so I'm not sure if it's good to filter this message in _try_scratch_mount. Maybe better to filter it in each failed cases? Or write another function (e.g. _try_scratch_mount_fail) ? Welcome review points. Thanks, Zorro common/rc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)