2022-06-28 04:37:39

by Zorro Lang

[permalink] [raw]
Subject: Re: [PATCH 1/5] ext4/050: support indirect as well as extent mapped journals

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

It looks good to me,

Reviewed-by: Zorro Lang <[email protected]>

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
>