Re: [PATCH] fsck.xfs: mount/umount xfs fs to replay log before running xfs_repair

[Dave Chinner <david@fromorbit.com>]

On Mon, Oct 10, 2022 at 03:40:51PM +0000, Darrick Wong wrote:
> LGTM, want to send this to the upstream list to start that discussion?
> 
> --D
> 
> ________________________________________
> From: Srikanth C S <srikanth.c.s@oracle.com>
> Sent: Monday, October 10, 2022 08:24
> To: linux-xfs@vger.kernel.org; Darrick Wong
> Cc: Rajesh Sivaramasubramaniom; Junxiao Bi
> Subject: [PATCH] fsck.xfs: mount/umount xfs fs to replay log before running xfs_repair
> 
> fsck.xfs does xfs_repair -e if fsck.mode=force is set. It is
> possible that when the machine crashes, the fs is in inconsistent
> state with the journal log not yet replayed. This can put the
> machine into rescue shell. To address this problem, mount and
> umount the fs before running xfs_repair.

What's the purpose of forcing xfs_repair to be run on every boot?
The whole point of having a journalling filesystem is to avoid
needing to run fsck on every boot.

I get why one might want to occasionally force a repair check on
boot (e.g. to repair a problem with the root filesystem), but this
is a -rescue operation- and really shouldn't be occurring
automatically on every boot or after a kernel crash.

If it is only occurring during rescue operations, then why is it a problem
dumping out to a shell for the admin performing rescue
operations to deal with this directly? e.g. if the fs has a
corrupted journal, then a mount cycle will not fix the problem and
the admin will still get dumped into a rescue shell to fix the
problem manually.

Hence I don't really know why anyone would be configuring their
systems like this:

> Run xfs_repair -e when fsck.mode=force and repair=auto or yes.

as it makes no sense at all for a journalling filesystem.

> If fsck.mode=force and fsck.repair=no, run xfs_repair -n without
> replaying the logs.

Nor is it clear why anyone would want force a boot time fsck and
then not repair the damage that might be found....

More explanation, please!

> Signed-off-by: Srikanth C S <srikanth.c.s@oracle.com>
> ---
>  fsck/xfs_fsck.sh | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/fsck/xfs_fsck.sh b/fsck/xfs_fsck.sh
> index 6af0f22..21a8c19 100755
> --- a/fsck/xfs_fsck.sh
> +++ b/fsck/xfs_fsck.sh
> @@ -63,8 +63,24 @@ if [ -n "$PS1" -o -t 0 ]; then
>  fi
> 
>  if $FORCE; then
> -       xfs_repair -e $DEV
> -       repair2fsck_code $?
> +       if $AUTO; then
> +               xfs_repair -e $DEV
> +                error=$?
> +                if [ $error -eq 2 ]; then
> +                        echo "Replaying log for $DEV"
> +                        mkdir -p /tmp/tmp_mnt
> +                        mount $DEV /tmp/tmp_mnt
> +                        umount /tmp/tmp_mnt
> +                        xfs_repair -e $DEV
> +                        error=$?
> +                        rmdir /tmp/tmp_mnt
> +                fi
> +        else
> +                #fsck.mode=force is set but fsck.repair=no
> +                xfs_repair -n $DEV
> +                error=$?
> +        fi
> +       repair2fsck_code $error
>          exit $?
>  fi

As a side note, the patch has damaged whitespace....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com