On 7/13/20 5:56 AM, Philippe Mathieu-Daudé wrote:
> On 7/10/20 7:06 AM, John Snow wrote:
>> cubieboard does not have a functioning reboot, it halts and QEMU does
>> not exit.
>>
>> vm.shutdown() is modified in a forthcoming patch that makes it less tolerant
>> of race conditions on shutdown; tests should consciously decide to WAIT
>> or to SHUTDOWN qemu.
>>
>> So long as this test is attempting to reboot, the correct choice would
>> be to WAIT for the VM to exit. However, since that's broken, we should
>> SHUTDOWN instead.
>>
>> SHUTDOWN is indeed what already happens when the test performs teardown,
>> however, if anyone fixes cubieboard reboot in the future, this test will
>> develop a new race condition that might be hard to debug.
>>
>> Therefore: remove the reboot test and make it obvious that the VM is
>> still running when the test concludes, where the test teardown will do
>> the right thing.
>>
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>> tests/acceptance/boot_linux_console.py | 8 ++------
>> 1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/acceptance/boot_linux_console.py
>> b/tests/acceptance/boot_linux_console.py
>> index 5867ef760c..8b8b828bc5 100644
>> --- a/tests/acceptance/boot_linux_console.py
>> +++ b/tests/acceptance/boot_linux_console.py
>> @@ -508,9 +508,7 @@ def test_arm_cubieboard_initrd(self):
>> 'Allwinner sun4i/sun5i')
>> exec_command_and_wait_for_pattern(self, 'cat /proc/iomem',
>> 'system-control@1c00000')
>> - exec_command_and_wait_for_pattern(self, 'reboot',
>> - 'reboot: Restarting system')
>> - # NB: Do not issue vm.wait() here, cubieboard's reboot does not
>> exit!
>> + # cubieboard's reboot is not functioning; omit reboot test.
>>
>> def test_arm_cubieboard_sata(self):
>> """
>> @@ -553,9 +551,7 @@ def test_arm_cubieboard_sata(self):
>> 'Allwinner sun4i/sun5i')
>> exec_command_and_wait_for_pattern(self, 'cat /proc/partitions',
>> 'sda')
>> - exec_command_and_wait_for_pattern(self, 'reboot',
>> - 'reboot: Restarting system')
>> - # NB: Do not issue vm.wait() here, cubieboard's reboot does not
>> exit!
>> + # cubieboard's reboot is not functioning; omit reboot test.
>>
>> def test_arm_orangepi(self):
>> """
>>
>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>
> Note, if I do the pull request, I might reorder this one before the
> previous one "tests/acceptance: wait() instead of shutdown() where
> appropriate".
>
you could -- in practice it didn't seem to matter. I tested both with
and without this patch.
I was just trying to isolate each intentional semantic change as its own
commit so it could be observed/understood/debated.
--js
