On 6/8/21 10:22 AM, Philippe Mathieu-Daudé wrote:
> On 6/7/21 3:22 PM, Thomas Huth wrote:
>> On 04/06/2021 17.51, Alex Bennée wrote:
>>> From: Philippe Mathieu-Daudé <phi...@redhat.com>
>>>
>>> Use the recently added generic qtest_has_accel() method to
>>> check if KVM is available.
>>>
>>> Suggested-by: Claudio Fontana <cfont...@suse.de>
>>> Reviewed-by: Andrew Jones <drjo...@redhat.com>
>>> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
>>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>>> Message-Id: <20210505125806.1263441-5-phi...@redhat.com>
>>> ---
>>> tests/qtest/arm-cpu-features.c | 25 +------------------------
>>> 1 file changed, 1 insertion(+), 24 deletions(-)
>>>
>>> diff --git a/tests/qtest/arm-cpu-features.c
>>> b/tests/qtest/arm-cpu-features.c
>>> index 8252b85bb8..7f4b252127 100644
>>> --- a/tests/qtest/arm-cpu-features.c
>>> +++ b/tests/qtest/arm-cpu-features.c
>>> @@ -26,21 +26,6 @@
>>> " 'arguments': { 'type': 'full', "
>>> #define QUERY_TAIL "}}"
>>> -static bool kvm_enabled(QTestState *qts)
>>> -{
>>> - QDict *resp, *qdict;
>>> - bool enabled;
>>> -
>>> - resp = qtest_qmp(qts, "{ 'execute': 'query-kvm' }");
>>> - g_assert(qdict_haskey(resp, "return"));
>>> - qdict = qdict_get_qdict(resp, "return");
>>> - g_assert(qdict_haskey(qdict, "enabled"));
>>> - enabled = qdict_get_bool(qdict, "enabled");
>>> - qobject_unref(resp);
>>> -
>>> - return enabled;
>>> -}
>>> -
>>> static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
>>> {
>>> return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
>>> @@ -493,14 +478,6 @@ static void
>>> test_query_cpu_model_expansion_kvm(const void *data)
>>> qts = qtest_init(MACHINE_KVM "-cpu max");
>>> - /*
>>> - * These tests target the 'host' CPU type, so KVM must be enabled.
>>> - */
>>> - if (!kvm_enabled(qts)) {
>>> - qtest_quit(qts);
>>> - return;
>>> - }
>>> -
>>> /* Enabling and disabling kvm-no-adjvtime should always work. */
>>> assert_has_feature_disabled(qts, "host", "kvm-no-adjvtime");
>>> assert_set_feature(qts, "host", "kvm-no-adjvtime", true);
>>> @@ -624,7 +601,7 @@ int main(int argc, char **argv)
>>> * order avoid attempting to run an AArch32 QEMU with KVM on
>>> * AArch64 hosts. That won't work and isn't easy to detect.
>>> */
>>> - if (g_str_equal(qtest_get_arch(), "aarch64")) {
>>> + if (g_str_equal(qtest_get_arch(), "aarch64") &&
>>> qtest_has_accel("kvm")) {
>>> qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
>>> NULL, test_query_cpu_model_expansion_kvm);
>>
>> I think this is wrong: query-kvm checks whether kvm is *enabled*, while
>> your new function only checks whether kvm has been built into the
>> binary. There is still the possibility that kvm has been built into the
>> binary, but is not available on the host, so in that case the test will
>> fail now.
Not enough coffee earlier. I think this is a documentation problem,
query-kvm returns a list of *runtime* accelerators:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg811144.html
IIUC what Paolo said, if something asks for an accelerator that
is not present at build-time, then this is a configuration problem,
not relevant for the management interface.
>>
>> Thus please drop / rework this patch.
>
> Indeed, this is unfortunate :(
>
