Re: [PATCH for-10.0] tests/functional: Convert the intel_iommu avocado test

[Eric Auger]

On 12/9/24 09:26, Thomas Huth wrote:
> On 09/12/2024 09.12, Eric Auger wrote:
>> Hi Thomas,
>>
>> On 12/6/24 19:17, Thomas Huth wrote:
>>> Convert the intel_iommu test to the new functional framework.
>>> This test needs some changes since we neither support the old
>>> 'LinuxTest'
>>> class in the functional framework yet, nor a way to use SSH for running
>>> commands in the guest. So we now directly download a Fedora kernel and
>>> initrd and set up the serial console for executing the commands and for
>>> looking for the results.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   MAINTAINERS                                   |   1 +
>>>   tests/functional/meson.build                  |   1 +
>>>   .../test_intel_iommu.py}                      | 119
>>> ++++++++----------
>>>   3 files changed, 51 insertions(+), 70 deletions(-)
>>>   rename tests/{avocado/intel_iommu.py =>
>>> functional/test_intel_iommu.py} (41%)
>>>   mode change 100644 => 100755
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index a62659b330..2ca452dbf9 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -3679,6 +3679,7 @@ S: Supported
>>>   F: hw/i386/intel_iommu.c
>>>   F: hw/i386/intel_iommu_internal.h
>>>   F: include/hw/i386/intel_iommu.h
>>> +F: tests/functional/test_intel_iommu.py
>>>     AMD-Vi Emulation
>>>   S: Orphan
>>> diff --git a/tests/functional/meson.build
>>> b/tests/functional/meson.build
>>> index 30c3eda7e4..dfc95fd904 100644
>>> --- a/tests/functional/meson.build
>>> +++ b/tests/functional/meson.build
>>> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>>>     tests_x86_64_system_thorough = [
>>>     'acpi_bits',
>>> +  'intel_iommu',
>>>     'x86_64_tuxrun',
>>>     'linux_initrd',
>>>     'multiprocess',
>>> diff --git a/tests/avocado/intel_iommu.py
>>> b/tests/functional/test_intel_iommu.py
>>> old mode 100644
>>> new mode 100755
>>> similarity index 41%
>>> rename from tests/avocado/intel_iommu.py
>>> rename to tests/functional/test_intel_iommu.py
>>> index 992583fa7d..6e47b1e9de
>>> --- a/tests/avocado/intel_iommu.py
>>> +++ b/tests/functional/test_intel_iommu.py
>>> @@ -1,3 +1,5 @@
>>> +#!/usr/bin/env python3
>>> +#
>>>   # INTEL_IOMMU Functional tests
>>>   #
>>>   # Copyright (c) 2021 Red Hat, Inc.
>>> @@ -7,116 +9,93 @@
>>>   #
>>>   # 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 import skipUnless
>>> -from avocado_qemu.linuxtest import LinuxTest
>>> +from qemu_test import LinuxKernelTest, Asset,
>>> exec_command_and_wait_for_pattern
>>> +
>>> +class IntelIOMMU(LinuxKernelTest):
>>>   -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable
>>> on GitLab')
>>> -class IntelIOMMU(LinuxTest):
>>> -    """
>>> -    :avocado: tags=arch:x86_64
>>> -    :avocado: tags=distro:fedora
>>> -    :avocado: tags=distro_version:31
>>> -    :avocado: tags=machine:q35
>>> -    :avocado: tags=accel:kvm
>>> -    :avocado: tags=intel_iommu
>>> -    :avocado: tags=flaky
>>> -    """
>>> +    ASSET_KERNEL = Asset(
>>> +       
>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
>>> +       
>>> '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
>>> +
>>> +    ASSET_INITRD = Asset(
>>> +       
>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
>>> +       
>>> '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>>>         IOMMU_ADDON =
>>> ',iommu_platform=on,disable-modern=off,disable-legacy=on'
>>> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>>>       kernel_path = None
>>>       initrd_path = None
>>>       kernel_params = None
>>>   -    def set_up_boot(self):
>>> -        path = self.download_boot()
>>> -        self.vm.add_args('-device', 'virtio-blk-pci,bus=pcie.0,' +
>>> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
>>> -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
>>> -        self.vm.add_args('-device', 'virtio-gpu-pci' +
>>> self.IOMMU_ADDON)
>>> -        self.vm.add_args('-drive',
>>> -                        
>>> 'file=%s,if=none,cache=writethrough,id=drv0' % path)
>>> -
>>> -    def setUp(self):
>>> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' +
>>> self.IOMMU_ADDON)
>>> -
>>>       def add_common_args(self):
>>>           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>>>           self.vm.add_args('-object',
>>>                            'rng-random,id=rng0,filename=/dev/urandom')
>>> +        self.vm.add_args('-device', 'virtio-net-pci' +
>>> self.IOMMU_ADDON)
>>> +        self.vm.add_args('-device', 'virtio-gpu-pci' +
>>> self.IOMMU_ADDON)
>>> +        self.vm.add_args("-m", "1G")
>>>   -    def common_vm_setup(self, custom_kernel=None):
>>> +    def common_vm_setup(self):
>>> +        self.set_machine('q35')
>>>           self.require_accelerator("kvm")
>>>           self.add_common_args()
>>>           self.vm.add_args("-accel", "kvm")
>>>   -        if custom_kernel is None:
>>> -            return
>>> -
>>> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
>>> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
>>> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
>>> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
>>> -        self.kernel_path = self.fetch_asset(kernel_url,
>>> asset_hash=kernel_hash)
>>> -        self.initrd_path = self.fetch_asset(initrd_url,
>>> asset_hash=initrd_hash)
>>> +        self.kernel_path = self.ASSET_KERNEL.fetch()
>>> +        self.initrd_path = self.ASSET_INITRD.fetch()
>>> +        self.kernel_params = self.default_kernel_params
>>>         def run_and_check(self):
>>>           if self.kernel_path:
>>>               self.vm.add_args('-kernel', self.kernel_path,
>>>                                '-append', self.kernel_params,
>>>                                '-initrd', self.initrd_path)
>>> -        self.launch_and_wait()
>>> -        self.ssh_command('cat /proc/cmdline')
>>> -        self.ssh_command('dmesg | grep -e DMAR -e IOMMU')
>>> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
>>> -        self.ssh_command('dnf -y install numactl-devel')
>> I understand you cannot use ssh yet but the bulk of the test was the
>> execution of the dnf install meaning we lose the main substance of it
>> through the conversion.
>
> Ah, I see, so this was exercising the virtio-net device with the IOMMU
> ... and I already wondered why there was this "dnf install" at the end
> without doing anything with  the numactl-devel package ... (a comment
> would have been helpful here)
yes you got it. The point was to exercise the protected virtio-net device.
>
> Ok, then please disregard this version of the patch, I guess I have to
> tackle this in a different way...
np

Eric
>
>  Thomas
>