On Tue, Jan 02, 2024 at 09:57:28 PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 02, 2024 at 02:13:51PM +0530, Chandan Babu R wrote:
>> This commit adds the ability to test metadump v2 to existing metadump tests.
>>
>> Signed-off-by: Chandan Babu R <chandanbabu@xxxxxxxxxx>
>> ---
>> tests/xfs/129 | 63 ++++++++++++---
>> tests/xfs/129.out | 4 +-
>> tests/xfs/234 | 63 ++++++++++++---
>> tests/xfs/234.out | 4 +-
>> tests/xfs/253 | 195 ++++++++++++++++++++++++++--------------------
>> tests/xfs/291 | 25 +++++-
>> tests/xfs/432 | 29 +++++--
>> tests/xfs/432.out | 3 +-
>> tests/xfs/503 | 94 +++++++++++++---------
>> tests/xfs/503.out | 12 +--
>> 10 files changed, 326 insertions(+), 166 deletions(-)
>>
>> diff --git a/tests/xfs/129 b/tests/xfs/129
>> index 6f2ef564..7226d57d 100755
>> --- a/tests/xfs/129
>> +++ b/tests/xfs/129
>> @@ -16,7 +16,11 @@ _cleanup()
>> {
>> cd /
>> _scratch_unmount > /dev/null 2>&1
>> - rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image
>> + [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
>> + _destroy_loop_device $logdev
>> + [[ -n $datadev ]] && _destroy_loop_device $datadev
>> + rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/data-image \
>> + $TEST_DIR/log-image
>> }
>>
>> # Import common functions.
>> @@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
>> $testdir/file2 $((nr * blksz)) $blksz >> $seqres.full
>> done
>>
>> -echo "Create metadump file"
>> _scratch_unmount
>> -_scratch_xfs_metadump $metadump_file
>>
>> -# Now restore the obfuscated one back and take a look around
>> -echo "Restore metadump"
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_mount
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
>> +max_md_version=1
>> +_scratch_metadump_v2_supported && max_md_version=2
>>
>> -echo "Check restored fs"
>> -_check_generic_filesystem $metadump_file
>> +echo "Create metadump file, restore it and check restored fs"
>> +for md_version in $(seq 1 $max_md_version); do
>> + # Determine the version to be passed to metadump/mdrestore
>> + version=""
>> + if [[ $max_md_version == 2 ]]; then
>> + version="-v $md_version"
>> + fi
>> +
>> + _scratch_xfs_metadump $metadump_file $version
>> +
>> + # Now restore the obfuscated one back and take a look around
>> +
>> + # Metadump v2 files can contain contents dumped from an external log
>> + # device. Use a temporary file to hold the log device contents restored
>> + # from such a metadump file.
>> + slogdev=$TEST_DIR/log-image
>> + if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
>> + slogdev=""
>> + fi
>> +
>> + SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \
>
> ^ space before tab
>
Sorry, I will fix it.
>> + _scratch_xfs_mdrestore $metadump_file
>> +
>> + datadev=$(_create_loop_device $TEST_DIR/data-image)
>> +
>> + logdev=${SCRATCH_LOGDEV}
>> + if [[ -s $TEST_DIR/log-image ]]; then
>> + logdev=$(_create_loop_device $TEST_DIR/log-image)
>> + fi
>> +
>> + SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
>> + SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
>> +
>> + [[ -z $logdev ]] && logdev=none
>> + _check_xfs_filesystem $datadev $logdev none
>> +
>> + if [[ -s $TEST_DIR/log-image ]]; then
>> + _destroy_loop_device $logdev
>> + logdev=""
>> + rm -f $TEST_DIR/log-image
>> + fi
>> +
>> + _destroy_loop_device $datadev
>> + datadev=""
>> + rm -f $TEST_DIR/data-image
>> +done
>
> Given the major differences between v1 and v2, would the two metadump
> tests be easier to understand if this loop were unrolled?
>
Yes, you are right. Unrolling the loop will improve the readability of the
code. I will make the required changes.
> I like that it mdrestores to a separate device to preserve the original
> $SCRATCH_DEV metadata between metadump v1 and v2 testing.
>
>> # success, all done
>> status=0
>> diff --git a/tests/xfs/129.out b/tests/xfs/129.out
>> index da6f43fd..0f24c431 100644
>> --- a/tests/xfs/129.out
>> +++ b/tests/xfs/129.out
>> @@ -1,6 +1,4 @@
>> QA output created by 129
>> Create the original file blocks
>> Reflink every other block
>> -Create metadump file
>> -Restore metadump
>> -Check restored fs
>> +Create metadump file, restore it and check restored fs
>> diff --git a/tests/xfs/234 b/tests/xfs/234
>> index 57d447c0..2f6b1f65 100755
>> --- a/tests/xfs/234
>> +++ b/tests/xfs/234
>> @@ -16,7 +16,11 @@ _cleanup()
>> {
>> cd /
>> _scratch_unmount > /dev/null 2>&1
>> - rm -rf $tmp.* $metadump_file $TEST_DIR/image
>> + [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
>> + _destroy_loop_device $logdev
>> + [[ -n $datadev ]] && _destroy_loop_device $datadev
>> + rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image \
>> + $TEST_DIR/log-image
>> }
>>
>> # Import common functions.
>> @@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
>> $XFS_IO_PROG -c "fpunch $((nr * blksz)) $blksz" $testdir/file1 >> $seqres.full
>> done
>>
>> -echo "Create metadump file"
>> _scratch_unmount
>> -_scratch_xfs_metadump $metadump_file
>>
>> -# Now restore the obfuscated one back and take a look around
>> -echo "Restore metadump"
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_mount
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
>> +max_md_version=1
>> +_scratch_metadump_v2_supported && max_md_version=2
>>
>> -echo "Check restored fs"
>> -_check_generic_filesystem $metadump_file
>> +echo "Create metadump file, restore it and check restored fs"
>> +for md_version in $(seq 1 $max_md_version); do
>> + # Determine the version to be passed to metadump/mdrestore
>> + version=""
>> + if [[ $max_md_version == 2 ]]; then
>> + version="-v $md_version"
>> + fi
>> +
>> + _scratch_xfs_metadump $metadump_file $version
>> +
>> + # Now restore the obfuscated one back and take a look around
>> +
>> + # Metadump v2 files can contain contents dumped from an external log
>> + # device. Use a temporary file to hold the log device contents restored
>> + # from such a metadump file.
>> + slogdev=$TEST_DIR/log-image
>> + if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
>> + slogdev=""
>> + fi
>> +
>> + SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \
>> + _scratch_xfs_mdrestore $metadump_file
>> +
>> + datadev=$(_create_loop_device $TEST_DIR/data-image)
>> +
>> + logdev=${SCRATCH_LOGDEV}
>> + if [[ -s $TEST_DIR/log-image ]]; then
>> + logdev=$(_create_loop_device $TEST_DIR/log-image)
>> + fi
>> +
>> + SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
>> + SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
>> +
>> + [[ -z $logdev ]] && logdev=none
>> + _check_xfs_filesystem $datadev $logdev none
>> +
>> + if [[ -s $TEST_DIR/log-image ]]; then
>> + _destroy_loop_device $logdev
>> + logdev=""
>> + rm -f $TEST_DIR/log-image
>> + fi
>> +
>> + _destroy_loop_device $datadev
>> + datadev=""
>> + rm -f $TEST_DIR/data-image
>> +done
>>
>> # success, all done
>> status=0
>> diff --git a/tests/xfs/234.out b/tests/xfs/234.out
>> index 463d4660..fc2ddd77 100644
>> --- a/tests/xfs/234.out
>> +++ b/tests/xfs/234.out
>> @@ -1,6 +1,4 @@
>> QA output created by 234
>> Create the original file blocks
>> Punch every other block
>> -Create metadump file
>> -Restore metadump
>> -Check restored fs
>> +Create metadump file, restore it and check restored fs
>> diff --git a/tests/xfs/253 b/tests/xfs/253
>> index ce902477..b69a1faf 100755
>> --- a/tests/xfs/253
>> +++ b/tests/xfs/253
>> @@ -52,114 +52,139 @@ function create_file() {
>> echo "Disciplyne of silence is goed."
>>
>> _scratch_mkfs >/dev/null 2>&1
>> -_scratch_mount
>>
>> -# Initialize and mount the scratch filesystem, then create a bunch
>> -# of files that exercise the original problem.
>> -#
>> -# The problem arose when a file name produced a hash that contained
>> -# either 0x00 (string terminator) or 0x27 ('/' character) in a
>> -# spot used to determine a character in an obfuscated name. This
>> -# occurred in one of 5 spots at the end of the name, at position
>> -# (last-4), (last-3), (last-2), (last-1), or (last).
>
> I wonder, could you create the scratch fs and only then move into
> testing v1 and v2 metadump? Rather than doing the create_file stuff
> twice? Or do we actually end up with a different fs between the two
> iterations?
>
Yes, Creating the fs once should be sufficient. I will implement the changes
that have been suggested.
> The other two tests here look good enough for now...
>
> --D
>
--
Chandan
