| Message ID | 20200918181951.21752-12-vsementsov@virtuozzo.com |
|---|---|
| State | Accepted |
| Commit | d2ace2b95ff35f71a532bc1f5c4cb60971feb4a8 |
| Headers | show |
| Series | preallocate filter | expand |
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 > > diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298 > new file mode 100644 > index 0000000000..fef10f6a7a > --- /dev/null > +++ b/tests/qemu-iotests/298 [...] > +class TestPreallocateBase(iotests.QMPTestCase): Perhaps a @iotests.skip_if_unsupported(['preallocate']) here? > + def setUp(self): > + iotests.qemu_img_create('-f', iotests.imgfmt, disk, str(10 * MiB)) [...] > +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. > + def setUp(self): > + iotests.qemu_img_create('-f', iotests.imgfmt, disk, str(10 * MiB)) > + iotests.qemu_img_create('-f', iotests.imgfmt, refdisk, str(10 * MiB)) > + > + def tearDown(self): > + os.remove(disk) > + os.remove(refdisk) > + > + def do_test(self, prealloc_mode, new_size): > + ret = iotests.qemu_io_silent('--image-opts', '-c', 'write 0 10M', '-c', > + f'truncate -m {prealloc_mode} {new_size}', > + drive_opts) > + self.assertEqual(ret, 0) > + > + ret = iotests.qemu_io_silent('-f', iotests.imgfmt, '-c', 'write 0 10M', > + '-c', > + f'truncate -m {prealloc_mode} {new_size}', > + refdisk) > + self.assertEqual(ret, 0) > + > + stat = os.stat(disk) > + refstat = os.stat(refdisk) > + > + # 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? > + 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. Max
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 >> >> diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298 >> new file mode 100644 >> index 0000000000..fef10f6a7a >> --- /dev/null >> +++ b/tests/qemu-iotests/298 > > [...] > >> +class TestPreallocateBase(iotests.QMPTestCase): > > Perhaps a > > @iotests.skip_if_unsupported(['preallocate']) > > here? > >> + def setUp(self): >> + iotests.qemu_img_create('-f', iotests.imgfmt, disk, str(10 * MiB)) > > [...] > >> +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 > >> + def setUp(self): >> + iotests.qemu_img_create('-f', iotests.imgfmt, disk, str(10 * MiB)) >> + iotests.qemu_img_create('-f', iotests.imgfmt, refdisk, str(10 * MiB)) >> + >> + def tearDown(self): >> + os.remove(disk) >> + os.remove(refdisk) >> + >> + def do_test(self, prealloc_mode, new_size): >> + ret = iotests.qemu_io_silent('--image-opts', '-c', 'write 0 10M', '-c', >> + f'truncate -m {prealloc_mode} {new_size}', >> + drive_opts) >> + self.assertEqual(ret, 0) >> + >> + ret = iotests.qemu_io_silent('-f', iotests.imgfmt, '-c', 'write 0 10M', >> + '-c', >> + f'truncate -m {prealloc_mode} {new_size}', >> + refdisk) >> + self.assertEqual(ret, 0) >> + >> + stat = os.stat(disk) >> + refstat = os.stat(refdisk) >> + >> + # 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 > >> + 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.. If I don't put new test in "auto", is there any chance that test would be automatically run somewhere? -- Best regards, Vladimir
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
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
25.09.2020 18:11, Vladimir Sementsov-Ogievskiy wrote: > 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.. Sorry, I see now that it sounds rude. >> >> 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 [..] >> -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. >> OK, so it doesn't work on BSD at all. I don't really want to investigate, but seems it's because of absence of fallocate. So let's drop "auto" group. Another thing: maybe, add auto-linux test group for running only in linux-build? So, new tests will be added to it because why not, and we will not bother with BSD and MacOS? -- Best regards, Vladimir
On 25.09.20 17:32, Vladimir Sementsov-Ogievskiy wrote: > 25.09.2020 18:11, Vladimir Sementsov-Ogievskiy wrote: >> 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.. > > Sorry, I see now that it sounds rude. I found it completely understandable, because I share the same sentiment. It’s the reason I’m so hesitant adding tests to auto. >>> 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. It kind of is simple, but it still takes so long that I don’t consider it simple. >> What another things you are >> running before sending a PULL? Actually, I’m not running any of the vm-build-* targets. (If I did, I wouldn’t have to ask you whether you did, because I’d just run them anyway and then tell you about any potential failures.) I compile everything on Fedora and Arch (x86-64), -m32 and -m64, clang and gcc, and one time with mingw. I run make check on everything but mingw, and then run the following iotests on Fedora gcc-m64 and Arch clang-m32 (on tmpfs): -qcow2 -x auto, -qcow2 -o compat=0.10, -qcow2 -o refcount_bits=1, -qcow2 -o data_file='$TEST_IMG.ext_data_file', -nbd, -raw, -luks, -vmdk, -vhdx, -qcow, -vdi, -vpc, -qed, -cloop, -parallels, -bochs And on non-tmpfs: -c none -qcow2 142 199 (At some point that meant that all iotests that don’t require root are at least run once. I should check whether that’s still the case, I suppose...) >>> 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 > > [..] > >>> -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. >>> > > OK, so it doesn't work on BSD at all. I don't really want to > investigate, but seems it's because of absence of fallocate. So let's > drop "auto" group. I’d be OK with that. > Another thing: maybe, add auto-linux test group for running only in > linux-build? So, new tests will be added to it because why not, and we > will not bother with BSD and MacOS? We have _supported_os in bash tests and supported_platforms in Python tests, so if you want a test in auto but run it only on Linux, you can specify that there. Max
On 25/09/2020 17.11, Vladimir Sementsov-Ogievskiy wrote: > 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. Running the tests on macOS is also quite simple if you have a github account. You simply add the "Cirrus-CI" from the marketplace to your forked qemu repository there, and then push your work to a branch in that repo. Cirrus-CI then automatically tests your stuff on macOS (and also FreeBSD), e.g.: https://cirrus-ci.com/build/4961684689256448 Thomas
diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298 new file mode 100644 index 0000000000..fef10f6a7a --- /dev/null +++ b/tests/qemu-iotests/298 @@ -0,0 +1,186 @@ +#!/usr/bin/env python3 +# +# Test for preallocate filter +# +# Copyright (c) 2020 Virtuozzo International GmbH. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +import os +import iotests + +MiB = 1024 * 1024 +disk = os.path.join(iotests.test_dir, 'disk') +overlay = os.path.join(iotests.test_dir, 'overlay') +refdisk = os.path.join(iotests.test_dir, 'refdisk') +drive_opts = f'node-name=disk,driver={iotests.imgfmt},' \ + f'file.node-name=filter,file.driver=preallocate,' \ + f'file.file.node-name=file,file.file.filename={disk}' + + +class TestPreallocateBase(iotests.QMPTestCase): + def setUp(self): + iotests.qemu_img_create('-f', iotests.imgfmt, disk, str(10 * MiB)) + + def tearDown(self): + try: + self.check_small() + check = iotests.qemu_img_check(disk) + self.assertFalse('leaks' in check) + self.assertFalse('corruptions' in check) + self.assertEqual(check['check-errors'], 0) + finally: + os.remove(disk) + + def check_big(self): + self.assertTrue(os.path.getsize(disk) > 100 * MiB) + + def check_small(self): + self.assertTrue(os.path.getsize(disk) < 10 * MiB) + + +class TestQemuImg(TestPreallocateBase): + def test_qemu_img(self): + p = iotests.QemuIoInteractive('--image-opts', drive_opts) + + p.cmd('write 0 1M') + p.cmd('flush') + + self.check_big() + + p.close() + + +class TestPreallocateFilter(TestPreallocateBase): + def setUp(self): + super().setUp() + self.vm = iotests.VM().add_drive(path=None, opts=drive_opts) + self.vm.launch() + + def tearDown(self): + self.vm.shutdown() + super().tearDown() + + def test_prealloc(self): + self.vm.hmp_qemu_io('drive0', 'write 0 1M') + self.check_big() + + def test_external_snapshot(self): + self.test_prealloc() + + result = self.vm.qmp('blockdev-snapshot-sync', node_name='disk', + snapshot_file=overlay, + snapshot_node_name='overlay') + self.assert_qmp(result, 'return', {}) + + # on reopen to r-o base preallocation should be dropped + self.check_small() + + self.vm.hmp_qemu_io('drive0', 'write 1M 1M') + + result = self.vm.qmp('block-commit', device='overlay') + self.assert_qmp(result, 'return', {}) + self.complete_and_wait() + + # commit of new megabyte should trigger preallocation + self.check_big() + + def test_reopen_opts(self): + result = self.vm.qmp('x-blockdev-reopen', **{ + 'node-name': 'disk', + 'driver': iotests.imgfmt, + 'file': { + 'node-name': 'filter', + 'driver': 'preallocate', + 'prealloc-size': 20 * MiB, + 'prealloc-align': 5 * MiB, + 'file': { + 'node-name': 'file', + 'driver': 'file', + 'filename': disk + } + } + }) + self.assert_qmp(result, 'return', {}) + + self.vm.hmp_qemu_io('drive0', 'write 0 1M') + self.assertTrue(os.path.getsize(disk) == 25 * MiB) + + +class TestTruncate(iotests.QMPTestCase): + def setUp(self): + iotests.qemu_img_create('-f', iotests.imgfmt, disk, str(10 * MiB)) + iotests.qemu_img_create('-f', iotests.imgfmt, refdisk, str(10 * MiB)) + + def tearDown(self): + os.remove(disk) + os.remove(refdisk) + + def do_test(self, prealloc_mode, new_size): + ret = iotests.qemu_io_silent('--image-opts', '-c', 'write 0 10M', '-c', + f'truncate -m {prealloc_mode} {new_size}', + drive_opts) + self.assertEqual(ret, 0) + + ret = iotests.qemu_io_silent('-f', iotests.imgfmt, '-c', 'write 0 10M', + '-c', + f'truncate -m {prealloc_mode} {new_size}', + refdisk) + self.assertEqual(ret, 0) + + stat = os.stat(disk) + refstat = os.stat(refdisk) + + # 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. + self.assertLess(abs(stat.st_blocks - refstat.st_blocks) * 512, + 1024 * 1024) + + def test_real_shrink(self): + self.do_test('off', '5M') + + def test_truncate_inside_preallocated_area__falloc(self): + self.do_test('falloc', '50M') + + def test_truncate_inside_preallocated_area__metadata(self): + self.do_test('metadata', '50M') + + def test_truncate_inside_preallocated_area__full(self): + self.do_test('full', '50M') + + def test_truncate_inside_preallocated_area__off(self): + self.do_test('off', '50M') + + def test_truncate_over_preallocated_area__falloc(self): + self.do_test('falloc', '150M') + + def test_truncate_over_preallocated_area__metadata(self): + self.do_test('metadata', '150M') + + def test_truncate_over_preallocated_area__full(self): + self.do_test('full', '150M') + + def test_truncate_over_preallocated_area__off(self): + self.do_test('off', '150M') + + +if __name__ == '__main__': + iotests.main(supported_fmts=['qcow2']) diff --git a/tests/qemu-iotests/298.out b/tests/qemu-iotests/298.out new file mode 100644 index 0000000000..fa16b5ccef --- /dev/null +++ b/tests/qemu-iotests/298.out @@ -0,0 +1,5 @@ +............. +---------------------------------------------------------------------- +Ran 13 tests + +OK 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 299 auto quick 300 migration 301 backing quick
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