Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch

[Cleber Rosa]

On 10/10/18 11:47 AM, Cleber Rosa wrote:
> 
> 
> On 10/10/18 10:28 AM, Eduardo Habkost wrote:
>> On Wed, Oct 10, 2018 at 10:15:15AM -0400, Cleber Rosa wrote:
>>>
>>>
>>> On 10/10/18 9:59 AM, Cleber Rosa wrote:
>>>>
>>>>
>>>> On 10/10/18 9:46 AM, Eduardo Habkost wrote:
>>>>> On Wed, Oct 10, 2018 at 08:35:38AM -0400, Cleber Rosa wrote:
>>>>>>
>>>>>>
>>>>>> On 10/10/18 7:00 AM, Philippe Mathieu-Daudé wrote:
>>>>>>> On 10/10/2018 01:26, Cleber Rosa wrote:
>>>>>>>> Some targets require a machine type to be set, as there's no default
>>>>>>>> (aarch64 is one example).  To give a consistent interface to users of
>>>>>>>> this API, this changes set_machine() so that a predefined default can
>>>>>>>> be used, if one is not given.  The approach used is exactly the same
>>>>>>>> with the console device type.
>>>>>>>>
>>>>>>>> Also, even when there's a default machine type, for some purposes,
>>>>>>>> testing included, it's better if outside code is explicit about the
>>>>>>>> machine type, instead of relying on whatever is set internally.
>>>>>>>>
>>>>>>>> Signed-off-by: Cleber Rosa <address@hidden>
>>>>>>>> ---
>>>>>>>>  scripts/qemu.py | 22 +++++++++++++++++++++-
>>>>>>>>  1 file changed, 21 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>>>>>>>> index d9e24a0c1a..fca9b76990 100644
>>>>>>>> --- a/scripts/qemu.py
>>>>>>>> +++ b/scripts/qemu.py
>>>>>>>> @@ -36,6 +36,15 @@ CONSOLE_DEV_TYPES = {
>>>>>>>>      r'^s390-ccw-virtio.*': 'sclpconsole',
>>>>>>>>      }
>>>>>>>>  
>>>>>>>> +#: Maps archictures to the preferred machine type
>>>>>>>> +MACHINE_TYPES = {
>>>>>>>> +    r'^aarch64$': 'virt',
>>>>>>>> +    r'^ppc$': 'g3beige',
>>>>>>>> +    r'^ppc64$': 'pseries',
>>>>>>>> +    r'^s390x$': 's390-ccw-virtio',
>>>>>>>> +    r'^x86_64$': 'q35',
>>>>>>>
>>>>>>> Why choose Q35 rather than PC (the default)?
>>>>>>>
>>>>>>> I was wondering about how to generate variants/machines.json but this is
>>>>>>> definitively something we want to do via a QMP query.
>>>>>>>
>>>>>>> Eduardo what do you think?
>>>>>>>
>>>>>>
>>>>>> It was motivated by Eduardo's initiative to make q35 the default "across
>>>>>> the board".  He can confirm and give more details.
>>>>>
>>>>> Making Q35 the default on applications using QEMU and libvirt is
>>>>> something I'd like to happen.  But I think the simplest way to do
>>>>> that is to change the QEMU default.  This way you won't need this
>>>>> table on qemu.py: you can just use the default provided by QEMU.
>>>>>
>>>>
>>>> The idea is to bring consistency on how we're calling
>>>> "qemu-system-$(ARCH)", and at the same time apply the "explicit is
>>>> better than implicit" rule.
>>>>
>>>> The most important fact is that some targets do not (currently) have
>>>> "the default provided by QEMU", aarch64 is one of them.
>>>>
>>>> - Cleber.
>>>>
>>>
>>> So I ended up not relaying the question properly: should we default
>>> (even if explicitly adding "-machine") to "pc"?
>>
>> I think using the default machine-type (when QEMU has a default)
>> would be less surprising for users of the qemu.py API.
>>
> 
> OK, agreed.
> 
>> Implicitly adding -machine when there's no default is also
>> surprising, but then it's a nice surprise: instead of crashing
>> you get a running VM.
>>
>> Now, there are two other questions related to this:
>>
>> If using 'pc' as default, should we always add -machine, or just
>> omit the machine-type name?  I think we should omit it unless the
>> caller asked for a specific machine-type name (because it would
>> be less surprising for users of the API).
>>
> 

Getting down to business, trying to apply those changes, I was faced
with a situation.  Actually, the same situation I faced a few months
ago.  Handling it was defered until it was *really* a blocker.
Basically the issue is: the set_console() method, which gives tests a
ready to use console, depends on knowing the machine type (see
CONSOLE_DEV_TYPES).

As a case study, let's look at "boot_console_linux.py":
 1) it sets the machine type explicitly
 2) it has nothing to do with the specific machine type
 3) the setting of a machine type is boiler plate code to set a console
 4) the console is used on the test's real purpose: verifying the Linux
kernel booted

Now, to be able to run the same test -- booting a Linux kernel -- on
*other target archs*, we need the same machinery.  Even more important:
to have similar tests we'll need to either abstract those features or
duplicate them.  This can be seen, at least in part, on the firmware
tests that Philippe sent to the list: they would also benefit from
having a console device ready to be used on the configured machine type[1]:

Assuming that we want to provide this type of machinery for free (or as
close as that) to the acceptance/functional tests, we need some source
of "known good" configuration for the targets we aim to support.

Let's restrict the discussion to the issue at hand, machine types, while
keeping in mind that the same pattern happened with devices types to use
as console before, and my experience running into default network device
types in further work (tests that interact with the guest by ssh'ing
into it).

The solutions I can think of are:

 1) run the target binary previous to the "real" run, and query
information -- this is what Avocado-VT does[2], and something I tried on
earlier versions of the acceptance tests infrastructure code

 2) attempt to get this information from the build system[3]

 3) hard code the "known" good configuration

I've previously worked on solutions along the lines of #1 and #2, but
the general feedback wasn't that positive, for valid reasons.  Eduardo
probably remembers this.

So, I'm tempted to try solution #3.  As much as duplicating target
defaults in qemu.py doesn't sound perfect, it seems to be the more
predictable and attainable solution at this point.

Thoughts?

Thanks!
- Cleber.

[1] - https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg00591.html
[2] -
https://github.com/avocado-framework/avocado-vt/blob/65.0/virttest/utils_misc.py#L2105
[3] - http://lists.gnu.org/archive/html/qemu-devel/2017-07/msg06757.html

> OK.
> 
>> About our default testing configuration for acceptance tests:
>> should acceptance tests run against PC by default?  Should it
>> test Q35?  Should we test both PC and Q35?  I'm not sure what's
>> the answer, but I think these decisions shouldn't affect the
>> qemu.py API at all.
>>
> 
> OK.
> 
> To make sure we're on the same page, we're still going to have default
> machine types, based on the arch, for those targets that don't provide
> one (aarch64 is one example).  Right?
> 
> - Cleber.
>