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

[Thomas Huth <thuth@redhat.com>]

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.

>       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')

?

> +        exec_command_and_wait_for_pattern(self,
> +                '/bin/cat /sys/devices/system/cpu/dispatching', '0')
> +
>       def test_single(self):
>           """
>           This test checks the simplest topology with a single CPU.
> @@ -194,3 +204,39 @@ def test_hotplug_full(self):
>           self.check_topology(3, 1, 1, 1, 'high', False)
>           self.check_topology(4, 1, 1, 1, 'medium', False)
>           self.check_topology(5, 2, 1, 1, 'high', True)
> +
> +
> +    def guest_set_dispatching(self, dispatching):
> +        exec_command(self,
> +                f'echo {dispatching} > /sys/devices/system/cpu/dispatching')
> +        self.vm.event_wait('CPU_POLARIZATION_CHANGE', self.event_timeout)
> +        exec_command_and_wait_for_pattern(self,
> +                '/bin/cat /sys/devices/system/cpu/dispatching', dispatching)
> +
> +
> +    def test_polarisation(self):
> +        """
> +        This test verifies that QEMU modifies the entitlement change after
> +        several guest polarization change requests.
> +
> +        :avocado: tags=arch:s390x
> +        :avocado: tags=machine:s390-ccw-virtio
> +        """
> +        self.kernel_init()
> +        self.vm.launch()
> +        self.wait_until_booted()
> +
> +        self.system_init()
> +        res = self.vm.qmp('query-cpu-polarization')
> +        self.assertEqual(res['return']['polarization'], 'horizontal')
> +        self.check_topology(0, 0, 0, 0, 'medium', False)
> +
> +        self.guest_set_dispatching('1');
> +        res = self.vm.qmp('query-cpu-polarization')
> +        self.assertEqual(res['return']['polarization'], 'vertical')
> +        self.check_topology(0, 0, 0, 0, 'medium', False)
> +
> +        self.guest_set_dispatching('0');
> +        res = self.vm.qmp('query-cpu-polarization')
> +        self.assertEqual(res['return']['polarization'], 'horizontal')
> +        self.check_topology(0, 0, 0, 0, 'medium', False)