1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information

Max Reitz posted 1 patch 4 years, 11 months ago
Test asan passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190513155254.27773-1-mreitz@redhat.com
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
There is a newer version of this series
tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
tests/qemu-iotests/175.out |  8 ++++----
2 files changed, 26 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information
Posted by Max Reitz 4 years, 11 months ago 
It is possible for an empty file to take up blocks on a filesystem.
Make iotest 175 take this into account.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
v2: [Nir]
- Use a function for filtering
- s/empty_blocks/extra_blocks/
---
 tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
 tests/qemu-iotests/175.out |  8 ++++----
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
index d0ffc495c2..b5eb0aa856 100755
--- a/tests/qemu-iotests/175
+++ b/tests/qemu-iotests/175
@@ -28,10 +28,25 @@ status=1	# failure is the default!
 
 _cleanup()
 {
-	_cleanup_test_img
+    _cleanup_test_img
+    rm -f "$TEST_DIR/empty"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
+# Some file systems sometimes allocate extra blocks independently of
+# the file size.  This function hides the resulting difference in the
+# stat -c '%b' output.
+# Parameter 1: Number of blocks an empty file occupies
+# Parameter 2: Image size in bytes
+_filter_blocks()
+{
+    extra_blocks=$1
+    img_size=$2
+
+    sed -e "s/blocks=$extra_blocks/nothing allocated/" \
+        -e "s/blocks=$((extra_blocks + img_size / 512))/everything allocated/"
+}
+
 # get standard environment, filters and checks
 . ./common.rc
 . ./common.filter
@@ -40,18 +55,21 @@ _supported_fmt raw
 _supported_proto file
 _supported_os Linux
 
-size=1m
+size=$((1 * 1024 * 1024))
+
+touch "$TEST_DIR/empty"
+extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")
 
 echo
 echo "== creating image with default preallocation =="
 _make_test_img $size | _filter_imgfmt
-stat -c "size=%s, blocks=%b" $TEST_IMG
+stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
 
 for mode in off full falloc; do
     echo
     echo "== creating image with preallocation $mode =="
     IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt
-    stat -c "size=%s, blocks=%b" $TEST_IMG
+    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks $size
 done
 
 # success, all done
diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
index 76c02c6a57..6d9a5ed84e 100644
--- a/tests/qemu-iotests/175.out
+++ b/tests/qemu-iotests/175.out
@@ -2,17 +2,17 @@ QA output created by 175
 
 == creating image with default preallocation ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
-size=1048576, blocks=0
+size=1048576, nothing allocated
 
 == creating image with preallocation off ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off
-size=1048576, blocks=0
+size=1048576, nothing allocated
 
 == creating image with preallocation full ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full
-size=1048576, blocks=2048
+size=1048576, everything allocated
 
 == creating image with preallocation falloc ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=falloc
-size=1048576, blocks=2048
+size=1048576, everything allocated
  *** done
-- 
2.21.0
Re: [Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information
Posted by Eric Blake 4 years, 11 months ago 
On 5/13/19 10:52 AM, Max Reitz wrote:
> It is possible for an empty file to take up blocks on a filesystem.
> Make iotest 175 take this into account.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> v2: [Nir]
> - Use a function for filtering
> - s/empty_blocks/extra_blocks/
> ---

> +# Some file systems sometimes allocate extra blocks independently of
> +# the file size.  This function hides the resulting difference in the
> +# stat -c '%b' output.
> +# Parameter 1: Number of blocks an empty file occupies
> +# Parameter 2: Image size in bytes
> +_filter_blocks()
> +{
> +    extra_blocks=$1
> +    img_size=$2
> +
> +    sed -e "s/blocks=$extra_blocks/nothing allocated/" \
> +        -e "s/blocks=$((extra_blocks + img_size / 512))/everything allocated/"
> +}

That turned out nicely.

Reviewed-by: Eric Blake <eblake@redhat.com>

> -size=1m
> +size=$((1 * 1024 * 1024))

Had to change since bash doesn't understand '1m' inside $(()).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information
Posted by Nir Soffer 4 years, 11 months ago 
On Mon, May 13, 2019, 18:52 Max Reitz <mreitz@redhat.com> wrote:

> It is possible for an empty file to take up blocks on a filesystem.
> Make iotest 175 take this into account.
>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> v2: [Nir]
> - Use a function for filtering
> - s/empty_blocks/extra_blocks/
> ---
>  tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
>  tests/qemu-iotests/175.out |  8 ++++----
>  2 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
> index d0ffc495c2..b5eb0aa856 100755
> --- a/tests/qemu-iotests/175
> +++ b/tests/qemu-iotests/175
> @@ -28,10 +28,25 @@ status=1    # failure is the default!
>
>  _cleanup()
>  {
> -       _cleanup_test_img
> +    _cleanup_test_img
> +    rm -f "$TEST_DIR/empty"
>  }
>  trap "_cleanup; exit \$status" 0 1 2 3 15
>
> +# Some file systems sometimes allocate extra blocks independently of
> +# the file size.  This function hides the resulting difference in the
> +# stat -c '%b' output.
> +# Parameter 1: Number of blocks an empty file occupies
> +# Parameter 2: Image size in bytes
> +_filter_blocks()
> +{
> +    extra_blocks=$1
> +    img_size=$2
> +
> +    sed -e "s/blocks=$extra_blocks/nothing allocated/" \
> +        -e "s/blocks=$((extra_blocks + img_size / 512))/everything
> allocated/"
> +}
> +
>  # get standard environment, filters and checks
>  . ./common.rc
>  . ./common.filter
> @@ -40,18 +55,21 @@ _supported_fmt raw
>  _supported_proto file
>  _supported_os Linux
>
> -size=1m
> +size=$((1 * 1024 * 1024))
> +
> +touch "$TEST_DIR/empty"
> +extra_blocks=$(stat -c '%b' "$TEST_DIR/empty")
>
>  echo
>  echo "== creating image with default preallocation =="
>  _make_test_img $size | _filter_imgfmt
> -stat -c "size=%s, blocks=%b" $TEST_IMG
> +stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks
> $size
>
>  for mode in off full falloc; do
>      echo
>      echo "== creating image with preallocation $mode =="
>      IMGOPTS=preallocation=$mode _make_test_img $size | _filter_imgfmt
> -    stat -c "size=%s, blocks=%b" $TEST_IMG
> +    stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks $extra_blocks
> $size
>  done
>
>  # success, all done
> diff --git a/tests/qemu-iotests/175.out b/tests/qemu-iotests/175.out
> index 76c02c6a57..6d9a5ed84e 100644
> --- a/tests/qemu-iotests/175.out
> +++ b/tests/qemu-iotests/175.out
> @@ -2,17 +2,17 @@ QA output created by 175
>
>  == creating image with default preallocation ==
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
> -size=1048576, blocks=0
> +size=1048576, nothing allocated
>
>  == creating image with preallocation off ==
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off
> -size=1048576, blocks=0
> +size=1048576, nothing allocated
>
>  == creating image with preallocation full ==
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full
> -size=1048576, blocks=2048
> +size=1048576, everything allocated
>
>  == creating image with preallocation falloc ==
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
> preallocation=falloc
> -size=1048576, blocks=2048
> +size=1048576, everything allocated
>   *** done
> --
> 2.21.0


Reviewed-by: Nir Soffer <nsoffer@redhat.com>

>
Re: [Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information
Posted by Max Reitz 4 years, 11 months ago 
On 13.05.19 17:52, Max Reitz wrote:
> It is possible for an empty file to take up blocks on a filesystem.
> Make iotest 175 take this into account.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> v2: [Nir]
> - Use a function for filtering
> - s/empty_blocks/extra_blocks/
> ---
>  tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
>  tests/qemu-iotests/175.out |  8 ++++----
>  2 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
> index d0ffc495c2..b5eb0aa856 100755
> --- a/tests/qemu-iotests/175
> +++ b/tests/qemu-iotests/175
> @@ -28,10 +28,25 @@ status=1	# failure is the default!
>  
>  _cleanup()
>  {
> -	_cleanup_test_img
> +    _cleanup_test_img
> +    rm -f "$TEST_DIR/empty"
>  }
>  trap "_cleanup; exit \$status" 0 1 2 3 15
>  
> +# Some file systems sometimes allocate extra blocks independently of
> +# the file size.  This function hides the resulting difference in the
> +# stat -c '%b' output.
> +# Parameter 1: Number of blocks an empty file occupies
> +# Parameter 2: Image size in bytes
> +_filter_blocks()
> +{
> +    extra_blocks=$1
> +    img_size=$2
> +
> +    sed -e "s/blocks=$extra_blocks/nothing allocated/" \
> +        -e "s/blocks=$((extra_blocks + img_size / 512))/everything allocated/"

Only now got around to creating an FS with similar characteristics to
Thomas’s.  Turns out this test still fails there because it prints
“blocks=nothing allocated050”...

Unless objections arise, I’ll just add a \$ to each pattern to fix that.
 (I could also just swap the lines, but that isn’t really right.)

Max
Re: [Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information
Posted by Nir Soffer 4 years, 11 months ago 
On Thu, May 16, 2019 at 2:03 PM Max Reitz <mreitz@redhat.com> wrote:

> On 13.05.19 17:52, Max Reitz wrote:
> > It is possible for an empty file to take up blocks on a filesystem.
> > Make iotest 175 take this into account.
> >
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Max Reitz <mreitz@redhat.com>
> > ---
> > v2: [Nir]
> > - Use a function for filtering
> > - s/empty_blocks/extra_blocks/
> > ---
> >  tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
> >  tests/qemu-iotests/175.out |  8 ++++----
> >  2 files changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
> > index d0ffc495c2..b5eb0aa856 100755
> > --- a/tests/qemu-iotests/175
> > +++ b/tests/qemu-iotests/175
> > @@ -28,10 +28,25 @@ status=1  # failure is the default!
> >
> >  _cleanup()
> >  {
> > -     _cleanup_test_img
> > +    _cleanup_test_img
> > +    rm -f "$TEST_DIR/empty"
> >  }
> >  trap "_cleanup; exit \$status" 0 1 2 3 15
> >
> > +# Some file systems sometimes allocate extra blocks independently of
> > +# the file size.  This function hides the resulting difference in the
> > +# stat -c '%b' output.
> > +# Parameter 1: Number of blocks an empty file occupies
> > +# Parameter 2: Image size in bytes
> > +_filter_blocks()
> > +{
> > +    extra_blocks=$1
> > +    img_size=$2
> > +
> > +    sed -e "s/blocks=$extra_blocks/nothing allocated/" \
> > +        -e "s/blocks=$((extra_blocks + img_size / 512))/everything
> allocated/"
>
> Only now got around to creating an FS with similar characteristics to
> Thomas’s.  Turns out this test still fails there because it prints
> “blocks=nothing allocated050”...
>
> Unless objections arise, I’ll just add a \$ to each pattern to fix that.
>  (I could also just swap the lines, but that isn’t really right.)
>

Sounds right.

Can you share how to create file system which reproduces this?
Maybe adding the details in the test would be useful.

Nir
Re: [Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information
Posted by Max Reitz 4 years, 11 months ago 
On 16.05.19 15:06, Nir Soffer wrote:
> On Thu, May 16, 2019 at 2:03 PM Max Reitz <mreitz@redhat.com
> <mailto:mreitz@redhat.com>> wrote:
> 
>     On 13.05.19 17:52, Max Reitz wrote:
>     > It is possible for an empty file to take up blocks on a filesystem.
>     > Make iotest 175 take this into account.
>     >
>     > Reported-by: Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>>
>     > Signed-off-by: Max Reitz <mreitz@redhat.com
>     <mailto:mreitz@redhat.com>>
>     > ---
>     > v2: [Nir]
>     > - Use a function for filtering
>     > - s/empty_blocks/extra_blocks/
>     > ---
>     >  tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
>     >  tests/qemu-iotests/175.out |  8 ++++----
>     >  2 files changed, 26 insertions(+), 8 deletions(-)
>     >
>     > diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
>     > index d0ffc495c2..b5eb0aa856 100755
>     > --- a/tests/qemu-iotests/175
>     > +++ b/tests/qemu-iotests/175
>     > @@ -28,10 +28,25 @@ status=1  # failure is the default!
>     > 
>     >  _cleanup()
>     >  {
>     > -     _cleanup_test_img
>     > +    _cleanup_test_img
>     > +    rm -f "$TEST_DIR/empty"
>     >  }
>     >  trap "_cleanup; exit \$status" 0 1 2 3 15
>     > 
>     > +# Some file systems sometimes allocate extra blocks independently of
>     > +# the file size.  This function hides the resulting difference in the
>     > +# stat -c '%b' output.
>     > +# Parameter 1: Number of blocks an empty file occupies
>     > +# Parameter 2: Image size in bytes
>     > +_filter_blocks()
>     > +{
>     > +    extra_blocks=$1
>     > +    img_size=$2
>     > +
>     > +    sed -e "s/blocks=$extra_blocks/nothing allocated/" \
>     > +        -e "s/blocks=$((extra_blocks + img_size /
>     512))/everything allocated/"
> 
>     Only now got around to creating an FS with similar characteristics to
>     Thomas’s.  Turns out this test still fails there because it prints
>     “blocks=nothing allocated050”...
> 
>     Unless objections arise, I’ll just add a \$ to each pattern to fix that.
>      (I could also just swap the lines, but that isn’t really right.)
> 
> 
> Sounds right.
> 
> Can you share how to create file system which reproduces this?
> Maybe adding the details in the test would be useful.

Sure:

$ mkfs.ext4 -b 1024 -I 128 /dev/loop0

-I 128 is the important one.  The block size just changes the number of
extra blocks (2 with 1024, 8 with 4096).

...I guess I’ll just a v3 now.

Max
Re: [Qemu-devel] [PATCH v2] iotests: Filter 175's allocation information
Posted by Max Reitz 4 years, 11 months ago 
On 13.05.19 17:52, Max Reitz wrote:
> It is possible for an empty file to take up blocks on a filesystem.
> Make iotest 175 take this into account.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> v2: [Nir]
> - Use a function for filtering
> - s/empty_blocks/extra_blocks/
> ---
>  tests/qemu-iotests/175     | 26 ++++++++++++++++++++++----
>  tests/qemu-iotests/175.out |  8 ++++----
>  2 files changed, 26 insertions(+), 8 deletions(-)

Thanks for the reviews, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.