Re: [PATCH v21 15/20] tests/avocado: s390x cpu topology polarisation

[Pierre Morel <pmorel@linux.ibm.com>]

On 7/5/23 10:53, Thomas Huth wrote:
> On 30/06/2023 11.17, Pierre Morel wrote:
>> Polarization is changed on a request from the guest.
>> Let's verify the polarization is accordingly set by QEMU.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   tests/avocado/s390_topology.py | 46 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/tests/avocado/s390_topology.py 
>> b/tests/avocado/s390_topology.py
>> index 1758ec1f13..2cf731cb1d 100644
>> --- a/tests/avocado/s390_topology.py
>> +++ b/tests/avocado/s390_topology.py
>> @@ -40,6 +40,7 @@ class S390CPUTopology(QemuSystemTest):
>>       The polarization is changed on a request from the guest.
>>       """
>>       timeout = 90
>> +    event_timeout = 1
>
> When running tests in CI and the machines are very loaded, the tests 
> can be stalled easily by multiple seconds. So using a timeout of 1 
> seconds sounds way too low for me. Please use at least 5 seconds, or 
> maybe even 10.


OK


>
>>       KERNEL_COMMON_COMMAND_LINE = ('printk.time=0 '
>>                                     'root=/dev/ram '
>> @@ -99,6 +100,15 @@ def kernel_init(self):
>>                            '-initrd', initrd_path,
>>                            '-append', kernel_command_line)
>>   +    def system_init(self):
>> +        self.log.info("System init")
>> +        exec_command(self, 'mount proc -t proc /proc')
>> +        time.sleep(0.2)
>> +        exec_command(self, 'mount sys -t sysfs /sys')
>> +        time.sleep(0.2)
>
> Hard coded sleeps are ugly... they are prone to race conditions (e.g. 
> on loaded test systems), and they artificially slow down the test 
> duration.
>
> What about doing all three commands in one statement instead:
>
>     exec_command_and_wait_for_pattern(self,
>            """mount proc -t proc /proc ;
>               mount sys -t sysfs /sys ;
>               /bin/cat /sys/devices/system/cpu/dispatching""",
>            '0')
>
> ?
>
OK , I use this. thx.

Regards,

Pierre