| Message ID | 1551442829-32359-1-git-send-email-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group | expand |
01.03.2019 15:20, Thomas Huth wrote: > iotest 235 currently only works with KVM - this is bad for systems where > it is not available, e.g. CI pipelines. The test also works when using > "tcg" as accelerator, so we can simply add that to the list of accelerators, > too. But still, there might be the case that someone compiled their > QEMU with --disable-tcg and still try to run the iotests in a CI pipeline > where KVM is not available - in that case it would be best to use the > "qtest" accelerator for this test. However, that currently hangs and I > did not succeed to get it working with "accel=qtest" yet. hmm, interesting, I can reproduce it. I think we'd better fix it instead.. I'll try but not now. Thus, as long > as this is not fixed, it's likely better to remove this test from the > "quick" group so that it does not fail on CI pipelines. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/qemu-iotests/235 | 2 +- > tests/qemu-iotests/group | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 > index d6edd97..90ef785 100755 > --- a/tests/qemu-iotests/235 > +++ b/tests/qemu-iotests/235 > @@ -49,7 +49,7 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, > str(size)) > > vm = QEMUMachine(iotests.qemu_prog) > -vm.add_args('-machine', 'accel=kvm') > +vm.add_args('-machine', 'accel=kvm:tcg') I tested now with accel=tcg, and it doesn't reproduce original bug.. On, the other hand, if kvm is not available anyway, why not run test for tcg, may be it'll find some other bug. > if iotests.qemu_default_machine == 's390-ccw-virtio': > vm.add_args('-no-shutdown') > vm.add_args('-drive', 'id=src,file=' + disk) > diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group > index b5ca63c..12ebeba 100644 > --- a/tests/qemu-iotests/group > +++ b/tests/qemu-iotests/group > @@ -234,7 +234,7 @@ > 232 auto quick > 233 auto quick > 234 auto quick migration > -235 auto quick > +235 auto > 236 auto quick > 237 rw auto quick > 238 auto quick >
On 3/1/19 7:20 AM, Thomas Huth wrote: > iotest 235 currently only works with KVM - this is bad for systems where > it is not available, e.g. CI pipelines. The test also works when using > "tcg" as accelerator, so we can simply add that to the list of accelerators, > too. But still, there might be the case that someone compiled their > QEMU with --disable-tcg and still try to run the iotests in a CI pipeline > where KVM is not available - in that case it would be best to use the > "qtest" accelerator for this test. However, that currently hangs and I > did not succeed to get it working with "accel=qtest" yet. Thus, as long > as this is not fixed, it's likely better to remove this test from the > "quick" group so that it does not fail on CI pipelines. > > Signed-off-by: Thomas Huth <thuth@redhat.com> What happens if we set accel=kvm:tcg but leave it in quick group? It will still fail in those cases where we have neither accelerator for whichever reason, but maybe that's better than just trying to prevent it from running ... ?
On 20/03/2019 18.32, John Snow wrote: > > > On 3/1/19 7:20 AM, Thomas Huth wrote: >> iotest 235 currently only works with KVM - this is bad for systems where >> it is not available, e.g. CI pipelines. The test also works when using >> "tcg" as accelerator, so we can simply add that to the list of accelerators, >> too. But still, there might be the case that someone compiled their >> QEMU with --disable-tcg and still try to run the iotests in a CI pipeline >> where KVM is not available - in that case it would be best to use the >> "qtest" accelerator for this test. However, that currently hangs and I >> did not succeed to get it working with "accel=qtest" yet. Thus, as long >> as this is not fixed, it's likely better to remove this test from the >> "quick" group so that it does not fail on CI pipelines. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> > > What happens if we set accel=kvm:tcg but leave it in quick group? It > will still fail in those cases where we have neither accelerator for > whichever reason, but maybe that's better than just trying to prevent it > from running ... ? ... but since it is failing, you can't use "make check-block" in the CI pipelines in that case. I had to manually select the test cases in the .gitlab-ci.yml file due to this. I don't mind too much, but IMHO "make check-block" should simply run everywhere, with every QEMU binary, and not only if you've compiled it with the right options and/or KVM is available. Otherwise, we maybe need a "make check-block-simple" or "make check-block-ci" target, that uses another new group beside the "quick" group? I.e. introduce a "ci" or "simple" group into tests/qemu-iotests/group ? Thomas
On 3/21/19 4:23 AM, Thomas Huth wrote: > On 20/03/2019 18.32, John Snow wrote: >> >> >> On 3/1/19 7:20 AM, Thomas Huth wrote: >>> iotest 235 currently only works with KVM - this is bad for systems where >>> it is not available, e.g. CI pipelines. The test also works when using >>> "tcg" as accelerator, so we can simply add that to the list of accelerators, >>> too. But still, there might be the case that someone compiled their >>> QEMU with --disable-tcg and still try to run the iotests in a CI pipeline >>> where KVM is not available - in that case it would be best to use the >>> "qtest" accelerator for this test. However, that currently hangs and I >>> did not succeed to get it working with "accel=qtest" yet. Thus, as long >>> as this is not fixed, it's likely better to remove this test from the >>> "quick" group so that it does not fail on CI pipelines. >>> >>> Signed-off-by: Thomas Huth <thuth@redhat.com> >> >> What happens if we set accel=kvm:tcg but leave it in quick group? It >> will still fail in those cases where we have neither accelerator for >> whichever reason, but maybe that's better than just trying to prevent it >> from running ... ? > > ... but since it is failing, you can't use "make check-block" in the CI > pipelines in that case. I had to manually select the test cases in the > .gitlab-ci.yml file due to this. I don't mind too much, but IMHO "make > check-block" should simply run everywhere, with every QEMU binary, and > not only if you've compiled it with the right options and/or KVM is > available. Otherwise, we maybe need a "make check-block-simple" or "make > check-block-ci" target, that uses another new group beside the "quick" > group? I.e. introduce a "ci" or "simple" group into > tests/qemu-iotests/group ? > It still feels like papering over the problem to me; if the test is failing on qtest that feels like an accurate test result we shouldn't hide or ignore...? I realize that's inconvenient, but is it common for us to have neither TCG nor KVM? (I genuinely don't know. I'd have thought that's fairly rare? Clearly not for you.) If the problem is that the test suite stops on first failure, should we amend the execution to continue on in those cases? --js (I guess I should look at why the test doesn't work under qtest, but I have some downstream work to finish before I could prioritize that.)
On 3/21/19 3:23 AM, Thomas Huth wrote: > pipelines in that case. I had to manually select the test cases in the > .gitlab-ci.yml file due to this. I don't mind too much, but IMHO "make > check-block" should simply run everywhere, with every QEMU binary, and > not only if you've compiled it with the right options and/or KVM is > available. Otherwise, we maybe need a "make check-block-simple" or "make > check-block-ci" target, that uses another new group beside the "quick" > group? I.e. introduce a "ci" or "simple" group into I don't know how to tell '-g quick' apart from '-g simple', but having a '-g ci' where we can add or remove tests from ci by editing qemu-iotests/group seems reasonable. Then switching 'make check-block' to prefer -g ci makes sense, while developers wanting a bit more coverage without long waits can still rely on -g quick.
diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 index d6edd97..90ef785 100755 --- a/tests/qemu-iotests/235 +++ b/tests/qemu-iotests/235 @@ -49,7 +49,7 @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, str(size)) vm = QEMUMachine(iotests.qemu_prog) -vm.add_args('-machine', 'accel=kvm') +vm.add_args('-machine', 'accel=kvm:tcg') if iotests.qemu_default_machine == 's390-ccw-virtio': vm.add_args('-no-shutdown') vm.add_args('-drive', 'id=src,file=' + disk) diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index b5ca63c..12ebeba 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -234,7 +234,7 @@ 232 auto quick 233 auto quick 234 auto quick migration -235 auto quick +235 auto 236 auto quick 237 rw auto quick 238 auto quick
iotest 235 currently only works with KVM - this is bad for systems where it is not available, e.g. CI pipelines. The test also works when using "tcg" as accelerator, so we can simply add that to the list of accelerators, too. But still, there might be the case that someone compiled their QEMU with --disable-tcg and still try to run the iotests in a CI pipeline where KVM is not available - in that case it would be best to use the "qtest" accelerator for this test. However, that currently hangs and I did not succeed to get it working with "accel=qtest" yet. Thus, as long as this is not fixed, it's likely better to remove this test from the "quick" group so that it does not fail on CI pipelines. Signed-off-by: Thomas Huth <thuth@redhat.com> --- tests/qemu-iotests/235 | 2 +- tests/qemu-iotests/group | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)