Thomas Huth <[email protected]> writes:
> On 17/03/2026 08.35, Markus Armbruster wrote:
>> Thomas Huth <[email protected]> writes:
>>
>>> On 12/03/2026 09.15, Markus Armbruster wrote:
>>>> Markus Armbruster <[email protected]> writes:
>>> ...
>>>>> These crashes escape tests/qtest/device-introspect-test, because it
>>>>> covers only HMP "device_add T,help", not CLI "-device T,help".
>>>>
>>>> Anyone wants to cook up a patch to cover -device?
>>>
>>> Quick'n'dirty patch:
>>>
>>> diff --git a/tests/qtest/device-introspect-test.c
>>> b/tests/qtest/device-introspect-test.c
>>> --- a/tests/qtest/device-introspect-test.c
>>> +++ b/tests/qtest/device-introspect-test.c
>>> @@ -105,6 +105,7 @@ static void test_one_device(QTestState *qts, const char
>>> *type)
>>> QDict *resp;
>>> char *help, *escaped;
>>> GRegex *comma;
>>> + int ret;
>>>
>>> g_test_message("Testing device '%s'", type);
>>>
>>> @@ -119,6 +120,17 @@ static void test_one_device(QTestState *qts, const
>>> char *type)
>>>
>>> help = qtest_hmp(qts, "device_add \"%s,help\"", escaped);
>>> g_free(help);
>>> +
>>> + if (!g_str_equal(type, "nonexistent") && !g_str_equal(type, "device"))
>>> {
>>> + help = g_strdup_printf("%s -device \"%s,help\" > /dev/null",
>>> qtest_qemu_binary(NULL), escaped);
>>> + ret = system(help);
>>
>> Not using qtest_init() or similar. Hmm.
>
> I mentioned it's quick-n-dirty ;-)
Fair, fair :)
>>> + g_free(help);
>>> + if (ret) {
>>> + fprintf(stderr, "Running qemu with -device %s,help failed!\n",
>>> escaped);
>>> + abort();
>>> + }
>>> + }
>>> +
>>> g_free(escaped);
>>> }
>>>
>>> But I think it needs some more work - we likely don't want to run this for
>>> each test run since it slows down the test quite a bit. And the
>>> g_test_quick() switch is already used for something else here.
>>
>> It's used to test with every machine type, not just with "none".
>>
>> I think we could also use it to additionally test -device just with
>> "none". Testing it with every machine type would be even slower, and I
>> doubt it's worthwhile.
>
> Agreed.
>
>> Another idea to conserve cycles: use a unit test instead.
>
> Unit tests don't have a way yet to determine which qemu-system-xyz binaries
> are available, do they? ... so that would need some more logic in the
> meson.build file there - not sure whether that's worth the effort, I think
> the device-introspect qtest is likely a better place?
A unit test isn't supposed to run qemu-system-xyz. Instead, it mocks up
the environment and just runs "the unit". In this case,
qdev_device_help().
This is less protection than a qtest, but it can also be a lot faster.
> Or maybe add it to scripts/device-crash-test ?
That's another option.
[...]