[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] tests/functional: Convert the intel_iommu avocado test
|
From: |
CLEMENT MATHIEU--DRIF |
|
Subject: |
Re: [PATCH v2] tests/functional: Convert the intel_iommu avocado test |
|
Date: |
Thu, 12 Dec 2024 11:11:47 +0000 |
On 12/12/2024 11:13, Thomas Huth wrote:
> Caution: External email. Do not open attachments or click links,
> unless this email comes from a known sender and you know the content
> is safe.
>
>
> On 12/12/2024 10.23, CLEMENT MATHIEU--DRIF wrote:
>> Hi Thomas,
>>
>> 2 minor comments but LGTM
>>
>> On 10/12/2024 13:56, Thomas Huth wrote:
>>> Caution: External email. Do not open attachments or click links,
>>> unless this email comes from a known sender and you know the content
>>> is safe.
>>>
>>>
>>> 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. Instead of configuring the cloud image via
>>> cloud-init, we now simply mount the file system manually from an initrd
>>> rescue shell.
>>>
>>> While the old test was exercising the network with a "dnf install"
>>> command (which is not the best option for the CI since this depends
>>> on third party servers), the new code is now setting up a little
>>> HTTP server in the guest and transfers a file from the guest to the
>>> host instead.
>>>
>>> The test should now run much faster and more reliable (since we
>>> don't depend on the third party servers for "dnf install" anymore),
>>> so we can also drop the @skipUnless decorator now.
>>>
>>> Signed-off-by: Thomas Huth<thuth@redhat.com>
>>> ---
>>> v2:
>>> - Download the cloud image qcow2 and use it as file system
>>> to exercise the virtio-block device
>>> - Instead of removing the "dnf install", transfer a file
>>> via virtio-net to the host
>>> - This needs find_free_port() now:
>>> Based-on:<20241204071911.664057-1-thuth@redhat.com>
>>>
>>> MAINTAINERS | 1 +
>>> tests/avocado/intel_iommu.py | 122 -------------------
>>> tests/functional/meson.build | 2 +
>>> tests/functional/test_intel_iommu.py | 176
>>> +++++++++++++++++++++++++++
>>> 4 files changed, 179 insertions(+), 122 deletions(-)
>>> delete mode 100644 tests/avocado/intel_iommu.py
>>> create mode 100755 tests/functional/test_intel_iommu.py
>>>
>>> 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/avocado/intel_iommu.py
>>> b/tests/avocado/intel_iommu.py
>>> deleted file mode 100644
>>> index 992583fa7d..0000000000
>>> --- a/tests/avocado/intel_iommu.py
>>> +++ /dev/null
>>> @@ -1,122 +0,0 @@
>>> -# INTEL_IOMMU Functional tests
>>> -#
>>> -# Copyright (c) 2021 Red Hat, Inc.
>>> -#
>>> -# Author:
>>> -# Eric Auger<eric.auger@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 import skipUnless
>>> -from avocado_qemu.linuxtest import LinuxTest
>>> -
>>> -@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
>>> - """
>>> -
>>> - IOMMU_ADDON =
>>> ',iommu_platform=on,disable-modern=off,disable-legacy=on'
>>> - 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')
>>> -
>>> - def common_vm_setup(self, custom_kernel=None):
>>> - 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)
>>> -
>>> - 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')
>>> -
>>> - def test_intel_iommu(self):
>>> - """
>>> - :avocado: tags=intel_iommu_intremap
>>> - """
>>> -
>>> - self.common_vm_setup(True)
>>> - self.vm.add_args('-device', 'intel-iommu,intremap=on')
>>> - self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> -
>>> - self.kernel_params = (self.distro.default_kernel_params +
>>> - ' quiet intel_iommu=on')
>>> - self.run_and_check()
>>> -
>>> - def test_intel_iommu_strict(self):
>>> - """
>>> - :avocado: tags=intel_iommu_strict
>>> - """
>>> -
>>> - self.common_vm_setup(True)
>>> - self.vm.add_args('-device', 'intel-iommu,intremap=on')
>>> - self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> - self.kernel_params = (self.distro.default_kernel_params +
>>> - ' quiet intel_iommu=on,strict')
>>> - self.run_and_check()
>>> -
>>> - def test_intel_iommu_strict_cm(self):
>>> - """
>>> - :avocado: tags=intel_iommu_strict_cm
>>> - """
>>> -
>>> - self.common_vm_setup(True)
>>> - self.vm.add_args('-device',
>>> 'intel-iommu,intremap=on,caching-mode=on')
>>> - self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> - self.kernel_params = (self.distro.default_kernel_params +
>>> - ' quiet intel_iommu=on,strict')
>>> - self.run_and_check()
>>> -
>>> - def test_intel_iommu_pt(self):
>>> - """
>>> - :avocado: tags=intel_iommu_pt
>>> - """
>>> -
>>> - self.common_vm_setup(True)
>>> - self.vm.add_args('-device', 'intel-iommu,intremap=on')
>>> - self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> - self.kernel_params = (self.distro.default_kernel_params +
>>> - ' quiet intel_iommu=on iommu=pt')
>>> - self.run_and_check()
>>> diff --git a/tests/functional/meson.build
>>> b/tests/functional/meson.build
>>> index 30c3eda7e4..84ffb7d98c 100644
>>> --- a/tests/functional/meson.build
>>> +++ b/tests/functional/meson.build
>>> @@ -27,6 +27,7 @@ test_timeouts = {
>>> 'arm_raspi2' : 120,
>>> 'arm_tuxrun' : 240,
>>> 'arm_sx1' : 360,
>>> + 'intel_iommu': 300,
>>> 'mips_malta' : 120,
>>> 'netdev_ethtool' : 180,
>>> 'ppc_40p' : 240,
>>> @@ -238,6 +239,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/functional/test_intel_iommu.py
>>> b/tests/functional/test_intel_iommu.py
>>> new file mode 100755
>>> index 0000000000..e234813cee
>>> --- /dev/null
>>> +++ b/tests/functional/test_intel_iommu.py
>>> @@ -0,0 +1,176 @@
>>> +#!/usr/bin/env python3
>>> +#
>>> +# INTEL_IOMMU Functional tests
>>> +#
>>> +# Copyright (c) 2021 Red Hat, Inc.
>>> +#
>>> +# Author:
>>> +# Eric Auger<eric.auger@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 hashlib
>>> +import urllib.request
>>> +
>>> +from qemu_test import LinuxKernelTest, Asset,
>>> exec_command_and_wait_for_pattern
>>> +from qemu_test.ports import Ports
>>> +
>>> +class IntelIOMMU(LinuxKernelTest):
>>> +
>>> + ASSET_KERNEL = Asset(
>>> +
>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/releases'
>>> + '/31/Server/x86_64/os/images/pxeboot/vmlinuz'),
>>> + 'd4738d03dbbe083ca610d0821d0a8f1488bebbdccef54ce33e3adb35fda00129')
>>> +
>>> + ASSET_INITRD = Asset(
>>> +
>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/releases'
>>> + '/31/Server/x86_64/os/images/pxeboot/initrd.img'),
>>> + '277cd6c7adf77c7e63d73bbb2cded8ef9e2d3a2f100000e92ff1f8396513cd8b')
>>> +
>>> + ASSET_DISKIMAGE = Asset(
>>> +
>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/releases'
>>> + '/31/Cloud/x86_64/images/Fedora-Cloud-Base-31-1.9.x86_64.qcow2'),
>>> + 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0')
>>> +
>>> + default_kernel_params = ('root=/dev/vda1 console=ttyS0
>>> net.ifnames=0 '
>>> + 'quiet rd.rescue ')
>>> + IOMMU_ADDON =
>>> ',iommu_platform=on,disable-modern=off,disable-legacy=on'
>>> + kernel_path = None
>>> + initrd_path = None
>>> + kernel_params = None
>>> +
>>> + def add_common_args(self, path):
>>> + self.vm.add_args('-drive',
>>> f'file={path},if=none,id=drv0,snapshot=on')
>>> + 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('-device', 'virtio-rng-pci,rng=r0')
>>> + self.vm.add_args('-object',
>>> 'rng-random,id=r0,filename=/dev/urandom')
>>> + self.vm.add_args("-m", "1G")
>>> + self.vm.add_args("-accel", "kvm")
>>> +
>>> + def common_vm_setup(self):
>>> + self.set_machine('q35')
>>> + self.require_accelerator("kvm")
>>> + self.require_netdev('user')
>>> +
>>> + self.kernel_path = self.ASSET_KERNEL.fetch()
>>> + self.initrd_path = self.ASSET_INITRD.fetch()
>>> + image_path = self.ASSET_DISKIMAGE.fetch()
>>> + self.add_common_args(image_path)
>>> + self.kernel_params = self.default_kernel_params
>>> +
>>> + def add_nic_args(self, hostport):
>>> + self.vm.add_args('-netdev',
>>> + 'user,id=n1,hostfwd=tcp::%d-:8080' %
>>> hostport)
>>
>> Maybe we should add a constant for 8080 which would help readers
>> understand
>> what it is.
>
> Yes, that's maybe a good idea, especially since I need the value again
> when
> starting the HTTP server in the guest.
>
> Aside from that: I just learnt yesterday that there is also a
> get_info_usernet_hostfwd_port() in the framework, so maybe I could
> even get
> rid of the hostport / find_free_port() function in this test ... so I'll
> send a v3 when I find some spare time for this.
Ok thanks
>
>>> + self.vm.add_args('-device',
>>> + 'virtio-net-pci,netdev=n1' +
>>> self.IOMMU_ADDON)
>>> +
>>> + def run_and_check(self, freeport):
>>> + self.add_nic_args(freeport)
>>> +
>>> + if self.kernel_path:
>>> + self.vm.add_args('-kernel', self.kernel_path,
>>> + '-append', self.kernel_params,
>>> + '-initrd', self.initrd_path)
>>> + self.vm.set_console()
>>> + self.vm.launch()
>>> + self.wait_for_console_pattern('Entering emergency mode.')
>>> + prompt = '# '
>>> + self.wait_for_console_pattern(prompt)
>>> +
>>> + # Copy a file (checked later), umount afterwards to drop
>>> disk cache:
>>> + exec_command_and_wait_for_pattern(self, 'mount /dev/vda1
>>> /sysroot',
>>> + prompt)
>>> + filename = '/boot/initramfs-5.3.7-301.fc31.x86_64.img'
>>
>> Shouldn't we define this after the assets at the top of the class?
>
> Since this is a file within the guest filesystem, not an asset on the
> host,
> I guess it's mostly a matter of taste. As long as it's only required
> in this
> function here, I think I'd rather like to keep it here.
>
> Thomas
>
LGTM
>
>>> + exec_command_and_wait_for_pattern(self, (f'cp /sysroot{filename}'
>>> + '
>>> /sysroot/root/data'),
>>> + prompt)
>>> + exec_command_and_wait_for_pattern(self, 'umount /sysroot',
>>> prompt)
>>> +
>>> + # Switch from initrd to the cloud image filesystem:
>>> + exec_command_and_wait_for_pattern(self, 'mount /dev/vda1
>>> /sysroot',
>>> + prompt)
>>> + exec_command_and_wait_for_pattern(self,
>>> + ('for d in dev proc sys run ; do '
>>> + 'mount -o bind /$d /sysroot/$d ; done'), prompt)
>>> + exec_command_and_wait_for_pattern(self, 'chroot /sysroot',
>>> prompt)
>>> +
>>> + # Checking for IOMMU enablement:
>>> + self.log.info("Checking whether IOMMU has been enabled...")
>>> + exec_command_and_wait_for_pattern(self, 'cat /proc/cmdline',
>>> + 'intel_iommu=on')
>>> + self.wait_for_console_pattern(prompt)
>>> + exec_command_and_wait_for_pattern(self, 'dmesg | grep DMAR:',
>>> + 'IOMMU enabled')
>>> + self.wait_for_console_pattern(prompt)
>>> + exec_command_and_wait_for_pattern(self,
>>> + 'find /sys/kernel/iommu_groups/
>>> -type l',
>>> + 'devices/0000:00:')
>>> + self.wait_for_console_pattern(prompt)
>>> +
>>> + # Check hard disk device via sha256sum:
>>> + self.log.info("Checking hard disk...")
>>> + hashsum =
>>> '0dc7472f879be70b2f3daae279e3ae47175ffe249691e7d97f47222b65b8a720'
>>> + exec_command_and_wait_for_pattern(self, 'sha256sum ' +
>>> filename,
>>> + hashsum)
>>> + self.wait_for_console_pattern(prompt)
>>> + exec_command_and_wait_for_pattern(self, 'sha256sum
>>> /root/data',
>>> + hashsum)
>>> + self.wait_for_console_pattern(prompt)
>>> +
>>> + # Check virtio-net via HTTP:
>>> + exec_command_and_wait_for_pattern(self, 'dhclient eth0',
>>> prompt)
>>> + exec_command_and_wait_for_pattern(self,
>>> + 'python3 -m http.server 8080 &
>>> sleep 1',
>>> + 'Serving HTTP on 0.0.0.0 port
>>> 8080')
>>> + hl = hashlib.sha256()
>>> + url = f'http://localhost:{freeport}{filename}'
>>> + self.log.info(f'Downloading {url} ...')
>>> + with urllib.request.urlopen(url) as response:
>>> + while True:
>>> + chunk = response.read(1 << 20)
>>> + if not chunk:
>>> + break
>>> + hl.update(chunk)
>>> +
>>> + digest = hl.hexdigest()
>>> + self.log.info(f'sha256sum of download is {digest}.')
>>> + self.assertEqual(digest, hashsum)
>>> +
>>> + def test_intel_iommu(self):
>>> + self.common_vm_setup()
>>> + self.vm.add_args('-device', 'intel-iommu,intremap=on')
>>> + self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> + self.kernel_params += 'intel_iommu=on'
>>> + with Ports() as ports:
>>> + self.run_and_check(ports.find_free_port())
>>> +
>>> + def test_intel_iommu_strict(self):
>>> + self.common_vm_setup()
>>> + self.vm.add_args('-device', 'intel-iommu,intremap=on')
>>> + self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> + self.kernel_params += 'intel_iommu=on,strict'
>>> + with Ports() as ports:
>>> + self.run_and_check(ports.find_free_port())
>>> +
>>> + def test_intel_iommu_strict_cm(self):
>>> + self.common_vm_setup()
>>> + self.vm.add_args('-device',
>>> 'intel-iommu,intremap=on,caching-mode=on')
>>> + self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> + self.kernel_params += 'intel_iommu=on,strict'
>>> + with Ports() as ports:
>>> + self.run_and_check(ports.find_free_port())
>>> +
>>> + def test_intel_iommu_pt(self):
>>> + self.common_vm_setup()
>>> + self.vm.add_args('-device', 'intel-iommu,intremap=on')
>>> + self.vm.add_args('-machine', 'kernel_irqchip=split')
>>> + self.kernel_params += 'intel_iommu=on iommu=pt'
>>> + with Ports() as ports:
>>> + self.run_and_check(ports.find_free_port())
>>> +
>>> +if __name__ == '__main__':
>>> + LinuxKernelTest.main()
>>> --
>>> 2.47.1
>>>
>>
>