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

[Max Reitz]

On 13.05.19 17:33, John Snow wrote:
> 
> 
> 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?

Hmmm. :-)  Sounds good, thanks.

I was thinking of just a comment.  “Needs variable $foo set.”  But a
parameter works nicely, yes.

Max

signature.asc
Description: OpenPGP digital signature