On Thu, Feb 20, 2025 at 03:20:29PM +0800, Boyang Xue wrote:
About the subject "ext4: add test case 061 for ext3 to ext4 conversion",
the case number is not fixed before merging, so better to change it as:
"ext4: test conversion from ext3 to ext4".
And do you have more detailed commit log at here?
> Signed-off-by: Boyang Xue <bxue@xxxxxxxxxx>
> ---
> tests/ext4/061 | 63 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/ext4/061.out | 17 +++++++++++++
> 2 files changed, 80 insertions(+)
> create mode 100755 tests/ext4/061
> create mode 100644 tests/ext4/061.out
>
> diff --git a/tests/ext4/061 b/tests/ext4/061
> new file mode 100755
> index 00000000..f42f2a92
> --- /dev/null
> +++ b/tests/ext4/061
> @@ -0,0 +1,63 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2025 Red Hat Inc. All Rights Reserved.
> +#
> +# FS QA Test 061
> +#
> +# Test conversion from ext3 to ext4 filesystem with various mount options
> +#
> +. ./common/preamble
> +_begin_fstest auto
quick?
And the doc/group-names.txt has one line:
convert btrfs ext[34] conversion tool
currently the "convert" group is only used for btrfs, but it metions ext[34].
So I'm wondering if we should add this group to this test.
CC ext4 list to get more reviewing.
> +
> +# Import common functions.
> +. ./common/filter
Do you use any filter helpers ?
> +
> +_supported_fs ext3 ext4
There's not _supported_fs helper anymore, if you're sure ext2 isn't suit for
this test, you can use _exclude_fs. But from your below codes, you don't need
it.
> +
> +_require_scratch
> +
> +MOUNT_OPTS_LIST=(
> + ""
> + "noatime"
> + "data=journal"
> + "data=writeback"
Why does this case test these 3 mount options particularly, not others?
> +)
> +
> +_cleanup()
> +{
> + if _is_dev_mounted "$SCRATCH_DEV" > /dev/null 2>&1; then
> + $UMOUNT_PROG "$SCRATCH_DEV" || _fail "_cleanup: umount failed"
Is this a test step or a cleanup step? If it's a test step, please move
it to offical test context. If it's a cleanup step, it's useless, due to
xfstests always trys to umount $SCRATCH_DEV.
This looks not like a _cleanup step, more likes a test step. So better
to move it to offical test context.
> + fi
> +}
> +
> +
> +fill_fs() {
> + echo "Filling filesystem..."
> + while true; do
> + dd if=/dev/urandom of="${SCRATCH_MNT}/file$(date +%s)" bs=1M count=4 \
> +> /dev/null 2>&1 || _fail "dd failed"
> + df -h ${SCRATCH_MNT} | awk 'NR==2 {print $5}' | grep -q '[9][0-9]%' \
> +&& break
> + done
> + echo "Filesystem almost full."
> +}
Can _fill_fs (in common/populate) help that?
> +
> +for mount_opt in "${MOUNT_OPTS_LIST[@]}"; do
> + echo "Starting test with mount options: '$mount_opt'"
> + mkfs.ext3 -F "$SCRATCH_DEV" 1g >> $seqres.full 2>&1 \
> +|| _fail "mkfs.ext3 failed"
> + mount -t ext3 -o "$mount_opt" "$SCRATCH_DEV" "$SCRATCH_MNT" \
> + >> $seqres.full 2>&1 || _fail "mount ext3 failed"
> + fill_fs
> + umount "$SCRATCH_MNT" >> $seqres.full 2>&1 || _fail "umount ext3 failed"
> + tune2fs -O extents,uninit_bg,dir_index "$SCRATCH_DEV" \
> +>> $seqres.full 2>&1 || _fail "tune2fs failed"
_require_command "$TUNE2FS_PROG" tune2fs
> + e2fsck -f -y "$SCRATCH_DEV" >> $seqres.full 2>&1 || _fail "e2fsck failed"
> + mount -t ext4 "$SCRATCH_DEV" "$SCRATCH_MNT" >> $seqres.full 2>&1 \
> +|| _fail "mount ext4 failed"
> + umount "$SCRATCH_MNT" >> $seqres.full 2>&1 || _fail "umount ext3 failed"
> + echo "Test with mount options: '$mount_opt' completed successfully."
> +done
How about using common helpers. Something likes:
# prepare an ext3
_scratch_mkfs -t ext3 >> $seqres.full 2>&1 || _fail "Fail to create ext3"
_scratch_mount
_fill_fs
_scratch_unmount
# convert ext3 to ext4
$TUNE2FS_PROG -O extents,uninit_bg,dir_index $SCRATCH_DEV
_check_scratch_fs
_scratch_mount
_scratch_unmount
(CC ext4 list too)
Thanks,
Zorro
> +
> +status=0
> +exit
> diff --git a/tests/ext4/061.out b/tests/ext4/061.out
> new file mode 100644
> index 00000000..ed2997a0
> --- /dev/null
> +++ b/tests/ext4/061.out
> @@ -0,0 +1,17 @@
> +QA output created by 061
> +Starting test with mount options: ''
> +Filling filesystem...
> +Filesystem almost full.
> +Test with mount options: '' completed successfully.
> +Starting test with mount options: 'noatime'
> +Filling filesystem...
> +Filesystem almost full.
> +Test with mount options: 'noatime' completed successfully.
> +Starting test with mount options: 'data=journal'
> +Filling filesystem...
> +Filesystem almost full.
> +Test with mount options: 'data=journal' completed successfully.
> +Starting test with mount options: 'data=writeback'
> +Filling filesystem...
> +Filesystem almost full.
> +Test with mount options: 'data=writeback' completed successfully.
> --
> 2.48.1
>
>
