[RFC] tests/acceptance: add a test for devices on s390x

This adds a very basic test for checking that we present devices
in a way that Linux can consume: boot with both virtio-net-ccw and
virtio-net-pci attached and then verify that Linux is able to see
and detect these devices.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---

A very basic test, but it would have caught the recent zPCI regression.

If anyone has a better idea than using early debug shells in the Debian
install image, please let me know. At least it's quick, as we can check
for the devices quite early in the boot sequence.

Not sure if running under both kvm and tcg on an s390 host would add
useful extra coverage. Also not sure if this needs fencing on any of the
public CIs (have not tried yet).

---
 tests/acceptance/s390_devices.py | 68 ++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 tests/acceptance/s390_devices.py

On 25/11/2020 14.58, Cornelia Huck wrote:
> This adds a very basic test for checking that we present devices
> in a way that Linux can consume: boot with both virtio-net-ccw and
> virtio-net-pci attached and then verify that Linux is able to see
> and detect these devices.

Thanks for tackling it!

> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> A very basic test, but it would have caught the recent zPCI regression.
> 
> If anyone has a better idea than using early debug shells in the Debian
> install image, please let me know. At least it's quick, as we can check
> for the devices quite early in the boot sequence.
> 
> Not sure if running under both kvm and tcg on an s390 host would add
> useful extra coverage. Also not sure if this needs fencing on any of the
> public CIs (have not tried yet).

We're only running the acceptance tests in the gitlab-CI, no worries about
the others.

> ---
>  tests/acceptance/s390_devices.py | 68 ++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 tests/acceptance/s390_devices.py
> 
> diff --git a/tests/acceptance/s390_devices.py b/tests/acceptance/s390_devices.py
> new file mode 100644
> index 000000000000..6ce47061f35d
> --- /dev/null
> +++ b/tests/acceptance/s390_devices.py

s390x_devices.py ?

Or maybe even machine_s390x.py instead, like the other machine*.py files?

> @@ -0,0 +1,68 @@
> +# Functional test that boots an s390x Linux guest with ccw and PCI devices
> +# attached and checks whether the devices are recognized by Linux
> +#
> +# Copyright (c) 2020 Red Hat, Inc.
> +#
> +# Author:
> +#  Cornelia Huck <cohuck@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +
> +import os
> +
> +from avocado_qemu import Test
> +from avocado_qemu import exec_command_and_wait_for_pattern
> +from avocado_qemu import wait_for_console_pattern
> +
> +class CheckS390xDevices(Test):
> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
> +
> +    def wait_for_console_pattern(self, success_message, vm=None):
> +        wait_for_console_pattern(self, success_message,
> +                                 failure_message='Kernel panic - not syncing',
> +                                 vm=vm)
> +
> +    timeout = 60

Running on public CIs can be slow ... I'd maybe directly start with 90 or
120 here.

> +    def test(self):
> +
> +        """
> +        :avocado: tags=arch:s390x
> +        :avocado: tags=machine:s390-ccw-virtio
> +        """
> +
> +        # XXX: switch to https when debian fixes their certificate
> +        kernel_url = ('http://archive.debian.org/debian/dists/jessie/main'
> +                      '/installer-s390x/current/images/generic/kernel.debian')
> +        kernel_hash = '5af1aa839754f4d8817fb5878b4d55dfc887f45d'
> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> +
> +        initrd_url = ('http://archive.debian.org/debian/dists/jessie/main'
> +                      '/installer-s390x/current/images/generic/initrd.debian')
> +        initrd_hash = '99252b28306184b876f979585e2d4bfe96b27464'
> +        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> +
> +        self.vm.set_console()
> +        kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> +                              'console=sclp0 root=/dev/ram0 BOOT_DEBUG=3')
> +        self.vm.add_args('-nographic',
> +                         '-kernel', kernel_path,
> +                         '-initrd', initrd_path,
> +                         '-append', kernel_command_line,
> +                         '-device', 'virtio-net-ccw,devno=fe.1.1111',
> +                         '-device', 'virtio-net-pci')

Maybe use '-device', 'virtio-net-pci,addr=6' or something similar to check a
non-default PCI address, too?

> +        self.vm.launch()
> +
> +        shell_ready = "sh: can't access tty; job control turned off"
> +        self.wait_for_console_pattern(shell_ready)
> +        # first debug shell is too early, we need to wait for device detection
> +        exec_command_and_wait_for_pattern(self, 'exit', shell_ready)
> +
> +        ccw_bus_id="0.1.1111"
> +        pci_bus_id="0000:00:00.0"
> +        exec_command_and_wait_for_pattern(self, 'ls /sys/bus/ccw/devices/',
> +                                          ccw_bus_id)
> +        exec_command_and_wait_for_pattern(self, 'ls /sys/bus/pci/devices/',
> +                                          pci_bus_id)
> 

Additional ideas (likely for later patches): Set a custom mac address for
the virtio-net devices and check whether they show up correctly in the
guest... Use "ping" to send some packets around (with -netdev user)...

 Thomas

On Wed, 25 Nov 2020 16:03:13 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 25/11/2020 14.58, Cornelia Huck wrote:
> > This adds a very basic test for checking that we present devices
> > in a way that Linux can consume: boot with both virtio-net-ccw and
> > virtio-net-pci attached and then verify that Linux is able to see
> > and detect these devices.  
> 
> Thanks for tackling it!
> 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> > 
> > A very basic test, but it would have caught the recent zPCI regression.
> > 
> > If anyone has a better idea than using early debug shells in the Debian
> > install image, please let me know. At least it's quick, as we can check
> > for the devices quite early in the boot sequence.
> > 
> > Not sure if running under both kvm and tcg on an s390 host would add
> > useful extra coverage. Also not sure if this needs fencing on any of the
> > public CIs (have not tried yet).  
> 
> We're only running the acceptance tests in the gitlab-CI, no worries about
> the others.
> 
> > ---
> >  tests/acceptance/s390_devices.py | 68 ++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >  create mode 100644 tests/acceptance/s390_devices.py
> > 
> > diff --git a/tests/acceptance/s390_devices.py b/tests/acceptance/s390_devices.py
> > new file mode 100644
> > index 000000000000..6ce47061f35d
> > --- /dev/null
> > +++ b/tests/acceptance/s390_devices.py  
> 
> s390x_devices.py ?
> 
> Or maybe even machine_s390x.py instead, like the other machine*.py files?

Makes sense.

> 
> > @@ -0,0 +1,68 @@
> > +# Functional test that boots an s390x Linux guest with ccw and PCI devices
> > +# attached and checks whether the devices are recognized by Linux
> > +#
> > +# Copyright (c) 2020 Red Hat, Inc.
> > +#
> > +# Author:
> > +#  Cornelia Huck <cohuck@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2 or
> > +# later.  See the COPYING file in the top-level directory.
> > +
> > +
> > +import os
> > +
> > +from avocado_qemu import Test
> > +from avocado_qemu import exec_command_and_wait_for_pattern
> > +from avocado_qemu import wait_for_console_pattern
> > +
> > +class CheckS390xDevices(Test):
> > +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
> > +
> > +    def wait_for_console_pattern(self, success_message, vm=None):
> > +        wait_for_console_pattern(self, success_message,
> > +                                 failure_message='Kernel panic - not syncing',
> > +                                 vm=vm)
> > +
> > +    timeout = 60  
> 
> Running on public CIs can be slow ... I'd maybe directly start with 90 or
> 120 here.

Ok; I found it hard to pick a sensible value here.

> 
> > +    def test(self):
> > +
> > +        """
> > +        :avocado: tags=arch:s390x
> > +        :avocado: tags=machine:s390-ccw-virtio
> > +        """
> > +
> > +        # XXX: switch to https when debian fixes their certificate
> > +        kernel_url = ('http://archive.debian.org/debian/dists/jessie/main'
> > +                      '/installer-s390x/current/images/generic/kernel.debian')
> > +        kernel_hash = '5af1aa839754f4d8817fb5878b4d55dfc887f45d'
> > +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> > +
> > +        initrd_url = ('http://archive.debian.org/debian/dists/jessie/main'
> > +                      '/installer-s390x/current/images/generic/initrd.debian')
> > +        initrd_hash = '99252b28306184b876f979585e2d4bfe96b27464'
> > +        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> > +
> > +        self.vm.set_console()
> > +        kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> > +                              'console=sclp0 root=/dev/ram0 BOOT_DEBUG=3')
> > +        self.vm.add_args('-nographic',
> > +                         '-kernel', kernel_path,
> > +                         '-initrd', initrd_path,
> > +                         '-append', kernel_command_line,
> > +                         '-device', 'virtio-net-ccw,devno=fe.1.1111',
> > +                         '-device', 'virtio-net-pci')  
> 
> Maybe use '-device', 'virtio-net-pci,addr=6' or something similar to check a
> non-default PCI address, too?

Not sure if addr= will do the trick, I may need to add a zpci device.

> 
> > +        self.vm.launch()
> > +
> > +        shell_ready = "sh: can't access tty; job control turned off"
> > +        self.wait_for_console_pattern(shell_ready)
> > +        # first debug shell is too early, we need to wait for device detection
> > +        exec_command_and_wait_for_pattern(self, 'exit', shell_ready)
> > +
> > +        ccw_bus_id="0.1.1111"
> > +        pci_bus_id="0000:00:00.0"
> > +        exec_command_and_wait_for_pattern(self, 'ls /sys/bus/ccw/devices/',
> > +                                          ccw_bus_id)
> > +        exec_command_and_wait_for_pattern(self, 'ls /sys/bus/pci/devices/',
> > +                                          pci_bus_id)
> >   
> 
> Additional ideas (likely for later patches): Set a custom mac address for
> the virtio-net devices and check whether they show up correctly in the
> guest... Use "ping" to send some packets around (with -netdev user)...

This needs a fully running userspace, though. I was planning on adding
more device types first.

Hi Cornelia,

On 11/25/20 4:03 PM, Thomas Huth wrote:
> On 25/11/2020 14.58, Cornelia Huck wrote:
>> This adds a very basic test for checking that we present devices
>> in a way that Linux can consume: boot with both virtio-net-ccw and
>> virtio-net-pci attached and then verify that Linux is able to see
>> and detect these devices.
> 
> Thanks for tackling it!
> 
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>
>> A very basic test, but it would have caught the recent zPCI regression.

Thanks for adding this test :)

>>
>> If anyone has a better idea than using early debug shells in the Debian
>> install image, please let me know. At least it's quick, as we can check
>> for the devices quite early in the boot sequence.

This is the simplest cheaper way I think.

Alternative is to use Guenter's images:
https://github.com/groeck/linux-build-test/tree/master/rootfs/s390

>>
>> Not sure if running under both kvm and tcg on an s390 host would add
>> useful extra coverage. Also not sure if this needs fencing on any of the
>> public CIs (have not tried yet).
> 
> We're only running the acceptance tests in the gitlab-CI, no worries about
> the others.
> 
>> ---
>>  tests/acceptance/s390_devices.py | 68 ++++++++++++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>  create mode 100644 tests/acceptance/s390_devices.py
>>
>> diff --git a/tests/acceptance/s390_devices.py b/tests/acceptance/s390_devices.py
>> new file mode 100644
>> index 000000000000..6ce47061f35d
>> --- /dev/null
>> +++ b/tests/acceptance/s390_devices.py
> 
> s390x_devices.py ?
> 
> Or maybe even machine_s390x.py instead, like the other machine*.py files?

Feel free to use whatever name/directory structure that help others to
find your tests (don't forget to add an entry to MAINTAINERS).

Regards,

Phil.

On Wed, 25 Nov 2020 17:04:05 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Hi Cornelia,
> 
> On 11/25/20 4:03 PM, Thomas Huth wrote:
> > On 25/11/2020 14.58, Cornelia Huck wrote:  
> >> This adds a very basic test for checking that we present devices
> >> in a way that Linux can consume: boot with both virtio-net-ccw and
> >> virtio-net-pci attached and then verify that Linux is able to see
> >> and detect these devices.  
> > 
> > Thanks for tackling it!
> >   
> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> >> ---
> >>
> >> A very basic test, but it would have caught the recent zPCI regression.  
> 
> Thanks for adding this test :)
> 
> >>
> >> If anyone has a better idea than using early debug shells in the Debian
> >> install image, please let me know. At least it's quick, as we can check
> >> for the devices quite early in the boot sequence.  
> 
> This is the simplest cheaper way I think.
> 
> Alternative is to use Guenter's images:
> https://github.com/groeck/linux-build-test/tree/master/rootfs/s390

I tried to use these, but it seems I would need a kernel with the
relevant drivers built-in for that, and neither Fedora nor Debian seem
to do that. Maybe I'm holding it wrong, but I think I'll just stick to
my current approach, as I have that one working :)

> 
> >>
> >> Not sure if running under both kvm and tcg on an s390 host would add
> >> useful extra coverage. Also not sure if this needs fencing on any of the
> >> public CIs (have not tried yet).  
> > 
> > We're only running the acceptance tests in the gitlab-CI, no worries about
> > the others.
> >   
> >> ---
> >>  tests/acceptance/s390_devices.py | 68 ++++++++++++++++++++++++++++++++
> >>  1 file changed, 68 insertions(+)
> >>  create mode 100644 tests/acceptance/s390_devices.py
> >>
> >> diff --git a/tests/acceptance/s390_devices.py b/tests/acceptance/s390_devices.py
> >> new file mode 100644
> >> index 000000000000..6ce47061f35d
> >> --- /dev/null
> >> +++ b/tests/acceptance/s390_devices.py  
> > 
> > s390x_devices.py ?
> > 
> > Or maybe even machine_s390x.py instead, like the other machine*.py files?  
> 
> Feel free to use whatever name/directory structure that help others to
> find your tests (don't forget to add an entry to MAINTAINERS).

Good point, I forgot about an explicit MAINTAINERS entry.

> 
> Regards,
> 
> Phil.
>

On Wed, 25 Nov 2020 16:30:34 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 25 Nov 2020 16:03:13 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 25/11/2020 14.58, Cornelia Huck wrote:  

> > > +    def test(self):
> > > +
> > > +        """
> > > +        :avocado: tags=arch:s390x
> > > +        :avocado: tags=machine:s390-ccw-virtio
> > > +        """
> > > +
> > > +        # XXX: switch to https when debian fixes their certificate
> > > +        kernel_url = ('http://archive.debian.org/debian/dists/jessie/main'
> > > +                      '/installer-s390x/current/images/generic/kernel.debian')
> > > +        kernel_hash = '5af1aa839754f4d8817fb5878b4d55dfc887f45d'
> > > +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> > > +
> > > +        initrd_url = ('http://archive.debian.org/debian/dists/jessie/main'
> > > +                      '/installer-s390x/current/images/generic/initrd.debian')
> > > +        initrd_hash = '99252b28306184b876f979585e2d4bfe96b27464'
> > > +        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> > > +
> > > +        self.vm.set_console()
> > > +        kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> > > +                              'console=sclp0 root=/dev/ram0 BOOT_DEBUG=3')
> > > +        self.vm.add_args('-nographic',
> > > +                         '-kernel', kernel_path,
> > > +                         '-initrd', initrd_path,
> > > +                         '-append', kernel_command_line,
> > > +                         '-device', 'virtio-net-ccw,devno=fe.1.1111',
> > > +                         '-device', 'virtio-net-pci')    
> > 
> > Maybe use '-device', 'virtio-net-pci,addr=6' or something similar to check a
> > non-default PCI address, too?  
> 
> Not sure if addr= will do the trick, I may need to add a zpci device.

It seems I need both a zpci device (to specify the uid) and a newer
kernel (so that the uid is actually used to construct the address in
the guest). I guess I should use snapshots.debian.org to get a stable
link to a newer version?

On 26/11/2020 13.05, Cornelia Huck wrote:
> On Wed, 25 Nov 2020 16:30:34 +0100
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
>> On Wed, 25 Nov 2020 16:03:13 +0100
>> Thomas Huth <thuth@redhat.com> wrote:
>>
>>> On 25/11/2020 14.58, Cornelia Huck wrote:  
> 
>>>> +    def test(self):
>>>> +
>>>> +        """
>>>> +        :avocado: tags=arch:s390x
>>>> +        :avocado: tags=machine:s390-ccw-virtio
>>>> +        """
>>>> +
>>>> +        # XXX: switch to https when debian fixes their certificate
>>>> +        kernel_url = ('http://archive.debian.org/debian/dists/jessie/main'
>>>> +                      '/installer-s390x/current/images/generic/kernel.debian')
>>>> +        kernel_hash = '5af1aa839754f4d8817fb5878b4d55dfc887f45d'
>>>> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>>>> +
>>>> +        initrd_url = ('http://archive.debian.org/debian/dists/jessie/main'
>>>> +                      '/installer-s390x/current/images/generic/initrd.debian')
>>>> +        initrd_hash = '99252b28306184b876f979585e2d4bfe96b27464'
>>>> +        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
>>>> +
>>>> +        self.vm.set_console()
>>>> +        kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
>>>> +                              'console=sclp0 root=/dev/ram0 BOOT_DEBUG=3')
>>>> +        self.vm.add_args('-nographic',
>>>> +                         '-kernel', kernel_path,
>>>> +                         '-initrd', initrd_path,
>>>> +                         '-append', kernel_command_line,
>>>> +                         '-device', 'virtio-net-ccw,devno=fe.1.1111',
>>>> +                         '-device', 'virtio-net-pci')    
>>>
>>> Maybe use '-device', 'virtio-net-pci,addr=6' or something similar to check a
>>> non-default PCI address, too?  
>>
>> Not sure if addr= will do the trick, I may need to add a zpci device.
> 
> It seems I need both a zpci device (to specify the uid) and a newer
> kernel (so that the uid is actually used to construct the address in
> the guest). I guess I should use snapshots.debian.org to get a stable
> link to a newer version?

Not sure ... I assume the links to archive.debian.org are less likely to
change? So maybe simply forget about testing a different PCI address for
now, the default should be good enough for a simple check.

 Thomas

PS: Seems like at least "ip addr" is working there already - so you could at
least check the MAC address setting?

On Thu, 26 Nov 2020 13:18:38 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 26/11/2020 13.05, Cornelia Huck wrote:
> > On Wed, 25 Nov 2020 16:30:34 +0100
> > Cornelia Huck <cohuck@redhat.com> wrote:
> >   
> >> On Wed, 25 Nov 2020 16:03:13 +0100
> >> Thomas Huth <thuth@redhat.com> wrote:
> >>  
> >>> On 25/11/2020 14.58, Cornelia Huck wrote:    
> >   
> >>>> +    def test(self):
> >>>> +
> >>>> +        """
> >>>> +        :avocado: tags=arch:s390x
> >>>> +        :avocado: tags=machine:s390-ccw-virtio
> >>>> +        """
> >>>> +
> >>>> +        # XXX: switch to https when debian fixes their certificate
> >>>> +        kernel_url = ('http://archive.debian.org/debian/dists/jessie/main'
> >>>> +                      '/installer-s390x/current/images/generic/kernel.debian')
> >>>> +        kernel_hash = '5af1aa839754f4d8817fb5878b4d55dfc887f45d'
> >>>> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> >>>> +
> >>>> +        initrd_url = ('http://archive.debian.org/debian/dists/jessie/main'
> >>>> +                      '/installer-s390x/current/images/generic/initrd.debian')
> >>>> +        initrd_hash = '99252b28306184b876f979585e2d4bfe96b27464'
> >>>> +        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> >>>> +
> >>>> +        self.vm.set_console()
> >>>> +        kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
> >>>> +                              'console=sclp0 root=/dev/ram0 BOOT_DEBUG=3')
> >>>> +        self.vm.add_args('-nographic',
> >>>> +                         '-kernel', kernel_path,
> >>>> +                         '-initrd', initrd_path,
> >>>> +                         '-append', kernel_command_line,
> >>>> +                         '-device', 'virtio-net-ccw,devno=fe.1.1111',
> >>>> +                         '-device', 'virtio-net-pci')      
> >>>
> >>> Maybe use '-device', 'virtio-net-pci,addr=6' or something similar to check a
> >>> non-default PCI address, too?    
> >>
> >> Not sure if addr= will do the trick, I may need to add a zpci device.  
> > 
> > It seems I need both a zpci device (to specify the uid) and a newer
> > kernel (so that the uid is actually used to construct the address in
> > the guest). I guess I should use snapshots.debian.org to get a stable
> > link to a newer version?  
> 
> Not sure ... I assume the links to archive.debian.org are less likely to
> change? So maybe simply forget about testing a different PCI address for
> now, the default should be good enough for a simple check.

I'd expect the snapshot.debian.org links to be stable as well (we
already use them in some places). The problem is that what is currently
available at archive.debian.org is really quite ancient (3.16 based is
the newest...). If we want to test something that at least resembles
real life usage, we should use at least use a 4.x kernel. The kernels
that do something with the uid will also end up using different default
pci addresses if no uid is provided, so the 0000:00:00.0 seems quite
unrealistic to come up nowadays.

> 
>  Thomas
> 
> PS: Seems like at least "ip addr" is working there already - so you could at
> least check the MAC address setting?

I'd prefer to do this in an add-on patch :)