[PATCH] xfs_admin: don't hide the xfs_repair output when upgrading

[PATCH] xfs_admin: don't hide the xfs_repair output when upgrading

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Currently, xfs_admin suppresses the output from xfs_repair when it tries
to upgrade a filesystem, and prints a rather unhelpful message if the
upgrade fails.

Neither of these behaviors are useful -- repair can fail for reasons
outside of the filesystem being mounted, and if it does, the admin will
never know what actually happened.

Worse yet, if repair finds corruptions on disk, the upgrade script
silently throws all that away, which means that nobody will ever be able
to report what happened if an upgrade trashes a filesystem.

Therefore, allow the console to capture all of repair's stdout/stderr
reports.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 db/xfs_admin.sh |   12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/db/xfs_admin.sh b/db/xfs_admin.sh
index 02f34b73..916050cb 100755
--- a/db/xfs_admin.sh
+++ b/db/xfs_admin.sh
@@ -51,17 +51,9 @@ case $# in
 		fi
 		if [ -n "$REPAIR_OPTS" ]
 		then
-			# Hide normal repair output which is sent to stderr
-			# assuming the filesystem is fine when a user is
-			# running xfs_admin.
-			# Ideally, we need to improve the output behaviour
-			# of repair for this purpose (say a "quiet" mode).
-			eval xfs_repair $REPAIR_DEV_OPTS $REPAIR_OPTS "$1" 2> /dev/null
+			echo "Running xfs_repair to upgrade filesystem."
+			eval xfs_repair $REPAIR_DEV_OPTS $REPAIR_OPTS "$1"
 			status=`expr $? + $status`
-			if [ $status -ne 0 ]
-			then
-				echo "Conversion failed, is the filesystem unmounted?"
-			fi
 		fi
 		;;
 	*)	echo $USAGE 1>&2

Re: [PATCH] xfs_admin: don't hide the xfs_repair output when upgrading

[Brian Foster]

On Fri, Mar 05, 2021 at 02:00:21PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Currently, xfs_admin suppresses the output from xfs_repair when it tries
> to upgrade a filesystem, and prints a rather unhelpful message if the
> upgrade fails.
> 
> Neither of these behaviors are useful -- repair can fail for reasons
> outside of the filesystem being mounted, and if it does, the admin will
> never know what actually happened.
> 
> Worse yet, if repair finds corruptions on disk, the upgrade script
> silently throws all that away, which means that nobody will ever be able
> to report what happened if an upgrade trashes a filesystem.
> 
> Therefore, allow the console to capture all of repair's stdout/stderr
> reports.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Personally, I prefer either the current behavior where a user can always
run xfs_repair directly to see the low level output, or even the verbose
flag option since ISTM that if repair fails, it's going to fail the next
time around for similar reasons in the most common cases. That said, I'm
not tied to any particular behavior and we've had negative feedback on
the current approach, so:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  db/xfs_admin.sh |   12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/db/xfs_admin.sh b/db/xfs_admin.sh
> index 02f34b73..916050cb 100755
> --- a/db/xfs_admin.sh
> +++ b/db/xfs_admin.sh
> @@ -51,17 +51,9 @@ case $# in
>  		fi
>  		if [ -n "$REPAIR_OPTS" ]
>  		then
> -			# Hide normal repair output which is sent to stderr
> -			# assuming the filesystem is fine when a user is
> -			# running xfs_admin.
> -			# Ideally, we need to improve the output behaviour
> -			# of repair for this purpose (say a "quiet" mode).
> -			eval xfs_repair $REPAIR_DEV_OPTS $REPAIR_OPTS "$1" 2> /dev/null
> +			echo "Running xfs_repair to upgrade filesystem."
> +			eval xfs_repair $REPAIR_DEV_OPTS $REPAIR_OPTS "$1"
>  			status=`expr $? + $status`
> -			if [ $status -ne 0 ]
> -			then
> -				echo "Conversion failed, is the filesystem unmounted?"
> -			fi
>  		fi
>  		;;
>  	*)	echo $USAGE 1>&2
>