Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 28 Jun 2022 12:24:57 +0800
From:   Zorro Lang <zlang@redhat.com>
To:     linux-ext4@vger.kernel.org
Cc:     fstests@vger.kernel.org
Subject: Re: [PATCH 1/5] ext4/050: support indirect as well as extent mapped
 journals
Message-ID: <20220628042457.zorfs2ghugnilqmm@zlang-mailbox>
References: <20220625030718.1215980-1-tytso@mit.edu>
 <20220625030718.1215980-2-tytso@mit.edu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220625030718.1215980-2-tytso@mit.edu>
Precedence: bulk

On Fri, Jun 24, 2022 at 11:07:14PM -0400, Theodore Ts'o wrote:
> Simplify the test and fix ext4/050 failures when running ext4 without
> extents enabled (e.g., in ext3 emulation mode).
> 
> Instead of relying on parsing debugfs output's (which varies depending
> on whether the journal inode is extent mapped or indirect block
> mapped), use debugfs's "cat" command to get the contents of the
> journal.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---

It looks good to me,

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

But as it contains many ext4 specific format things, so I hope to get one more
reviewing from ext4 mail list, before I merge it.

Thanks,
Zorro

>  tests/ext4/050 | 58 +++++---------------------------------------------
>  1 file changed, 5 insertions(+), 53 deletions(-)
> 
> diff --git a/tests/ext4/050 b/tests/ext4/050
> index 79961957..6f93b86d 100755
> --- a/tests/ext4/050
> +++ b/tests/ext4/050
> @@ -22,55 +22,6 @@ _require_command "$DEBUGFS_PROG" debugfs
>  checkpoint_journal=$here/src/checkpoint_journal
>  _require_test_program "checkpoint_journal"
>  
> -# convert output from stat<journal_inode> to list of block numbers
> -get_journal_extents() {
> -	inode_info=$($DEBUGFS_PROG $SCRATCH_DEV -R "stat <8>" 2>> $seqres.full)
> -	echo -e "\nJournal info:" >> $seqres.full
> -	echo "$inode_info" >> $seqres.full
> -
> -	extents_line=$(echo "$inode_info" | awk '/EXTENTS:/{ print NR; exit }')
> -	get_extents=$(echo "$inode_info" | sed -n "$(($extents_line + 1))"p)
> -
> -	# get just the physical block numbers
> -	get_extents=$(echo "$get_extents" |  perl -pe 's|\(.*?\):||g' | sed -e 's/, /\n/g' | perl -pe 's|(\d+)-(\d+)|\1 \2|g')
> -
> -	echo "$get_extents"
> -}
> -
> -# checks all extents are zero'd out except for the superblock
> -# arg 1: extents (output of get_journal_extents())
> -check_extents() {
> -	echo -e "\nChecking extents:" >> $seqres.full
> -	echo "$1" >> $seqres.full
> -
> -	super_block="true"
> -	echo "$1" | while IFS= read line; do
> -		start_block=$(echo $line | cut -f1 -d' ')
> -		end_block=$(echo $line | cut -f2 -d' ' -s)
> -
> -		# if first block of journal, shouldn't be wiped
> -		if [ "$super_block" == "true" ]; then
> -			super_block="false"
> -
> -			#if super block only block in this extent, skip extent
> -			if [ -z "$end_block" ]; then
> -				continue;
> -			fi
> -			start_block=$(($start_block + 1))
> -		fi
> -
> -		if [ ! -z "$end_block" ]; then
> -			blocks=$(($end_block - $start_block + 1))
> -		else
> -			blocks=1
> -		fi
> -
> -		check=$(od $SCRATCH_DEV --skip-bytes=$(($start_block * $blocksize)) --read-bytes=$(($blocks * $blocksize)) -An -v | sed -e 's/[0 \t\n\r]//g')
> -
> -		[ ! -z "$check" ] && echo "error" && break
> -	done
> -}
> -
>  testdir="${SCRATCH_MNT}/testdir"
>  
>  _scratch_mkfs_sized $((64 * 1024 * 1024)) >> $seqres.full 2>&1
> @@ -93,11 +44,12 @@ sync --file-system $testdir/1
>  # call ioctl to checkpoint and zero-fill journal blocks
>  $checkpoint_journal $SCRATCH_MNT --erase=zeroout || _fail "ioctl returned error"
>  
> -extents=$(get_journal_extents)
> -
>  # check journal blocks zeroed out
> -ret=$(check_extents "$extents")
> -[ "$ret" = "error" ] && _fail "Journal was not zero-filled"
> +$DEBUGFS_PROG $SCRATCH_DEV -R "cat <8>" 2> /dev/null | od >> $seqres.full
> +check=$($DEBUGFS_PROG $SCRATCH_DEV -R "cat <8>" 2> /dev/null | \
> +	    od --skip-bytes="$blocksize" -An -v | sed -e '/^[0 \t]*$/d')
> +
> +[ ! -z "$check" ] && _fail "Journal was not zeroed"
>  
>  _scratch_unmount >> $seqres.full 2>&1
>  
> -- 
> 2.31.0
>