Re: [PATCH 3/5] populate: reformat external ext[34] journal devices when restoring a cached image

From: Zorro Lang <zlang@redhat.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: linux-xfs@vger.kernel.org, fstests@vger.kernel.org
Subject: Re: [PATCH 3/5] populate: reformat external ext[34] journal devices when restoring a cached image
Date: Thu, 13 Oct 2022 23:47:17 +0800	[thread overview]
Message-ID: <20221013154717.tmg4i2ebkmyzew2r@zlang-mailbox> (raw)
In-Reply-To: <166553913911.422450.17214876114235793554.stgit@magnolia>

On Tue, Oct 11, 2022 at 06:45:39PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> The fs population code has the ability to save cached metadumps of
> filesystems to save time when running fstests.  The cached images should
> be unmounted cleanly, so we never save the contents of external journal
> devices.
> 
> Unfortunately, the cache restore code fails to reset the external
> journal when restoring a clean image, so we ignore cached images because
> the journal doesn't match the filesystem.  This makes test runtimes
> longer than they need to be.
> 
> Solve this by reformatting the external journal to match the filesystem.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/populate |   20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/common/populate b/common/populate
> index 0bd78e0a0a..66c55b682f 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -16,6 +16,9 @@ _require_populate_commands() {
>  		_require_command "$XFS_DB_PROG" "xfs_db"
>  		_require_command "$WIPEFS_PROG" "wipefs"
>  		;;
> +	ext*)
> +		_require_command "$DUMPE2FS_PROG" "dumpe2fs"
> +		;;
>  	esac
>  }
>  
> @@ -871,9 +874,20 @@ _scratch_populate_restore_cached() {
>  		return $res
>  		;;
>  	"ext2"|"ext3"|"ext4")
> -		# ext4 cannot e2image external logs, so we cannot restore
> -		test -n "${SCRATCH_LOGDEV}" && return 1
> -		e2image -r "${metadump}" "${SCRATCH_DEV}" && return 0
> +		e2image -r "${metadump}" "${SCRATCH_DEV}"
> +		ret=$?
> +		test $ret -ne 0 && return $ret
> +
> +		# ext4 cannot e2image external logs, so we have to reformat
> +		# the scratch device to match the restored fs
> +		if [ -n "${SCRATCH_LOGDEV}" ]; then
> +			local fsuuid="$($DUMPE2FS_PROG -h "${SCRATCH_DEV}" 2>/dev/null | \
> +					grep 'Journal UUID:' | \
> +					sed -e 's/Journal UUID:[[:space:]]*//g')"

This patch looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

Just ask, how about combine that grep and sed lines to:

  sed -n '/Journal UUID:/s/Journal UUID:[[:space:]]*//p'

That's not a big deal, both are OK to me.

Thanks,
Zorro

> +			$MKFS_EXT4_PROG -O journal_dev "${SCRATCH_LOGDEV}" \
> +					-F -U "${fsuuid}"
> +		fi
> +		return 0
>  		;;
>  	esac
>  	return 1
> 