Re: [PATCH] tests/qtest/boot-sector: Check that the guest did not panic

[Thomas Huth <thuth@redhat.com>]

On 12/02/2021 14.18, Philippe Mathieu-Daudé wrote:
> On 2/12/21 12:31 PM, Thomas Huth wrote:
>> The s390-ccw bios code panics if it can not boot successfully. In
>> this case, it does not make sense that we wait the full 600 seconds
>> for the boot sector test to finish and can signal the failure
>> immediately, thus let's check the status of the guest with the
>> "query-status" QMP command here, too.
>>
>> Reported-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/qtest/boot-sector.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/tests/qtest/boot-sector.c b/tests/qtest/boot-sector.c
>> index 24df5c4734..ea8f264661 100644
>> --- a/tests/qtest/boot-sector.c
>> +++ b/tests/qtest/boot-sector.c
>> @@ -138,6 +138,7 @@ void boot_sector_test(QTestState *qts)
>>       uint8_t signature_low;
>>       uint8_t signature_high;
>>       uint16_t signature;
>> +    QDict *qrsp, *qret;
>>       int i;
>>   
>>       /* Wait at most 600 seconds (test is slow with TCI and --enable-debug) */
>> @@ -155,6 +156,14 @@ void boot_sector_test(QTestState *qts)
>>           if (signature == SIGNATURE) {
>>               break;
>>           }
>> +
>> +        /* check that guest is still in "running" state and did not panic */
>> +        qrsp = qtest_qmp(qts, "{ 'execute': 'query-status' }");
>> +        qret = qdict_get_qdict(qrsp, "return");
>> +        g_assert_nonnull(qret);
>> +        g_assert_cmpstr(qdict_get_try_str(qret, "status"), ==, "running");
> 
> Interesting idea. Does it make sense to have a similar (optional?) check
> done in QEMUMachine? This could benefit integration tests, quicker exit
> on failure.

Well, it only makes sense in cases where the guest is causing a panic event. 
That's what the s390 ccw bios is doing, but other firmwares do *not* panic 
in case they cannot boot the guest.

It might be also useful for the acceptance tests if they can trigger a panic 
event, but I think we already check for the "Kernel panic" in the console 
output in most cases, so I guess that's enough already?

  Thomas