Re: [PATCH 16/16] tests/acceptance: Test the MIPSsim machine

[Philippe Mathieu-Daudé]

On 9/29/20 11:38 AM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
>> +Alex/Daniel
>>
>> On 9/28/20 10:33 PM, Willian Rampazzo wrote:
>>> On Mon, Sep 28, 2020 at 2:31 PM Philippe Mathieu-Daudé <f4bug@amsat.org> 
>>> wrote:
>>>>
>>>> Add a test for the mipssim machine, based on the recommended
>>>> test setup from Thomas Huth:
>>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg606846.html
>>>>
>>>> The test is quick and can be run as:
>>>>
>>>>   $ avocado --show=console run -t machine:mipssim tests/acceptance/
>>>>    (1/1) 
>>>> tests/acceptance/machine_mips_mipssim.py:MipsSimMachine.test_mipssim_linux_console:
>>>>   console: Linux version 3.6.11 (root@711bb8ba16a7) (gcc version 4.8.3 
>>>> (Buildroot 2014.11) ) #2 Sun Sep 27 13:39:35 UTC 2020
>>>>   console: Setting default memory size 0x02000000
>>>>   console: bootconsole [early0] enabled
>>>>   console: CPU revision is: 00019300 (MIPS 24Kc)
>>>>   console: FPU revision is: 00739300
>>>>   ...
>>>>   console: CPU frequency 12.00 MHz
>>>>   console: Calibrating delay loop... 950.27 BogoMIPS (lpj=4751360)
>>>>   ...
>>>>   console: MIPSNet Ethernet driver. Version: 2007-11-17. (c)2005 MIPS 
>>>> Technologies, Inc.
>>>>   ...
>>>>   console: Welcome to Buildroot
>>>>   console: buildroot login: root
>>>>   console: # root
>>>>   console: -sh: root: not found
>>>>   console: # ping -c 3 10.0.2.2
>>>>   console: PING 10.0.2.2 (10.0.2.2): 56 data bytes
>>>>   console: 64 bytes from 10.0.2.2: seq=0 ttl=255 time=48.231 ms
>>>>   console: 64 bytes from 10.0.2.2: seq=1 ttl=255 time=9.407 ms
>>>>   console: 64 bytes from 10.0.2.2: seq=2 ttl=255 time=2.298 ms
>>>>   console: --- 10.0.2.2 ping statistics ---
>>>>   console: 3 packets transmitted, 3 packets received, 0% packet loss
>>>>   PASS (7.99 s)
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>> Cc: Thomas Huth <huth@tuxfamily.org>
>>>> ---
>>>>  MAINTAINERS                              |  1 +
>>>>  tests/acceptance/machine_mips_mipssim.py | 56 ++++++++++++++++++++++++
>>>>  2 files changed, 57 insertions(+)
>>>>  create mode 100644 tests/acceptance/machine_mips_mipssim.py
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 5eed1e692b4..17d8a012b0e 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -240,6 +240,7 @@ F: include/hw/misc/mips_*
>>>>  F: include/hw/timer/mips_gictimer.h
>>>>  F: tests/acceptance/linux_ssh_mips_malta.py
>>>>  F: tests/acceptance/machine_mips_malta.py
>>>> +F: tests/acceptance/machine_mips_mipssim.py
>>>>  F: tests/tcg/mips/
>>>>  K: ^Subject:.*(?i)mips
>>>>
>>>> diff --git a/tests/acceptance/machine_mips_mipssim.py 
>>>> b/tests/acceptance/machine_mips_mipssim.py
>>>> new file mode 100644
>>>> index 00000000000..b2749917b08
>>>> --- /dev/null
>>>> +++ b/tests/acceptance/machine_mips_mipssim.py
>>>> @@ -0,0 +1,56 @@
>>>> +# Functional tests for the MIPS simulator (MIPSsim machine)
>>>> +#
>>>> +# Copyright (c) 2020 Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> +#
>>>> +# This work is licensed under the terms of the GNU GPL, version 2 or 
>>>> later.
>>>> +# See the COPYING file in the top-level directory.
>>>> +#
>>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>>> +
>>>> +import os
>>>> +import logging
>>>> +import time
>>>> +
>>>> +from avocado import skipUnless
>>>> +from avocado_qemu import Test
>>>> +from avocado_qemu import exec_command_and_wait_for_pattern
>>>> +from avocado_qemu import interrupt_interactive_console_until_pattern
>>>> +from avocado_qemu import wait_for_console_pattern
>>>> +
>>>> +class MipsSimMachine(Test):
>>>> +
>>>> +    timeout = 30
>>>> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
>>>> +
>>>> +    @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted 
>>>> code')
>>>> +    def test_mipssim_linux_console(self):
>>>> +        """
>>>> +        Boots the Linux kernel and checks that the console is operational
>>>> +        :avocado: tags=arch:mipsel
>>>> +        :avocado: tags=machine:mipssim
>>>> +        :avocado: tags=device:mipsnet
>>>> +        """
>>>> +        kernel_url = ('https://github.com/philmd/qemu-testing-blob/raw/'
>>>> +                      '32ea5764e1de8fffa0d59366c44822cd06d7c8e0/'
>>>> +                      'mips/mipssim/mipsel/vmlinux')
>>>
>>> So, are you willing to maintain some images on your GitHub to avoid
>>> the image changes when they are not found?
>>
>> No, I am not willing to do that.
>>
>> But I see it pointless to have everyone (including CI) to spend 1h
>> building this image, when the sources and build recipe is available,
>> making the built image reproducible.
> 
> I agree we don't want to build from scratch each time. However as we
> move from relying on third parties (which outside of the major distros
> has proved very flaky) we are getting to the point of managing our GPL
> liabilities.
> 
> After a bit of digging in your branches I see this is a buildroot
> recipe running in Docker. Are they fully reproducible?

Yes, I wrote this Dockerfile to 1/ allow others to use/test but also
2/ so I don't have to manually do this again.

BTW Thomas suggested to add a 'make legal-info' final step, since
buildroot provides it (I was not aware of that):
https://buildroot.org/downloads/manual/manual.html#legal-info

> Could we have a:
> 
>   make acceptance-image-mipsel
> 
> which built the same blob that gets inserted into the avocado cache
> should your github url ever disappear?

Yes but that would mean add this Dockerfile into the QEMU repository,
right? Where to put the .config, QEMU repo too? Then keep the *built*
image + sources + legal-info out of tree. Finally the Avocado test
containing the image SHA1 in QEMU tree. Is that OK?

> 
>> I'm not sure we need to run all the tests all the time, but I'd like
>> to have the tests:
>>
>> 1/ provided with a series, so reviewer are free to rebuild the image
>>    and test this series works as expected,
>> 2/ committed in the repository, so maintainers can verify future changes
>>    doesn't introduce regression.
> 
> Yes these are great goals - my only worry is the shifting sands some of
> these images seem to exist on.
> 
>>>
>>>> +        kernel_hash = '0f9aeca3a2e25b5b0cc4999571f39a7ad58cdc43'
>>>> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>>>> +
>>>> +        initrd_url = ('https://github.com/philmd/qemu-testing-blob/raw/'
>>>> +                      '32ea5764e1de8fffa0d59366c44822cd06d7c8e0/'
>>>> +                      'mips/mipssim/mipsel/rootfs.cpio')
>>>> +        initrd_hash = 'b20359bdfae66387e5a17d6692686d59c189417b'
>>>> +        initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
>>>> +
>>>> +        self.vm.set_console()
>>>> +        self.vm.add_args('-kernel', kernel_path,
>>>> +                         '-initrd', initrd_path,
>>>> +                         '-append', self.KERNEL_COMMON_COMMAND_LINE)
>>>> +        self.vm.launch()
>>>> +
>>>> +        wait_for_console_pattern(self, 'Welcome to Buildroot')
>>>> +        interrupt_interactive_console_until_pattern(self,
>>>> +                                                    
>>>> interrupt_string='root\r',
>>>> +                                                    success_message='#')
>>>> +        pattern = '3 packets transmitted, 3 packets received, 0% packet 
>>>> loss'
>>>> +        exec_command_and_wait_for_pattern(self, 'ping -c 3 10.0.2.2', 
>>>> pattern)
>>>> --
>>>> 2.26.2
>>>>
>>>>
>>>
>>> IMHO, although the solution of spreading images over personal
>>> repositories is not the best, I don't have a strong opinion to block
>>> this, so
>>
>> If the URL is stable (stable commit or even better, tag) it should
>> be OK.
>>
>> tag is used in the quanta-gsj tests, see commit 4fe986dd448
>> ("tests/acceptance: console boot tests for quanta-gsj"):
>>
>>   image_url = (
>>           'https://github.com/hskinnemoen/openbmc/releases/download/'
>>           '20200711-gsj-qemu-0/obmc-phosphor-image-gsj.static.mtd.gz')
>>   image_hash = '14895e634923345cb5c8776037ff7876df96f6b1'
>>
>>>
>>> Reviewed-by: Willian Rampazzo (willianr@redhat.com)
>>>
>>
>> Thanks!
>>
>> Phil.
> 
>