[Qemu-devel] [PATCH] tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group

Thomas Huth posted 1 patch 6 years, 8 months ago
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu failed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1551442829-32359-1-git-send-email-thuth@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
tests/qemu-iotests/235   | 2 +-
tests/qemu-iotests/group | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH] tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group
Posted by Thomas Huth 6 years, 8 months ago
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(-)

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


Re: [Qemu-devel] [Qemu-block] [PATCH] tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group
Posted by John Snow 6 years, 7 months ago

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 ... ?

Re: [Qemu-devel] [Qemu-block] [PATCH] tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group
Posted by Thomas Huth 6 years, 7 months ago
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

Re: [Qemu-devel] [Qemu-block] [PATCH] tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group
Posted by John Snow 6 years, 7 months ago

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

Re: [Qemu-devel] [Qemu-block] [PATCH] tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group
Posted by Eric Blake 6 years, 7 months ago
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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] tests/qemu-iotests/235: Allow fallback to tcg and remove it from quick group
Posted by Vladimir Sementsov-Ogievskiy 6 years, 8 months ago
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
> 


-- 
Best regards,
Vladimir