Claudio Fontana <cfont...@suse.de> writes:
> On 4/19/21 12:29 PM, Thomas Huth wrote:
>> On 19/04/2021 12.24, Claudio Fontana wrote:
>>> On 4/19/21 12:22 PM, Thomas Huth wrote:
>>>> On 16/04/2021 18.27, Claudio Fontana wrote:
>>>>> Skip the test_device_intro_concrete for now for ARM KVM-only build,
>>>>> as on ARM we currently build devices for ARM that are not
>>>>> compatible with a KVM-only build.
>>>>>
>>>>> We can remove this workaround when we fix this in KConfig etc,
>>>>> and we only list and build machines that are compatible with KVM
>>>>> for KVM-only builds.
>>>>>
>>>>> Signed-off-by: Claudio Fontana <cfont...@suse.de>
>>>>> Cc: Philippe Mathieu-Daudé <f4...@amsat.org>
>>>>> ---
>>>>> tests/qtest/device-introspect-test.c | 18 ++++++++++++++++++
>>>>> 1 file changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/tests/qtest/device-introspect-test.c
>>>>> b/tests/qtest/device-introspect-test.c
>>>>> index bbec166dbc..1ff15e2247 100644
>>>>> --- a/tests/qtest/device-introspect-test.c
>>>>> +++ b/tests/qtest/device-introspect-test.c
>>>>> @@ -329,12 +329,30 @@ int main(int argc, char **argv)
>>>>> qtest_add_func("device/introspect/none", test_device_intro_none);
>>>>> qtest_add_func("device/introspect/abstract",
>>>>> test_device_intro_abstract);
>>>>> qtest_add_func("device/introspect/abstract-interfaces",
>>>>> test_abstract_interfaces);
>>>>> +
>>>>> + /*
>>>>> + * XXX currently we build also boards for ARM that are incompatible
>>>>> with KVM.
>>>>> + * We therefore need to check this explicitly, and only test virt
>>>>> for kvm-only
>>>>> + * arm builds.
>>>>> + * After we do the work of Kconfig etc to ensure that only
>>>>> KVM-compatible boards
>>>>> + * are built for the kvm-only build, we could remove this.
>>>>> + */
>>>>> +#ifndef CONFIG_TCG
>>>>> + {
>>>>> + const char *arch = qtest_get_arch();
>>>>> + if (strcmp(arch, "arm") == 0 || strcmp(arch, "aarch64") == 0) {
>>>>> + goto add_machine_test_done;
>>>>> + }
>>>>> + }
>>>>> +#endif /* !CONFIG_TCG */
>>>>> if (g_test_quick()) {
>>>>> qtest_add_data_func("device/introspect/concrete/defaults/none",
>>>>> g_strdup(common_args),
>>>>> test_device_intro_concrete);
>>>>> } else {
>>>>> qtest_cb_for_every_machine(add_machine_test_case, true);
>>>>> }
>>>>> + goto add_machine_test_done;
>>>>
>>>> That last goto does not make much sense, since the label is right below.
>>>
>>> It is necessary to remove the warning that is otherwise produced about the
>>> unused label IIRC.
>>
>> Ah, ok.
>>
>> Alternatively, you could maybe also drop the label completely and simply
>> write the #ifndef block above like this (note the "else"):
>>
>> #ifndef CONFIG_TCG
>> const char *arch = qtest_get_arch();
>> if (!strcmp(arch, "arm") || !strcmp(arch, "aarch64")) {
>> /* Do nothing */
>> }
>> else
>> #endif /* !CONFIG_TCG */
>>
>> ... not sure what's nicer, though.
>>
>> Thomas
>>
>
> Indeed, I tried a couple of combinations, in the end to me the less confusing
> was the goto one,
> the #ifdef containing an open else is in my opinion worse, more
> error-prone, but I am open to additional comments/ideas.
Surely a helper makes intent clearer?
/*
* XXX currently we build also boards for ARM that are incompatible with KVM.
* We therefore need to check this explicitly, and only test virt for kvm-only
* arm builds.
* After we do the work of Kconfig etc to ensure that only KVM-compatible
boards
* are built for the kvm-only build, we could remove this.
*/
static bool skip_machine_tests(void)
{
#ifndef CONFIG_TCG
const char *arch = qtest_get_arch();
if (strcmp(arch, "arm") == 0 || strcmp(arch, "aarch64") == 0) {
return true;
}
#endif /* !CONFIG_TCG */
return false;
}
int main(int argc, char **argv)
{
g_test_init(&argc, &argv, NULL);
qtest_add_func("device/introspect/list", test_device_intro_list);
qtest_add_func("device/introspect/list-fields", test_qom_list_fields);
qtest_add_func("device/introspect/none", test_device_intro_none);
qtest_add_func("device/introspect/abstract", test_device_intro_abstract);
qtest_add_func("device/introspect/abstract-interfaces",
test_abstract_interfaces);
if (!skip_machine_tests()) {
if (g_test_quick()) {
qtest_add_data_func("device/introspect/concrete/defaults/none",
g_strdup(common_args),
test_device_intro_concrete);
} else {
qtest_cb_for_every_machine(add_machine_test_case, true);
}
}
return g_test_run();
}
>
> Thanks,
>
> Claudio
--
Alex Bennée
