[PATCH] common/xfs: remove bad xfs_repair -t option

[PATCH] common/xfs: remove bad xfs_repair -t option

[Zorro Lang]

The xfs_repair "-t" option shouldn't be used alone. An interval must
follow the -t option, or xfs_repair will report errors. And only
modify reporting interval is useless, if we don't enable ag_stride.

Signed-off-by: Zorro Lang <zlang@redhat.com>
---

Hi,

I don't know why we must need the -t option for xfs_repair, I can't
find any description to explain it. But I can find an explanation
about why we use "-t" for _xfs_check.

  # xfs_check runs out of memory on large files, so even providing the test
  # option (-t) to avoid indexing the free space trees doesn't make it pass on
  # large filesystems. Avoid it.

The -t option for xfs_repair is totally different with it for
xfs_check, maybe -m option is more useful if we think about the
memory size.

And the -t option need to work with -o ag_stride together. I'd like
to remove the "-t" option directly, due to I really don't know why
we need it, or how to give it a proper number.

If the original author knows why we need it, and can give me some
suggestions, please help.

Thanks,
Zorro

 common/xfs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/common/xfs b/common/xfs
index ecf54bbf..61a3c2d9 100644
--- a/common/xfs
+++ b/common/xfs
@@ -193,7 +193,6 @@ _scratch_xfs_repair()
 		SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
 	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
 		SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
-	[ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
 	$XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
 }
 
-- 
2.14.4

Re: [PATCH] common/xfs: remove bad xfs_repair -t option

[Darrick J. Wong]

On Tue, Jul 10, 2018 at 05:41:29PM +0800, Zorro Lang wrote:
> The xfs_repair "-t" option shouldn't be used alone. An interval must
> follow the -t option, or xfs_repair will report errors. And only
> modify reporting interval is useless, if we don't enable ag_stride.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>

Makes sense to me, at least given the getopt arg ("t:") in xfs_repair.c.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> Hi,
> 
> I don't know why we must need the -t option for xfs_repair, I can't
> find any description to explain it. But I can find an explanation
> about why we use "-t" for _xfs_check.
> 
>   # xfs_check runs out of memory on large files, so even providing the test
>   # option (-t) to avoid indexing the free space trees doesn't make it pass on
>   # large filesystems. Avoid it.
> 
> The -t option for xfs_repair is totally different with it for
> xfs_check, maybe -m option is more useful if we think about the
> memory size.
> 
> And the -t option need to work with -o ag_stride together. I'd like
> to remove the "-t" option directly, due to I really don't know why
> we need it, or how to give it a proper number.
> 
> If the original author knows why we need it, and can give me some
> suggestions, please help.
> 
> Thanks,
> Zorro
> 
>  common/xfs | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/common/xfs b/common/xfs
> index ecf54bbf..61a3c2d9 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -193,7 +193,6 @@ _scratch_xfs_repair()
>  		SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
>  		SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
> -	[ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
>  	$XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
>  }
>  
> -- 
> 2.14.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] common/xfs: remove bad xfs_repair -t option

[Eric Sandeen]

On 7/10/18 4:41 AM, Zorro Lang wrote:
> The xfs_repair "-t" option shouldn't be used alone. An interval must
> follow the -t option, or xfs_repair will report errors. And only
> modify reporting interval is useless, if we don't enable ag_stride.
> 
> Signed-off-by: Zorro Lang <zlang@redhat.com>
> ---

I agree, -t looks like it should only have been used for xfs_check.
This seems to go back a long way in history [1], but I think it has always
been broken.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>


[1] commit 6c6ba523c29d78a009d237e2a019d65beb5fced8
Author: fsgqa <fsgqa>
Date:   Fri Aug 29 06:04:54 2003 +0000

    QA updates to enable simplified large filesystem testing

> Hi,
> 
> I don't know why we must need the -t option for xfs_repair, I can't
> find any description to explain it. But I can find an explanation
> about why we use "-t" for _xfs_check.
> 
>   # xfs_check runs out of memory on large files, so even providing the test
>   # option (-t) to avoid indexing the free space trees doesn't make it pass on
>   # large filesystems. Avoid it.
> 
> The -t option for xfs_repair is totally different with it for
> xfs_check, maybe -m option is more useful if we think about the
> memory size.
> 
> And the -t option need to work with -o ag_stride together. I'd like
> to remove the "-t" option directly, due to I really don't know why
> we need it, or how to give it a proper number.
> 
> If the original author knows why we need it, and can give me some
> suggestions, please help.
> 
> Thanks,
> Zorro
> 
>  common/xfs | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/common/xfs b/common/xfs
> index ecf54bbf..61a3c2d9 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -193,7 +193,6 @@ _scratch_xfs_repair()
>  		SCRATCH_OPTIONS="-l$SCRATCH_LOGDEV"
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
>  		SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -r$SCRATCH_RTDEV"
> -	[ "$LARGE_SCRATCH_DEV" = yes ] && SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
>  	$XFS_REPAIR_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
>  }
>  
>