Re: [PATCH v4 0/4] avocado-qemu: New SMMUv3 and intel IOMMU tests

[Eric Auger]

Hi William, Philippe,

On 7/5/21 11:24 PM, Willian Rampazzo wrote:
> On Mon, Jul 5, 2021 at 6:20 PM Philippe Mathieu-Daudé <philmd@redhat.com> 
> wrote:
>> On 7/5/21 11:10 PM, Willian Rampazzo wrote:
>>> Hi Eric,
>>>
>>> On Mon, Jul 5, 2021 at 4:55 AM Eric Auger <eric.auger@redhat.com> wrote:
>>>> Hi Wainer,
>>>>
>>>> On 7/1/21 1:22 AM, Wainer dos Santos Moschetta wrote:
>>>>> Hi,
>>>>>
>>>>> On 6/29/21 5:17 PM, Eric Auger wrote:
>>>>>> Hi Cleber, all,
>>>>>>
>>>>>> On 6/29/21 4:36 PM, Eric Auger wrote:
>>>>>>> This series adds ARM SMMU and Intel IOMMU functional
>>>>>>> tests using Fedora cloud-init images.
>>>>>>>
>>>>>>> ARM SMMU tests feature guests with and without RIL
>>>>>>> (range invalidation support) using respectively fedora 33
>>>>>>> and 31.  For each, we test the protection of virtio-net-pci
>>>>>>> and virtio-block-pci devices. Also strict=no and passthrough
>>>>>>> modes are tested. So there is a total of 6 tests.
>>>>>>>
>>>>>>> The series applies on top of Cleber's series:
>>>>>>> - [PATCH 0/3] Acceptance Tests: support choosing specific
>>>>>>>
>>>>>>> Note:
>>>>>>> - SMMU tests 2, 3, 5, 6 (resp. test_smmu_noril_passthrough and
>>>>>>> test_smmu_noril_nostrict) pass but the log reports:
>>>>>>> "WARN: Test passed but there were warnings during execution."
>>>>>>> This seems due to the lack of hash when fetching the kernel and
>>>>>>> initrd through fetch_asset():
>>>>>>> WARNI| No hash provided. Cannot check the asset file integrity.
>>>>>> I wanted to emphasize that point and wondered how we could fix that
>>>>>> issue. Looks a pity the tests get tagged as WARN due to a lack of sha1.
>>>>>> Any advice?
>>>>> As Willian mentioned somewhere, to supress the WARN you can pass the
>>>>> kernel and initrd checksums (sha1) to the fetch_asset() method.
>>>>>
>>>>> Below is an draft implementation. It would need to fill out the
>>>>> remaining checksums and adjust the `smmu.py` tests.
>>>>>
>>>>> - Wainer
>>>>>
>>>>> ----
>>>>>
>>>>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>>>>> b/tests/acceptance/avocado_qemu/__init__.py
>>>>> index 00eb0bfcc8..83637e2654 100644
>>>>> --- a/tests/acceptance/avocado_qemu/__init__.py
>>>>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>>>>> @@ -312,6 +312,8 @@ class LinuxDistro:
>>>>>                  {'checksum':
>>>>> 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0',
>>>>>                  'pxeboot_url':
>>>>> "https://archives.fedoraproject.org/pub/archive/fedora/";
>>>>> "linux/releases/31/Everything/x86_64/os/images/pxeboot/",
>>>>> +                'pxeboot_initrd_chksum':
>>>>> 'dd0340a1b39bd28f88532babd4581c67649ec5b1',
>>>>> +                'pxeboot_vmlinuz_chksum':
>>>>> '5b6f6876e1b5bda314f93893271da0d5777b1f3c',
>>>> where did you get the checksum? I don't see any at the URL? Did you
>>>> generate it yourself?
>>> It is possible to use the hash you generate from the downloaded file.
>>>
>>> While I was reviewing this series, I thought it makes more sense to
>>> have Wainer's path applied first and then have your changes. I did
>>> this here, with the addition of myu suggestions in the series:
>>> https://gitlab.com/willianrampazzo/qemu/-/commits/test_eric_auger_v5.
>> Off-list review is a bit unhandy (in particular when asked on the list).
>>
>> Why don't you post your improvements as v5? I don't think Eric will be
>> offended: this is the opposite, you are helping him to get his patches
>> merged ;)
> Oh, I did review each of his patches in the list and also already made
> the changes to speed up the process :)

Yes the review is really helpful. I will have a double check at your
changes and respin quickly.
>
> He mentioned today to me that his series is still depending on one
> from Cleber that was not merged yet, so we need to wait for that.
Yep. The soft freeze is quickly approaching and that would be cool to
see those tests landing upstream.

Thanks

Eric
>
>>> Feel free to pick it and resend a new version.
>>>
>>> Wainer, check if you agree with the changes to your patch and ack it.
>>>
>>> Regards,