* [Qemu-devel] [PATCH] iotests: Filter 175's allocation information
@ 2019-05-10 21:19 Max Reitz
2019-05-10 21:45 ` Nir Soffer
0 siblings, 1 reply; 5+ messages in thread
From: Max Reitz @ 2019-05-10 21:19 UTC (permalink / raw)
To: qemu-block; +Cc: Kevin Wolf, Nir Soffer, Thomas Huth, qemu-devel, Max Reitz
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>
---
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")
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/"
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] iotests: Filter 175's allocation information
2019-05-10 21:19 [Qemu-devel] [PATCH] iotests: Filter 175's allocation information Max Reitz
@ 2019-05-10 21:45 ` Nir Soffer
2019-05-13 13:20 ` Max Reitz
0 siblings, 1 reply; 5+ messages in thread
From: Nir Soffer @ 2019-05-10 21:45 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, Thomas Huth, QEMU Developers, qemu-block
On Sat, May 11, 2019 at 12:19 AM 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>
> ---
> 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?
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"?
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.
> 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
>
Otherwise looks good.
Nir
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] iotests: Filter 175's allocation information
2019-05-10 21:45 ` Nir Soffer
@ 2019-05-13 13:20 ` Max Reitz
2019-05-13 15:33 ` John Snow
0 siblings, 1 reply; 5+ messages in thread
From: Max Reitz @ 2019-05-13 13:20 UTC (permalink / raw)
To: Nir Soffer; +Cc: Kevin Wolf, Thomas Huth, QEMU Developers, qemu-block
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] iotests: Filter 175's allocation information
2019-05-13 13:20 ` Max Reitz
@ 2019-05-13 15:33 ` John Snow
2019-05-13 15:34 ` Max Reitz
0 siblings, 1 reply; 5+ messages in thread
From: John Snow @ 2019-05-13 15:33 UTC (permalink / raw)
To: Max Reitz, Nir Soffer
Cc: Kevin Wolf, Thomas Huth, QEMU Developers, qemu-block
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 <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
>
You can always parameterize the filter so that the relationship is
explicit, no? Does that still feel icky?
--js
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] iotests: Filter 175's allocation information
2019-05-13 15:33 ` John Snow
@ 2019-05-13 15:34 ` Max Reitz
0 siblings, 0 replies; 5+ messages in thread
From: Max Reitz @ 2019-05-13 15:34 UTC (permalink / raw)
To: John Snow, Nir Soffer
Cc: Kevin Wolf, Thomas Huth, QEMU Developers, qemu-block
[-- Attachment #1: Type: text/plain, Size: 3470 bytes --]
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 <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
>>
>
> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-13 15:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-10 21:19 [Qemu-devel] [PATCH] iotests: Filter 175's allocation information Max Reitz
2019-05-10 21:45 ` Nir Soffer
2019-05-13 13:20 ` Max Reitz
2019-05-13 15:33 ` John Snow
2019-05-13 15:34 ` Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).