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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7071 bytes --]

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)
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


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