On Fri, Aug 26, 2016 at 02:10:00PM +0800, Xiao Yang wrote:
> On 2016/08/26 12:48, Zorro Lang wrote:
> >On Fri, Aug 26, 2016 at 11:36:52AM +0800, Xiao Yang wrote:
> >>Make sure xfs_repair can't clear the log by default when it is corrupted.
> >>xfs_repair always and only clear the log when the -L parameter is specified.
> >>This has updated by:
> >>Commit f2053bc ("xfs_repair: don't clear the log by default")
> >>
> >>Signed-off-by: Xiao Yang<yangx.jy@xxxxxxxxxxxxxx>
> >>---
> >> common/rc | 4 ++--
> >> tests/xfs/098 | 2 +-
> >> 2 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/common/rc b/common/rc
> >>index 3fb0600..c693a31 100644
> >>--- a/common/rc
> >>+++ b/common/rc
> >>@@ -1143,9 +1143,9 @@ _repair_scratch_fs()
> >Hi Xiao
> >
> >You should explain why you changed this function in commit log. Or
> >the reviewer can't understand why you change it.
> >
> >> xfs)
> >> _scratch_xfs_repair "$@" 2>&1
> >> res=$?
> >>- if [ "$res" -eq 2 ]; then
> >>+ if [ "$res" -ne 0 ]; then
> >Hi Darrick,
> >
> >The xfs_repair manpage said:
> >xfs_repair run without the -n option will always return a status code of 0.
> >
> >I don't understand why you think it return 2 here? (Please check below)
> >
> Hi Zorro
>
> I don't understand why it return 2 here too. I want to change this
> function because xfs_repair
> without -L option return 1 when log is corrupted on newer xfsprogs-dev.
> >> echo "xfs_repair returns $res; replay log?"
> >>- _scratch_mount
> >>+ _scratch_mount 2>&1
> >> res=$?
> >> if [ "$res" -gt 0 ]; then
> >> echo "mount returns $res; zap log?"
> >>diff --git a/tests/xfs/098 b/tests/xfs/098
> >>index d91d617..eb33bb1 100755
> >>--- a/tests/xfs/098
> >>+++ b/tests/xfs/098
> >>@@ -93,7 +93,7 @@ echo "+ mount image"
> >> _scratch_mount 2>/dev/null&& _fail "mount should not succeed"
> >>
> >> echo "+ repair fs"
> >>-_scratch_xfs_repair>> $seqres.full 2>&1
> >>+_repair_scratch_fs>> $seqres.full
You should print the stderr to $seqres.full too. Because in
"_repair_scratch_fs", its code likes below:
xfs)
_scratch_xfs_repair "$@" 2>&1
>>> This repair won't clear the corrupted log anymore.
res=$?
if [ "$res" -eq 2 ]; then
echo "xfs_repair returns $res; replay log?"
_scratch_mount
>>> So this mount maybe failed if it can't deal with the corrupted log.
>>> If it print some error messages, it'll break the golden image of xfs/098
res=$?
if [ "$res" -gt 0 ]; then
echo "mount returns $res; zap log?"
_scratch_xfs_repair -L 2>&1
> >If just call xfs_repair without any options, the _repair_scratch_fs won't
> >help to call xfs_repair -L I think.
> >
> >So I think this patch won't fix the problem.
> >
> >Feel free to correct me, if I misunderstand something:)
> >
> >Thanks,
> >Zorro
> >
> If xfs_repair without any option succeed to repair filesystem when
> log is corrupted,
> _repair_scratch_fs don't need to call xfs_repair -L. If it failed
> to repair filesystem,
> _repair_scratch_fs needs to call xfs_repair -L.
Oh, sorry, I just tried to run ths case. The "_scratch_xfs_repair" really return
non-zero when it try to repair a corrupted xfs...
But the manpage(man xfs_repair) really said:
xfs_repair run without the -n option will always return a status code of 0.
Maybe we should update the manpage? I'll check it later.
Any way, there's still a problem in your patch, please see above:
Thanks,
Zorro
>
> Thanks
> Xiao Yang.
> >>
> >> echo "+ mount image (2)"
> >> _scratch_mount
> >>--
> >>1.8.3.1
> >>
> >>
> >>
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe fstests" in
> >>the body of a message to majordomo@xxxxxxxxxxxxxxx
> >>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> >.
> >
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
