2023-11-23 03:05:39

by Andreas Dilger

[permalink] [raw]
Subject: [PATCH] tests: make m_assume_storage_prezeroed more robust

Don't assume that mke2fs is going to zero out an exact number
of blocks when run with/without "-E assume_storage_prezeroed",
since this depends on a number of different options that are
not specified in the test script.

Instead, check that the number of blocks zeroed in the image is
a small fraction (1/40th) of the number of blocks zeroed when
"-E assume_sotrage_prezeroed" is not given, which makes it more
robust when running in different environments. This varies from
1/47 in the original test to 1/91 in my local test environment.

Avoid "losetup --sector-size 4096", use "mke2fs -b 4096" instead.
Clean up the loop device before checking "stat" so that all
blocks are flushed to the backing storage before calling sync.
Only one loop device and test file is needed for the test.

Signed-off-by: Andreas Dilger <[email protected]>
---
tests/m_assume_storage_prezeroed/expect | 2 -
tests/m_assume_storage_prezeroed/script | 64 ++++++++++++-------------
2 files changed, 30 insertions(+), 36 deletions(-)
delete mode 100644 tests/m_assume_storage_prezeroed/expect

diff --git a/tests/m_assume_storage_prezeroed/expect b/tests/m_assume_storage_prezeroed/expect
deleted file mode 100644
index b735e2425..000000000
--- a/tests/m_assume_storage_prezeroed/expect
+++ /dev/null
@@ -1,2 +0,0 @@
-> 10000
-224
diff --git a/tests/m_assume_storage_prezeroed/script b/tests/m_assume_storage_prezeroed/script
index 1a8d84635..eea881428 100644
--- a/tests/m_assume_storage_prezeroed/script
+++ b/tests/m_assume_storage_prezeroed/script
@@ -10,48 +10,44 @@ if test "$(id -u)" -ne 0 ; then
elif ! command -v losetup >/dev/null ; then
echo "$test_name: $test_description: skipped (no losetup)"
else
- dd if=/dev/zero of=$TMPFILE.1 bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1
- dd if=/dev/zero of=$TMPFILE.2 bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1
+ dd if=/dev/zero of=$TMPFILE bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1

- LOOP1=$(losetup --show --sector-size 4096 -f $TMPFILE.1)
- if [ ! -b "$LOOP1" ]; then
- echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
- rm -f $TMPFILE.1 $TMPFILE.2
- exit 0
- fi
- LOOP2=$(losetup --show --sector-size 4096 -f $TMPFILE.2)
- if [ ! -b "$LOOP2" ]; then
- echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
- rm -f $TMPFILE.1 $TMPFILE.2
- losetup -d $LOOP1
- exit 0
+ LOOP=$(losetup --show -f $TMPFILE)
+ if [ ! -b "$LOOP" ]; then
+ echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
+ rm -f $TMPFILE
+ exit 0
fi

- echo $MKE2FS -o Linux -t ext4 $LOOP1 >> $LOG 2>&1
- $MKE2FS -o Linux -t ext4 $LOOP1 >> $LOG 2>&1
+ cmd="$MKE2FS -o Linux -t ext4 -b 4096"
+ echo "$cmd $LOOP" >> $LOG
+ $cmd $LOOP >> $LOG 2>&1
+ losetup -d $LOOP
sync
- stat $TMPFILE.1 >> $LOG 2>&1
- SZ=$(stat -c "%b" $TMPFILE.1)
- if test $SZ -gt 10000 ; then
- echo "> 10000" > $OUT
- else
- echo "$SZ" > $OUT
+ stat $TMPFILE >> $LOG 2>&1
+ BLOCKS_DEF=$(stat -c "%b" $TMPFILE)
+
+ > $TMPFILE
+ dd if=/dev/zero of=$TMPFILE bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1
+ LOOP=$(losetup --show -f $TMPFILE)
+ if [ ! -b "$LOOP" ]; then
+ echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
+ rm -f $TMPFILE
+ exit 0
fi

- echo $MKE2FS -o Linux -t ext4 -E assume_storage_prezeroed=1 $LOOP2 >> $LOG 2>&1
- $MKE2FS -o Linux -t ext4 -E assume_storage_prezeroed=1 $LOOP2 >> $LOG 2>&1
+ cmd+=" -E assume_storage_prezeroed=1"
+ echo "$cmd $LOOP" >> $LOG
+ $cmd $TMPFILE >> $LOG 2>&1
+ losetup -d $LOOP
sync
- stat $TMPFILE.2 >> $LOG 2>&1
- stat -c "%b" $TMPFILE.2 >> $OUT
-
- losetup -d $LOOP1
- losetup -d $LOOP2
- rm -f $TMPFILE.1 $TMPFILE.2
+ stat $TMPFILE >> $LOG 2>&1
+ BLOCKS_ASP=$(stat -c "%b" $TMPFILE)

- cmp -s $OUT $EXP
- status=$?
+ echo "blocks_dev: $BLOCKS_DEF blocks_asp: ${BLOCKS_ASP}" >> $LOG

- if [ "$status" = 0 ] ; then
+ # should use less than 1/20 of the blocks with assume_storage_prezeroed
+ if (( $BLOCKS_DEF > $BLOCKS_ASP * 40 )) ; then
echo "$test_name: $test_description: ok"
touch $test_name.ok
else
@@ -60,4 +56,4 @@ else
diff $EXP $OUT >> $test_name.failed
fi
fi
-unset LOG OUT EXP FILE_SIZE LOOP1 LOOP2
+unset LOG OUT EXP FILE_SIZE LOOP
--
2.25.1



2024-04-01 22:19:14

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH] tests: make m_assume_storage_prezeroed more robust

On Nov 22, 2023, at 8:03 PM, Andreas Dilger <[email protected]> wrote:
>
> Don't assume that mke2fs is going to zero out an exact number
> of blocks when run with/without "-E assume_storage_prezeroed",
> since this depends on a number of different options that are
> not specified in the test script.

Another patch that looks like it fell between the cracks. This
helps the m_assume_storage_prezeroed test pass consistently on
different systems.

Cheers, Andreas

> Instead, check that the number of blocks zeroed in the image is
> a small fraction (1/40th) of the number of blocks zeroed when
> "-E assume_sotrage_prezeroed" is not given, which makes it more
> robust when running in different environments. This varies from
> 1/47 in the original test to 1/91 in my local test environment.
>
> Avoid "losetup --sector-size 4096", use "mke2fs -b 4096" instead.
> Clean up the loop device before checking "stat" so that all
> blocks are flushed to the backing storage before calling sync.
> Only one loop device and test file is needed for the test.
>
> Signed-off-by: Andreas Dilger <[email protected]>
> ---
> tests/m_assume_storage_prezeroed/expect | 2 -
> tests/m_assume_storage_prezeroed/script | 64 ++++++++++++-------------
> 2 files changed, 30 insertions(+), 36 deletions(-)
> delete mode 100644 tests/m_assume_storage_prezeroed/expect
>
> diff --git a/tests/m_assume_storage_prezeroed/expect b/tests/m_assume_storage_prezeroed/expect
> deleted file mode 100644
> index b735e2425..000000000
> --- a/tests/m_assume_storage_prezeroed/expect
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -> 10000
> -224
> diff --git a/tests/m_assume_storage_prezeroed/script b/tests/m_assume_storage_prezeroed/script
> index 1a8d84635..eea881428 100644
> --- a/tests/m_assume_storage_prezeroed/script
> +++ b/tests/m_assume_storage_prezeroed/script
> @@ -10,48 +10,44 @@ if test "$(id -u)" -ne 0 ; then
> elif ! command -v losetup >/dev/null ; then
> echo "$test_name: $test_description: skipped (no losetup)"
> else
> - dd if=/dev/zero of=$TMPFILE.1 bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1
> - dd if=/dev/zero of=$TMPFILE.2 bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1
> + dd if=/dev/zero of=$TMPFILE bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1
>
> - LOOP1=$(losetup --show --sector-size 4096 -f $TMPFILE.1)
> - if [ ! -b "$LOOP1" ]; then
> - echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
> - rm -f $TMPFILE.1 $TMPFILE.2
> - exit 0
> - fi
> - LOOP2=$(losetup --show --sector-size 4096 -f $TMPFILE.2)
> - if [ ! -b "$LOOP2" ]; then
> - echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
> - rm -f $TMPFILE.1 $TMPFILE.2
> - losetup -d $LOOP1
> - exit 0
> + LOOP=$(losetup --show -f $TMPFILE)
> + if [ ! -b "$LOOP" ]; then
> + echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
> + rm -f $TMPFILE
> + exit 0
> fi
>
> - echo $MKE2FS -o Linux -t ext4 $LOOP1 >> $LOG 2>&1
> - $MKE2FS -o Linux -t ext4 $LOOP1 >> $LOG 2>&1
> + cmd="$MKE2FS -o Linux -t ext4 -b 4096"
> + echo "$cmd $LOOP" >> $LOG
> + $cmd $LOOP >> $LOG 2>&1
> + losetup -d $LOOP
> sync
> - stat $TMPFILE.1 >> $LOG 2>&1
> - SZ=$(stat -c "%b" $TMPFILE.1)
> - if test $SZ -gt 10000 ; then
> - echo "> 10000" > $OUT
> - else
> - echo "$SZ" > $OUT
> + stat $TMPFILE >> $LOG 2>&1
> + BLOCKS_DEF=$(stat -c "%b" $TMPFILE)
> +
> + > $TMPFILE
> + dd if=/dev/zero of=$TMPFILE bs=1 count=0 seek=$FILE_SIZE >> $LOG 2>&1
> + LOOP=$(losetup --show -f $TMPFILE)
> + if [ ! -b "$LOOP" ]; then
> + echo "$test_name: $DESCRIPTION: skipped (no loop devices)"
> + rm -f $TMPFILE
> + exit 0
> fi
>
> - echo $MKE2FS -o Linux -t ext4 -E assume_storage_prezeroed=1 $LOOP2 >> $LOG 2>&1
> - $MKE2FS -o Linux -t ext4 -E assume_storage_prezeroed=1 $LOOP2 >> $LOG 2>&1
> + cmd+=" -E assume_storage_prezeroed=1"
> + echo "$cmd $LOOP" >> $LOG
> + $cmd $TMPFILE >> $LOG 2>&1
> + losetup -d $LOOP
> sync
> - stat $TMPFILE.2 >> $LOG 2>&1
> - stat -c "%b" $TMPFILE.2 >> $OUT
> -
> - losetup -d $LOOP1
> - losetup -d $LOOP2
> - rm -f $TMPFILE.1 $TMPFILE.2
> + stat $TMPFILE >> $LOG 2>&1
> + BLOCKS_ASP=$(stat -c "%b" $TMPFILE)
>
> - cmp -s $OUT $EXP
> - status=$?
> + echo "blocks_dev: $BLOCKS_DEF blocks_asp: ${BLOCKS_ASP}" >> $LOG
>
> - if [ "$status" = 0 ] ; then
> + # should use less than 1/20 of the blocks with assume_storage_prezeroed
> + if (( $BLOCKS_DEF > $BLOCKS_ASP * 40 )) ; then
> echo "$test_name: $test_description: ok"
> touch $test_name.ok
> else
> @@ -60,4 +56,4 @@ else
> diff $EXP $OUT >> $test_name.failed
> fi
> fi
> -unset LOG OUT EXP FILE_SIZE LOOP1 LOOP2
> +unset LOG OUT EXP FILE_SIZE LOOP


Cheers, Andreas






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP