Re: [PATCH] tests/boot-serial-test: Allow the HPPA machine to shudown

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 1/24/20 8:02 AM, Thomas Huth wrote:
> On 23/01/2020 22.37, Philippe Mathieu-Daudé wrote:
>> On 1/23/20 7:29 PM, Philippe Mathieu-Daudé wrote:
>>> On 1/23/20 5:39 AM, Thomas Huth wrote:
>>>> On 23/01/2020 01.36, Philippe Mathieu-Daudé wrote:
>>>>> The boot-serial test uses SeaBIOS on HPPA, and expects to read the
>>>>> "SeaBIOS wants SYSTEM HALT" string, see [*]:
>>>>>
>>>>>    122 void __VISIBLE __noreturn hlt(void)
>>>>>    123 {
>>>>>    124     if (pdc_debug)
>>>>>    125         printf("HALT initiated from %p\n",
>>>>> __builtin_return_address(0));
>>>>>    126     printf("SeaBIOS wants SYSTEM HALT.\n\n");
>>>>>    127     asm volatile("\t.word 0xfffdead0": : :"memory");
>>>>>    128     while (1);
>>>>>    129 }
>>>>>
>>>>> A 'SYSTEM HALT' would really halts the CPU, but SeaBIOS implements
>>>>> it as an infinite loop.
>>>>>
>>>>> If SeaBIOS does not use the expected serial port but another device,
>>>>> we might poll the console indefinitely while the machine is halted.
>>>>>
>>>>> Allow the HPPA machine to 'shutdown'. When it does, we'll get
>>>>> a qtest error:
>>>>>
>>>>>     $ make check-qtest-hppa
>>>>>       TEST    check-qtest-hppa: tests/qtest/boot-serial-test
>>>>>     ** (tests/qtest/boot-serial-test:31924): ERROR **: 01:12:37.604:
>>>>> Failed to find expected string. Please check
>>>>> '/tmp/qtest-boot-serial-sjxoM6Q'
>>>>>     ERROR - Bail out! FATAL-ERROR: Failed to find expected string.
>>>>> Please check '/tmp/qtest-boot-serial-sjxoM6Q'
>>>>>     make: *** [tests/Makefile.include:628: check-qtest-hppa] Error 1
>>>>
>>>> The tests are run with -no-shutdown. Why does qemu exit in that case?
>>>
>>> Because the HPPA firmware called HALT.
>>>
>>>> Sounds like a bug in another place, and not in the boot-serial-test.
>>>
>>> Yes, the bug is elsewhere, but with the bug the boot-serial-test hangs
>>> forever.... No output on the console, qtest waiting indefinitely.
>>
>> Richard explained me on IRC what you probably meant, which was not
>> obvious to me at first.
> 
> I think I also did not really understand what you tried to do here ;-)
> 
> The -no-shutdown is required, too, otherwise you could get a race
> between the test reading the serial output and the termination of QEMU,
> see commit 7150d34a1d60851d73d6ab6783b12b1d25e68f86, and I think we need
> that for HPPA, too.
> 
>> Now I found in check_guest_output():
>>
>>      /* Poll serial output... */
>>      while (1) {
>>          ...
>>          /* Wait at most 360 seconds.  */
>>          now = time(NULL);
>>          if (now - start >= 360) {
>>              break;
>>          }
>>          g_usleep(10000);
>>      }
>>
>> $ QTEST_QEMU_BINARY=hppa-softmmu/qemu-system-hppa \
>>    time tests/qtest/boot-serial-test -k
>> /hppa/boot-serial/hppa:
>> ** (tests/qtest/boot-serial-test:18604): ERROR **: 22:33:25.010: Failed
>> to find expected string. Please check '/tmp/qtest-boot-serial-sZq7ljM'
>> Command terminated by signal 5
>> 0.31user 0.66system 6:00.07elapsed 0%CPU
>>
>> Indeed the test fails after 6min, I guess I didn't expect that much
>> while testing interactively.
> 
> Yeah, the huge timeout is ugly, but it is required for very slow hosts,
> see commit 627fce617868df87b3757375a2a0318ad2beb381.
> 
> So if you want to "fix" your problem, I think you could maybe add a
> check for the QEMU events here instead. Or add a check to see whether
> the registers of the guest change between iterations to make sure that
> the guest is still alive (that way you also handle the case that the
> guest crashed and loops forever in a "branch self" instruction). Or
> simply continue with the 360s timeout, it's long, but it should trigger
> only in case of other bugs, so it's maybe not worth the effort to add
> more logic here.

Good idea. With QMP it is easy:

{ "execute": "query-status" }
{
     "return": {
         "status": "shutdown",
         "singlestep": false,
         "running": false
     }
}