| Message ID | 20250220072226.175071-1-bxue@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] ext4: add test case 061 for ext3 to ext4 conversion | expand |
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@redhat.com> > --- > 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 > >
I'm curious why this is something that you care about this particular case? Historically, Red Hat (nor any other enterprise distribution) has supported changing or adding ext4 feature flags. What users were told to do was to create a new ext4 file system, and then copy the files (or do a backup/restore) to the new file system. Over a decade ago, when I was at Google, we did do a transition of file sytems from ext2 to ext4 (in no-journal mode) for all files stored in our cluster file systems. Our experience for our partcular workload is that the performance delta between an existing file system that was formatted w/o extents, and uninit_bg, and then converted to "ext4" using tune2fs, was roughly half the performance improvement of a file system that was freshly formatted to use ext4 once the file system was filled to roughly the same level, so that file system aging wastaken into account. So users who do the conversion won't get the full benefits of ext4. Also, the exact set of file system features that we changed was different from from what you are testing. I'd also note that this test is just filling a 1G file system, then changing the file system features, and then running fsck on the file system, and then trying to mount the file system. This isn't actually a very exhaustive test of doing a "conversion". If you really want to do a "coversion" test, what I'd recommend is to fill the TEST_DEV to be about 50% full, and then make the changes to the file system features, and then running the xfstests using the ext4 file system type without SCRATCH_DEV being set up. This would do a much better job of testing this case, without needing to add a new test to xfstest that to be honest, isn't really all that effective at testing this case. What did we do, 10+ year ago? Well, we ran xfstests using the a file system config[1] that didn't have flex_bg set, since that's one of the primary differences when using a "converted" file system. We then using tune2fs to convert 1% of the file systems in a single failure domain of the cluster file system, and watched looking for potential problems, and measuring for any performances gains (or losses). We then gradually increased the percentage of file systems converted to 2%, 5%, etc which is considered best practice[2] when doing this kind of feature rollout. [1] https://github.com/tytso/xfstests-bld/blob/master/test-appliance/files/root/fs/ext4/cfg/ext3conv [2] https://books.google.com/books?id=81UrjwEACAAJ Cheers, - Ted
Hi Ted, Thanks for your reply. I learned a lot from the text. On Thu, Feb 20, 2025 at 11:16 PM Theodore Ts'o <tytso@mit.edu> wrote: > > I'm curious why this is something that you care about this particular > case? Historically, Red Hat (nor any other enterprise distribution) > has supported changing or adding ext4 feature flags. What users were > told to do was to create a new ext4 file system, and then copy the > files (or do a backup/restore) to the new file system. We had learned we did have customers insist on doing the conversion rather than creating a fresh new ext4 and copying files for some reason, so we hope our test can cover this use case. > Over a decade ago, when I was at Google, we did do a transition of > file sytems from ext2 to ext4 (in no-journal mode) for all files > stored in our cluster file systems. Our experience for our partcular > workload is that the performance delta between an existing file system > that was formatted w/o extents, and uninit_bg, and then converted to > "ext4" using tune2fs, was roughly half the performance improvement of > a file system that was freshly formatted to use ext4 once the file > system was filled to roughly the same level, so that file system aging > wastaken into account. > > So users who do the conversion won't get the full benefits of ext4. > Also, the exact set of file system features that we changed was > different from from what you are testing. I'd also note that this > test is just filling a 1G file system, then changing the file system > features, and then running fsck on the file system, and then trying to > mount the file system. > > This isn't actually a very exhaustive test of doing a "conversion". > If you really want to do a "coversion" test, what I'd recommend is to > fill the TEST_DEV to be about 50% full, and then make the changes to > the file system features, and then running the xfstests using the ext4 > file system type without SCRATCH_DEV being set up. This would do a > much better job of testing this case, without needing to add a new > test to xfstest that to be honest, isn't really all that effective at > testing this case. I'm curious why the TEST_DEV is about 50% full is more effective at testing this case? > > What did we do, 10+ year ago? Well, we ran xfstests using the a file > system config[1] that didn't have flex_bg set, since that's one of the > primary differences when using a "converted" file system. We then > using tune2fs to convert 1% of the file systems in a single failure > domain of the cluster file system, and watched looking for potential > problems, and measuring for any performances gains (or losses). We > then gradually increased the percentage of file systems converted to > 2%, 5%, etc which is considered best practice[2] when doing this kind > of feature rollout. > I'm wondering is the [1] ext4 config can be used to simulated a tune2fs converted ext3 fs? > [1] https://github.com/tytso/xfstests-bld/blob/master/test-appliance/files/root/fs/ext4/cfg/ext3conv > [2] https://books.google.com/books?id=81UrjwEACAAJ > > Cheers, > > - Ted > Cheers, Boyang
On Fri, Feb 21, 2025 at 01:06:04AM +0800, Boyang Xue wrote: > > This isn't actually a very exhaustive test of doing a "conversion". > > If you really want to do a "coversion" test, what I'd recommend is to > > fill the TEST_DEV to be about 50% full, and then make the changes to > > the file system features, and then running the xfstests using the ext4 > > file system type without SCRATCH_DEV being set up. This would do a > > much better job of testing this case, without needing to add a new > > test to xfstest that to be honest, isn't really all that effective at > > testing this case. > > I'm curious why the TEST_DEV is about 50% full is more effective at > testing this case? The reason why this is useful is because we are starting with a file system that 50% of the space filled with files using indirect files, and when you run the tests, you are created and deleting files using extents. That's actualy not perfect. What we would want to do is to create ext2/ext3-style files using indirect block maps, that would then be mutated using fstress and fsx in parallel with files being created, mutated, and deleted using extent-mapped files. The basic idea is that you want to actually exercise *using* the file system after it is converted -- reading and writing files, allocating and dealocating blocks and inodes, after converting the file sytem -- and then making sure that the kernel doesn't crash or result in a corrupted file system. Your proposed test just fills the file system, unmounts it, uses tune2fs to add the extents feature flag, and then runs e2fsck on the file system, and then tries mounting the file system. That's pretty much guaranteed to work, since running fsck.ext4 on a file system that was originally created as ext3, with lots of indirect mapped files, and then adding the extent flag, essentially exercises *zero* new or different code paths. Cheers, - Ted
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 + +# Import common functions. +. ./common/filter + +_supported_fs ext3 ext4 + +_require_scratch + +MOUNT_OPTS_LIST=( + "" + "noatime" + "data=journal" + "data=writeback" +) + +_cleanup() +{ + if _is_dev_mounted "$SCRATCH_DEV" > /dev/null 2>&1; then + $UMOUNT_PROG "$SCRATCH_DEV" || _fail "_cleanup: umount failed" + 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." +} + +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" + 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 + +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.
Signed-off-by: Boyang Xue <bxue@redhat.com> --- 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