common/xfs: remove bad xfs_repair -t option

PATCH In review Zorro Lang 3 participants 3 messages Jul 10, 2018

Share & tools

Zorro Lang

author

2018-07-10 09:41 UTC #0 lore raw

PATCH 1/1 common/xfs: remove bad xfs_repair -t option

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 <[email protected]>

---

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(-)

common/xfs +0 -1

index ecf54bbf..61a3c2d9 100644
@@ -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
 }

Signature · 1 line

2.14.4

Darrick J. Wong

Reviewed-by on v1 1/1

2018-07-10 14:43 UTC #1 lore raw

Quoted 5 lines · 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 <[email protected]>

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

Reviewed-by Darrick J. Wong <[email protected]>

--D

Quoted 48 lines

---
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(-)
common/xfs +0 -1
index ecf54bbf..61a3c2d9 100644
@@ -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
 }
 
Signature · 6 lines
2.14.4

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Eric Sandeen

2018-07-10 14:43 UTC #2 lore raw

PATCH 1/1 common/xfs: remove bad xfs_repair -t option

In reply to Zorro Lang

Quoted 6 lines · 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 <[email protected]>
---

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 <[email protected]>

[1] commit 6c6ba523c29d78a009d237e2a019d65beb5fced8

Author: fsgqa <fsgqa>
Date:   Fri Aug 29 06:04:54 2003 +0000

QA updates to enable simplified large filesystem testing

Quoted 40 lines

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(-)
common/xfs +0 -1
index ecf54bbf..61a3c2d9 100644
@@ -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
 }
 