From: Cleber Rosa <crosa@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
"Eduardo Habkost" <ehabkost@redhat.com>
Cc: "Fam Zheng" <famz@redhat.com>, "Laszlo Ersek" <lersek@redhat.com>,
qemu-devel@nongnu.org,
"Philippe Mathieu-Daudé" <pmathieu@redhat.com>,
"Stefan Hajnoczi" <stefanha@redhat.com>,
"Caio Carrara" <ccarrara@redhat.com>,
"Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch
Date: Wed, 10 Oct 2018 14:08:36 -0400 [thread overview]
Message-ID: <3bbd53aa-fab9-91c2-82be-e70b3bedeba9@redhat.com> (raw)
In-Reply-To: <0840c8a1-fba4-647e-c4a6-cd40ef3b7564@redhat.com>
On 10/10/18 12:08 PM, Philippe Mathieu-Daudé wrote:
> 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 <crosa@redhat.com>
>>>>>>>>>> ---
>>>>>>>>>> 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.
>
And a CI system may run even more. And QE will hopefully run an absurd
number of tests.
And, all of those groups will be working and collaborating on the same
code base, with no secret sauces.
"I have a dream", and that dream is developer A reporting a test
failure, and developer B checking the CI/QE database to find out that it
also happens on different scenarios - or only on a specific scenario.
IMO this can quickly point you towards the failure cause.
> Now if you plan a release, you are not an individual developer :)
>
Yep, true.
>>
>> 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".)
>
Oh no, I agree with you. I'm already envisioning a few things:
* Proposing a set of tags
* Combinatorial Independent Testing
* Maybe breaking down the "tests/acceptance" directory into target and
non-target specific (think of "qemu-img" only tests)
* Using the Avocado Job API to define the test suites (some old
example, still Avocado-VT related:
https://www.redhat.com/archives/avocado-devel/2018-April/msg00015.html)
Regards,
- Cleber.
> Regards,
>
> Phil.
>
next prev parent reply other threads:[~2018-10-10 18:08 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-09 23:26 [Qemu-devel] [PATCH v2 0/7] Acceptance Tests: basic architecture support Cleber Rosa
2018-10-09 23:26 ` [Qemu-devel] [PATCH v2 1/7] Acceptance Tests: improve docstring on pick_default_qemu_bin() Cleber Rosa
2018-10-09 23:26 ` [Qemu-devel] [PATCH v2 2/7] Acceptance Tests: introduce arch parameter and attribute Cleber Rosa
2018-10-10 11:03 ` Philippe Mathieu-Daudé
2018-10-09 23:26 ` [Qemu-devel] [PATCH v2 3/7] scripts/qemu.py: add method and private attribute for arch Cleber Rosa
2018-10-10 10:52 ` Philippe Mathieu-Daudé
2018-10-09 23:26 ` [Qemu-devel] [PATCH v2 4/7] scripts/qemu.py: set predefined machine type based on arch Cleber Rosa
2018-10-10 11:00 ` Philippe Mathieu-Daudé
2018-10-10 12:35 ` Cleber Rosa
2018-10-10 13:46 ` Eduardo Habkost
2018-10-10 13:59 ` Cleber Rosa
2018-10-10 14:15 ` Cleber Rosa
2018-10-10 14:28 ` Eduardo Habkost
2018-10-10 15:26 ` Philippe Mathieu-Daudé
2018-10-10 15:58 ` Cleber Rosa
2018-10-10 16:08 ` Philippe Mathieu-Daudé
2018-10-10 18:08 ` Cleber Rosa [this message]
2018-10-10 15:31 ` Daniel P. Berrangé
2018-10-10 16:02 ` Cleber Rosa
2018-10-10 15:47 ` Cleber Rosa
2018-10-10 16:23 ` Peter Maydell
2018-10-10 17:52 ` Cleber Rosa
2018-10-10 18:07 ` Peter Maydell
2018-10-10 19:54 ` Cleber Rosa
2018-10-11 17:31 ` Peter Maydell
2018-10-11 0:17 ` Cleber Rosa
2018-10-11 3:42 ` Eduardo Habkost
2018-10-11 4:43 ` Cleber Rosa
2018-10-11 17:21 ` Eduardo Habkost
2018-10-09 23:26 ` [Qemu-devel] [PATCH v2 5/7] Acceptance Tests: set machine type Cleber Rosa
2018-10-09 23:26 ` [Qemu-devel] [PATCH v2 6/7] Acceptance Tests: add variants definition for architectures Cleber Rosa
2018-10-10 10:51 ` Philippe Mathieu-Daudé
2018-10-10 12:59 ` Cleber Rosa
2018-10-10 10:59 ` Philippe Mathieu-Daudé
2018-10-10 12:48 ` Cleber Rosa
2018-10-09 23:26 ` [Qemu-devel] [PATCH v2 7/7] Acceptance Tests: change the handling of tests for specific archs Cleber Rosa
2018-10-10 10:50 ` Philippe Mathieu-Daudé
2018-10-10 13:09 ` Cleber Rosa
2018-10-13 11:08 ` Philippe Mathieu-Daudé
2018-10-15 13:52 ` Cleber Rosa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3bbd53aa-fab9-91c2-82be-e70b3bedeba9@redhat.com \
--to=crosa@redhat.com \
--cc=alex.bennee@linaro.org \
--cc=ccarrara@redhat.com \
--cc=ehabkost@redhat.com \
--cc=famz@redhat.com \
--cc=lersek@redhat.com \
--cc=philmd@redhat.com \
--cc=pmathieu@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.