Re: [PATCH v22 14/20] tests/avocado: s390x cpu topology core

[Nina Schoetterl-Glausch]

On Thu, 2023-09-07 at 10:27 +0200, Thomas Huth wrote:
> On 01/09/2023 17.58, Nina Schoetterl-Glausch wrote:
> > From: Pierre Morel <pmorel@linux.ibm.com>
> > 
> > Introduction of the s390x cpu topology core functions and
> > basic tests.
> > 
> > We test the correlation between the command line and
> > the QMP results in query-cpus-fast for various CPU topology.
> > 
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > ---
> >   MAINTAINERS                    |   1 +
> >   tests/avocado/s390_topology.py | 200 +++++++++++++++++++++++++++++++++
> >   2 files changed, 201 insertions(+)
> >   create mode 100644 tests/avocado/s390_topology.py
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 7f2937b0f2..d1b8676666 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1703,6 +1703,7 @@ F: hw/s390x/cpu-topology.c
> >   F: target/s390x/kvm/stsi-topology.c
> >   F: docs/devel/s390-cpu-topology.rst
> >   F: docs/system/s390x/cpu-topology.rst
> > +F: tests/avocado/s390_topology.py
> >   
> >   X86 Machines
> >   ------------
> > diff --git a/tests/avocado/s390_topology.py b/tests/avocado/s390_topology.py
> > new file mode 100644
> > index 0000000000..3a54fe2e00
> > --- /dev/null
> > +++ b/tests/avocado/s390_topology.py
> > @@ -0,0 +1,200 @@

[...]

> > +    def test_default(self):
> > +        """
> > +        This test checks the implicit topology.
> > +
> > +        :avocado: tags=arch:s390x
> > +        :avocado: tags=machine:s390-ccw-virtio
> > +        """
> > +        self.kernel_init()
> > +        self.vm.add_args('-smp',
> > +                         
> > '13,drawers=2,books=2,sockets=3,cores=2,maxcpus=24')
> > +        self.vm.launch()
> > +        self.wait_until_booted()
> > +        self.check_topology(0, 0, 0, 0, 'medium', False)
> > +        self.check_topology(1, 0, 0, 0, 'medium', False)
> > +        self.check_topology(2, 1, 0, 0, 'medium', False)
> > +        self.check_topology(3, 1, 0, 0, 'medium', False)
> > +        self.check_topology(4, 2, 0, 0, 'medium', False)
> > +        self.check_topology(5, 2, 0, 0, 'medium', False)
> > +        self.check_topology(6, 0, 1, 0, 'medium', False)
> > +        self.check_topology(7, 0, 1, 0, 'medium', False)
> > +        self.check_topology(8, 1, 1, 0, 'medium', False)
> > +        self.check_topology(9, 1, 1, 0, 'medium', False)
> > +        self.check_topology(10, 2, 1, 0, 'medium', False)
> > +        self.check_topology(11, 2, 1, 0, 'medium', False)
> > +        self.check_topology(12, 0, 0, 1, 'medium', False)
> > +
> > +    def test_move(self):
> > +        """
> > +        This test checks the topology modification by moving a CPU
> > +        to another socket: CPU 0 is moved from socket 0 to socket 2.
> > +
> > +        :avocado: tags=arch:s390x
> > +        :avocado: tags=machine:s390-ccw-virtio
> > +        """
> > +        self.kernel_init()
> > +        self.vm.add_args('-smp',
> > +                         
> > '1,drawers=2,books=2,sockets=3,cores=2,maxcpus=24')
> > +        self.vm.launch()
> > +        self.wait_until_booted()
> > +
> > +        self.check_topology(0, 0, 0, 0, 'medium', False)
> > +        res = self.vm.qmp('set-cpu-topology',
> > +                          {'core-id': 0, 'socket-id': 2, 'entitlement': 
> > 'low'})
> > +        self.assertEqual(res['return'], {})
> > +        self.check_topology(0, 2, 0, 0, 'low', False)
> 
> Booting a Linux kernel in the guest always takes some precious seconds of 
> testing time ... I wonder whether it maybe makes sense to merge the above 
> two tests into one?

IMO it's cleaner to keep them separated.

> > +    def test_hotplug_full(self):
> > +        """
> > +        This test verifies that a hotplugged defined with '-device'
> > +        command line argument finds its right place inside the topology.
> 
> "hotplug" is normally rather a "device_add" via HMP/QMP during run-time, so 
> I'd rather avoid this term here. Maybe call it "test_device_option" or so?

I'll go with test_dash_device.

> 
> > +        :avocado: tags=arch:s390x
> > +        :avocado: tags=machine:s390-ccw-virtio
> > +        """
> > +        self.kernel_init()
> > +        self.vm.add_args('-smp',
> > +                         
> > '1,drawers=2,books=2,sockets=3,cores=2,maxcpus=24')
> > +        self.vm.add_args('-device', 'max-s390x-cpu,core-id=10')
> > +        self.vm.add_args('-device',
> > +                         'max-s390x-cpu,'
> > +                         
> > 'core-id=1,socket-id=0,book-id=1,drawer-id=1,entitlement=low')
> > +        self.vm.add_args('-device',
> > +                         'max-s390x-cpu,'
> > +                         
> > 'core-id=2,socket-id=0,book-id=1,drawer-id=1,entitlement=medium')
> > +        self.vm.add_args('-device',
> > +                         'max-s390x-cpu,'
> > +                         
> > 'core-id=3,socket-id=1,book-id=1,drawer-id=1,entitlement=high')
> > +        self.vm.add_args('-device',
> > +                         'max-s390x-cpu,'
> > +                         'core-id=4,socket-id=1,book-id=1,drawer-id=1')
> > +        self.vm.add_args('-device',
> > +                         'max-s390x-cpu,'
> > +                         
> > 'core-id=5,socket-id=2,book-id=1,drawer-id=1,dedicated=true')
> > +
> > +        self.vm.launch()
> > +        self.wait_until_booted()
> > +
> > +        self.check_topology(10, 2, 1, 0, 'medium', False)
> > +        self.check_topology(1, 0, 1, 1, 'low', False)
> > +        self.check_topology(2, 0, 1, 1, 'medium', False)
> > +        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)
> 
>   Thomas
>