On 7/6/21 4:39 PM, Willian Rampazzo wrote: > On Tue, Jul 6, 2021 at 11:25 AM Philippe Mathieu-Daudé > <phi...@redhat.com> wrote: >> >> On 7/6/21 3:57 PM, Eric Auger wrote: >>> Hi Philippe, >>> >>> On 7/6/21 3:34 PM, Philippe Mathieu-Daudé wrote: >>>> On 7/6/21 3:17 PM, Eric Auger wrote: >>>>> Add new tests checking the good behavior of the SMMUv3 protecting >>>>> 2 virtio pci devices (block and net). We check the guest boots and >>>>> we are able to install a package. Different guest configs are tested: >>>>> standard, passthrough an strict=0. This is tested with both fedora 31 and >>>>> 33. The former uses a 5.3 kernel without range invalidation whereas the >>>>> latter uses a 5.8 kernel that features range invalidation. >>>>> >>>>> Signed-off-by: Eric Auger <eric.au...@redhat.com> >>>>> Reviewed-by: Willian Rampazzo <willi...@redhat.com> >>>>> Reviewed-by: Wainer dos Santos Moschetta <waine...@redhat.com> >>>>> Tested-by: Wainer dos Santos Moschetta <waine...@redhat.com> >>>>> >>>>> --- >>>>> >>>>> v4 -> v5: >>>>> - Added the skipIf statement (William) and William's R-b >>>>> - added Wainer's R-b and T-b >>>>> --- >>>>> tests/acceptance/smmu.py | 133 +++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 133 insertions(+) >>>>> create mode 100644 tests/acceptance/smmu.py >>>>> + 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() >>>> IIUC above is the 'run' part and below is the 'check' part? >>>> >>>> So the check succeed if the VM booted, right? >>>> >>>>> + self.ssh_command('cat /proc/cmdline') >>>>> + self.ssh_command('dnf -y install numactl-devel') >>> >>> For the test to succeed, the guests needs to boot, the ssh connection >>> needs to be established and the dnf command needs to complete. >>> See launch_and_wait() in __init__.py >> >> OK. I see the Test class provide a .fail() method: >> >> fail(msg=None) >> >> Signals a test failure unconditionally, with msg or None >> for the error message. >> >> but no .succeed() one... >> >> Willian: Should we add a comment here mentioning the test >> succeeded by that point? Would it be more explicit to add >> .succeed() in avocado.Test or avocado_qemu.Test? >> > > If the test does not fail, it will succeed anyway. > > I miss some assert statements in this test, telling what is really > tested, but as it won't run on CI, I don't see a problem adding it now > and extending it later.
Sure, but a one-line comment can be easily added by the maintainer taking this series ;)