Re: [PATCH v6 11/15] iotests: add 298 to test new preallocate filter driver

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

25.09.2020 12:11, Max Reitz wrote:
> On 25.09.20 10:49, Vladimir Sementsov-Ogievskiy wrote:
>> 25.09.2020 11:26, Max Reitz wrote:
>>> On 18.09.20 20:19, Vladimir Sementsov-Ogievskiy wrote:
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>> ---
>>>>    tests/qemu-iotests/298     | 186 +++++++++++++++++++++++++++++++++++++
>>>>    tests/qemu-iotests/298.out |   5 +
>>>>    tests/qemu-iotests/group   |   1 +
>>>>    3 files changed, 192 insertions(+)
>>>>    create mode 100644 tests/qemu-iotests/298
>>>>    create mode 100644 tests/qemu-iotests/298.out
> 
> [...]
> 
>>>> +class TestTruncate(iotests.QMPTestCase):
>>>
>>> The same decorator could be placed here, although this class doesn’t
>>> start a VM, and so is unaffected by the allowlist.  Still may be
>>> relevant in case of block modules, I don’t know.
>>
>> Or just global test skip at file top
> 
> Hm.  Like verify_quorum()?  Is there a generic function for that already?
> 
> [...]
> 
>>>> +        # Probably we'll want preallocate filter to keep align to
>>>> cluster when
>>>> +        # shrink preallocation, so, ignore small differece
>>>> +        self.assertLess(abs(stat.st_size - refstat.st_size), 64 * 1024)
>>>> +
>>>> +        # Preallocate filter may leak some internal clusters (for
>>>> example, if
>>>> +        # guest write far over EOF, skipping some clusters - they
>>>> will remain
>>>> +        # fallocated, preallocate filter don't care about such
>>>> leaks, it drops
>>>> +        # only trailing preallocation.
>>>
>>> True, but that isn’t what’s happening here.  (We only write 10M at 0, so
>>> there are no gaps.)  Why do we need this 1M margin?
>>
>> We write 10M, but qcow2 also writes metadata as it wants
> 
> Ah, yes, sure.  Shouldn’t result in 1M, but why not.
> 
>>>> +        self.assertLess(abs(stat.st_blocks - refstat.st_blocks) * 512,
>>>> +                        1024 * 1024)
>>>
>>> [...]
>>>
>>>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>>>> index ff59cfd2d4..15d5f9619b 100644
>>>> --- a/tests/qemu-iotests/group
>>>> +++ b/tests/qemu-iotests/group
>>>> @@ -307,6 +307,7 @@
>>>>    295 rw
>>>>    296 rw
>>>>    297 meta
>>>> +298 auto quick
>>>
>>> I wouldn’t mark it as quick, there is at least one preallocate=full of
>>> 140M, and one of 40M, plus multiple 10M data writes and falloc
>>> preallocations.
>>>
>>> Also, since you mark it as “auto”, have you run this test on all
>>> CI-relevant hosts?  (Among other things I can’t predict) I wonder how
>>> preallocation behaves on macOS.  Just because that one was always a bit
>>> weird about not-really-data areas.
>>>
>>
>> Ofcourse, I didn't run on all hosts. I'm a bit out of sync about this..
> 
> Well, someone has to do it.  The background story is that tests are
> added to auto all the time (because “why not”), and then they fail on
> BSD or macOS.  We have BSD docker test build targets at least, so they
> can be easily tested.  (Well, it takes like half an hour, but you know.)
> 
> (We don’t have macOS builds, as far as I can tell, but I personally
> don’t even know why we run the iotests on macOS at all.  (Well, I also
> wonder about the BSDs, but given the test build targets, I shouldn’t
> complain, I suppose.))
> 
> (Though macOS isn’t part of the gating CI, is it?  I seem to remember
> macOS errors are generally only reported to me half a week after the
> pull request is merged, which is even worse.)
> 
> Anyway.  I just ran the test for OpenBSD
> (EXTRA_CONFIGURE_OPTS='--target-list=x86_64-softmmu' \
>     make vm-build-openbsd)

Oh, I didn't know that it's so simple. What another things you are running before sending a PULL?

> and got some failures:
> 
> --- /home/qemu/qemu-test.PGo2ls/src/tests/qemu-iotests/298.out  Fri Sep
> 25 07:10:31 2020
> +++ /home/qemu/qemu-test.PGo2ls/build/tests/qemu-iotests/298.out.bad
> Fri Sep 25 08:57:56 2020
> @@ -1,5 +1,67 @@
> -.............
> +qemu-io: Failed to resize underlying file: Unsupported preallocation
> mode: falloc
> +qemu-io: Failed to resize underlying file: Unsupported preallocation
> mode: falloc
> +FFFF.F...F...
> +======================================================================
> +FAIL: test_external_snapshot (__main__.TestPreallocateFilter)
>   ----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "298", line 81, in test_external_snapshot
> +    self.test_prealloc()
> +  File "298", line 78, in test_prealloc
> +    self.check_big()
> +  File "298", line 48, in check_big
> +    self.assertTrue(os.path.getsize(disk) > 100 * MiB)
> +AssertionError: False is not true
> +
> +======================================================================
> +FAIL: test_prealloc (__main__.TestPreallocateFilter)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "298", line 78, in test_prealloc
> +    self.check_big()
> +  File "298", line 48, in check_big
> +    self.assertTrue(os.path.getsize(disk) > 100 * MiB)
> +AssertionError: False is not true
> +
> +======================================================================
> +FAIL: test_reopen_opts (__main__.TestPreallocateFilter)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "298", line 119, in test_reopen_opts
> +    self.assertTrue(os.path.getsize(disk) == 25 * MiB)
> +AssertionError: False is not true
> +
> +======================================================================
> +FAIL: test_qemu_img (__main__.TestQemuImg)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "298", line 61, in test_qemu_img
> +    self.check_big()
> +  File "298", line 48, in check_big
> +    self.assertTrue(os.path.getsize(disk) > 100 * MiB)
> +AssertionError: False is not true
> +
> +======================================================================
> +FAIL: test_truncate_inside_preallocated_area__falloc
> (__main__.TestTruncate)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "298", line 161, in test_truncate_inside_preallocated_area__falloc
> +    self.do_test('falloc', '50M')
> +  File "298", line 135, in do_test
> +    self.assertEqual(ret, 0)
> +AssertionError: 1 != 0
> +
> +======================================================================
> +FAIL: test_truncate_over_preallocated_area__falloc (__main__.TestTruncate)
> +----------------------------------------------------------------------
> +Traceback (most recent call last):
> +  File "298", line 173, in test_truncate_over_preallocated_area__falloc
> +    self.do_test('falloc', '150M')
> +  File "298", line 135, in do_test
> +    self.assertEqual(ret, 0)
> +AssertionError: 1 != 0
> +
> +----------------------------------------------------------------------
>   Ran 13 tests
> 
> -OK
> +FAILED (failures=6)
> 
>> If I don't put new test in "auto", is there any chance that test would
>> be automatically run somewhere?
> 
> I run all tests before pull requests at least.
> 
> Max
> 


-- 
Best regards,
Vladimir