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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2947 bytes --]

On 10.05.19 23:45, Nir Soffer wrote:
> On Sat, May 11, 2019 at 12:19 AM Max Reitz <mreitz@redhat.com
> <mailto: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 <mailto:thuth@redhat.com>>
>     Signed-off-by: Max Reitz <mreitz@redhat.com <mailto:mreitz@redhat.com>>
>     ---
>      tests/qemu-iotests/175     | 15 +++++++++++----
>      tests/qemu-iotests/175.out |  8 ++++----
>      2 files changed, 15 insertions(+), 8 deletions(-)
> 
>     diff --git a/tests/qemu-iotests/175 b/tests/qemu-iotests/175
>     index d0ffc495c2..b5652a3889 100755
>     --- a/tests/qemu-iotests/175
>     +++ b/tests/qemu-iotests/175
>     @@ -28,7 +28,8 @@ 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
> 
>     @@ -40,18 +41,24 @@ _supported_fmt raw
>      _supported_proto file
>      _supported_os Linux
> 
>     -size=1m
>     +size=$((1 * 1024 * 1024))
> 
>     +
>     +touch "$TEST_DIR/empty"
>     +empty_blocks=$(stat -c '%b' "$TEST_DIR/empty")
> 
> 
> Maybe extra_blocks?

Why not.

>      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 \
>     +    | sed -e "s/blocks=$empty_blocks/nothing allocated/"
> 
>      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 \
>     +        | sed -e "s/blocks=$empty_blocks/nothing allocated/" \
>     +        | sed -e "s/blocks=$((empty_blocks + size /
>     512))/everything allocated/"
> 
> 
> "fully allocated"?

I didn’t like that because that sounds like it only applies to
preallocation=full.

> Maybe add a helper like this:
> 
> _filter_blocks() {
>         # Some file systems sometimes allocate extra blocks
>         sed -e "s/blocks=$empty_blocks/nothing allocated/" \
>                -e "s/blocks=$((empty_blocks + size / 512))/everything
> allocated/"
> }
> 
> So we can do:
> 
>     stat -c "size=%s, blocks=%b" $TEST_IMG | _filter_blocks
> 
> And it is also clear why we need to run sed without looking up the
> commit message.

Makes sense to me, but I find it a bit awkward to make a filter rely on
a data value determined outside of the filter...  I’ll see what I can do
to calm my conscience.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]