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

[John Snow]

On 5/13/19 9:20 AM, Max Reitz wrote:
> On 10.05.19 23:45, Nir Soffer wrote:
>> On Sat, May 11, 2019 at 12:19 AM Max Reitz <address@hidden
>> <mailto:address@hidden>> 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 <address@hidden <mailto:address@hidden>>
>>     Signed-off-by: Max Reitz <address@hidden <mailto:address@hidden>>
>>     ---
>>      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
> 

You can always parameterize the filter so that the relationship is
explicit, no? Does that still feel icky?

--js