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

[Philippe Mathieu-Daudé]

On 10/10/2018 17:58, Cleber Rosa wrote:
> 
> 
> On 10/10/18 11:26 AM, Philippe Mathieu-Daudé wrote:
>> On 10/10/2018 16:28, 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.
>>>
>>> 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).
>>
>> I agree with that.
>>
> 
> 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.
>>
>> If I'm going to submit contributions to some subsystem, I'd like to run
>> all the tests that cover this subsystem, previous to annoy the maintainer.
>>
>> For example if a series target the "X86 Machines" subsystem, then I'd
>> expect the JSON variant to test both PC and Q35.
>>
> 
> I agree, and we'll get there, but I'd rather do it in small steps.

Sure.

> 
> The reason is that we want every single FAIL/ERROR on the acceptance
> tests to really flag a regression, so we need careful execution and
> validation prior to increasing the "test matrix".
> 
> At the same time, we need to be careful to not grow the default
> acceptance tests execution to a point that people won't run it. I've
> just heard similar feedback regarding Avocado-VT, that has *too many*
> tests that make it impractical for the individual developer to run.

The problem here is not the number of tests but the set of tests selected.

A test should have a description of what it is testing, the covered
devices/modes/...
>From this description it should be easy to add filtering rules.

>From a developer point of view, I'll run the tests covering the areas I
modified.
A maintainer runs more extensive tests on his subsystems.

Now if you plan a release, you are not an individual developer :)

> 
> The plans we have for that is to define some sane test sets (probably
> based on tags and/or variants files), and at the same time, rely on
> combinatorial independent testing to reduce the number of test
> variations to make it practical for the regular developer to run on his
> environment.

OK, we'll get there! :)

(I don't want to reject people to contribute tests because "we already
have too many".)

Regards,

Phil.