From 47fdc8fb82fc8dd182b4923a69cefadc87419e0d Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Thu, 17 Nov 2022 17:25:20 +0000 Subject: Run docker probe only if docker or podman are available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docker probe uses "sudo -n" which can cause an e-mail with a security warning each time when configure is run. Therefore run docker probe only if either docker or podman are available. That avoids the problematic "sudo -n" on build environments which have neither docker nor podman installed. Fixes: c4575b59155e2e00 ("configure: store container engine in config-host.mak") Signed-off-by: Stefan Weil Message-Id: <20221030083510.310584-1-sw@weilnetz.de> Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Message-Id: <20221117172532.538149-2-alex.bennee@linaro.org> --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 66928692b0..26c7bc5154 100755 --- a/configure +++ b/configure @@ -1780,7 +1780,7 @@ fi # functions to probe cross compilers container="no" -if test $use_containers = "yes"; then +if test $use_containers = "yes" && (has "docker" || has "podman"); then case $($python "$source_path"/tests/docker/docker.py probe) in *docker) container=docker ;; podman) container=podman ;; -- cgit v1.2.3 From a4b14b46d91eb24415fafd5a8aa8f9514c817a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Le=20Goater?= Date: Thu, 17 Nov 2022 17:25:21 +0000 Subject: tests/avocado/machine_aspeed.py: Reduce noise on the console for SDK tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Aspeed SDK images are based on OpenBMC which starts a lot of services. The output noise on the console can break from time to time the test waiting for the logging prompt. Change the U-Boot bootargs variable to add "quiet" to the kernel command line and reduce the output volume. This also drops the test on the CPU id which was nice to have but not essential. Signed-off-by: Cédric Le Goater Message-Id: <20221104075347.370503-1-clg@kaod.org> Signed-off-by: Alex Bennée Message-Id: <20221117172532.538149-3-alex.bennee@linaro.org> --- tests/avocado/machine_aspeed.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py index fba6527026..1fc385e1c8 100644 --- a/tests/avocado/machine_aspeed.py +++ b/tests/avocado/machine_aspeed.py @@ -12,6 +12,7 @@ from avocado_qemu import QemuSystemTest from avocado_qemu import wait_for_console_pattern from avocado_qemu import exec_command from avocado_qemu import exec_command_and_wait_for_pattern +from avocado_qemu import interrupt_interactive_console_until_pattern from avocado.utils import archive from avocado import skipIf @@ -182,6 +183,8 @@ class AST2x00Machine(QemuSystemTest): class AST2x00MachineSDK(QemuSystemTest): + EXTRA_BOOTARGS = ' quiet' + # FIXME: Although these tests boot a whole distro they are still # slower than comparable machine models. There may be some # optimisations which bring down the runtime. In the meantime they @@ -194,7 +197,7 @@ class AST2x00MachineSDK(QemuSystemTest): failure_message='Kernel panic - not syncing', vm=vm) - def do_test_arm_aspeed_sdk_start(self, image, cpu_id): + def do_test_arm_aspeed_sdk_start(self, image): self.require_netdev('user') self.vm.set_console() self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw', @@ -202,9 +205,13 @@ class AST2x00MachineSDK(QemuSystemTest): self.vm.launch() self.wait_for_console_pattern('U-Boot 2019.04') - self.wait_for_console_pattern('## Loading kernel from FIT Image') + interrupt_interactive_console_until_pattern( + self, 'Hit any key to stop autoboot:', 'ast#') + exec_command_and_wait_for_pattern( + self, 'setenv bootargs ${bootargs}' + self.EXTRA_BOOTARGS, 'ast#') + exec_command_and_wait_for_pattern( + self, 'boot', '## Loading kernel from FIT Image') self.wait_for_console_pattern('Starting kernel ...') - self.wait_for_console_pattern('Booting Linux on physical CPU ' + cpu_id) @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') def test_arm_ast2500_evb_sdk(self): @@ -221,7 +228,7 @@ class AST2x00MachineSDK(QemuSystemTest): archive.extract(image_path, self.workdir) self.do_test_arm_aspeed_sdk_start( - self.workdir + '/ast2500-default/image-bmc', '0x0') + self.workdir + '/ast2500-default/image-bmc') self.wait_for_console_pattern('ast2500-default login:') @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') @@ -243,7 +250,7 @@ class AST2x00MachineSDK(QemuSystemTest): self.vm.add_args('-device', 'ds1338,bus=aspeed.i2c.bus.5,address=0x32'); self.do_test_arm_aspeed_sdk_start( - self.workdir + '/ast2600-default/image-bmc', '0xf00') + self.workdir + '/ast2600-default/image-bmc') self.wait_for_console_pattern('ast2600-default login:') exec_command_and_wait_for_pattern(self, 'root', 'Password:') exec_command_and_wait_for_pattern(self, '0penBmc', 'root@ast2600-default:~#') -- cgit v1.2.3 From e558220df0739474d5877616173cb072df0c8257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:22 +0000 Subject: tests/docker: allow user to override check target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is useful when trying to bisect a particular failing test behind a docker run. For example: make docker-test-clang@fedora \ TARGET_LIST=arm-softmmu \ TEST_COMMAND="meson test qtest-arm/qos-test" \ J=9 V=1 Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221117172532.538149-4-alex.bennee@linaro.org> --- tests/docker/Makefile.include | 2 ++ tests/docker/common.rc | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index c87f14477a..fc7a3b7e71 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -184,6 +184,7 @@ docker: @echo ' TARGET_LIST=a,b,c Override target list in builds.' @echo ' EXTRA_CONFIGURE_OPTS="..."' @echo ' Extra configure options.' + @echo ' TEST_COMMAND="..." Override the default `make check` target.' @echo ' IMAGES="a b c ..": Restrict available images to subset.' @echo ' TESTS="x y z .." Restrict available tests to subset.' @echo ' J=[0..9]* Overrides the -jN parameter for make commands' @@ -230,6 +231,7 @@ docker-run: docker-qemu-src $(if $(NETWORK),$(if $(subst $(NETWORK),,1),--net=$(NETWORK)),--net=none) \ -e TARGET_LIST=$(subst $(SPACE),$(COMMA),$(TARGET_LIST)) \ -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \ + -e TEST_COMMAND="$(TEST_COMMAND)" \ -e V=$V -e J=$J -e DEBUG=$(DEBUG) \ -e SHOW_ENV=$(SHOW_ENV) \ $(if $(NOUSER),, \ diff --git a/tests/docker/common.rc b/tests/docker/common.rc index e6f8cee0d6..9a33df2832 100755 --- a/tests/docker/common.rc +++ b/tests/docker/common.rc @@ -63,12 +63,12 @@ check_qemu() { # default to make check unless the caller specifies if [ $# = 0 ]; then - INVOCATION="check" + INVOCATION="${TEST_COMMAND:-make $MAKEFLAGS check}" else - INVOCATION="$@" + INVOCATION="make $MAKEFLAGS $@" fi - make $MAKEFLAGS $INVOCATION + $INVOCATION } test_fail() -- cgit v1.2.3 From 668725ce6bab12f7d5130fd46d99d0dc6fefe733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:23 +0000 Subject: docs/devel: add a maintainers section to development process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't currently have a clear place in the documentation to describe the roles and responsibilities of a maintainer. Lets create one so we can. I've moved a few small bits out of other files to try and keep everything in one place. Signed-off-by: Alex Bennée Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221117172532.538149-5-alex.bennee@linaro.org> --- MAINTAINERS | 2 +- docs/devel/code-of-conduct.rst | 2 + docs/devel/index-process.rst | 1 + docs/devel/maintainers.rst | 107 +++++++++++++++++++++++++++++++ docs/devel/submitting-a-pull-request.rst | 12 ++-- 5 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 docs/devel/maintainers.rst diff --git a/MAINTAINERS b/MAINTAINERS index be151f0024..366538f9e2 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -23,7 +23,7 @@ Descriptions of section entries: W: Web-page with status/info Q: Patchwork web based patch tracking system site T: SCM tree type and location. Type is one of: git, hg, quilt, stgit. - S: Status, one of the following: + S: Status, one of the following (keep in sync with docs/devel/maintainers.rst): Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. Odd Fixes: It has a maintainer but they don't have time to do diff --git a/docs/devel/code-of-conduct.rst b/docs/devel/code-of-conduct.rst index 195444d1b4..f734ed0317 100644 --- a/docs/devel/code-of-conduct.rst +++ b/docs/devel/code-of-conduct.rst @@ -1,3 +1,5 @@ +.. _code_of_conduct: + Code of Conduct =============== diff --git a/docs/devel/index-process.rst b/docs/devel/index-process.rst index d0d7a200fd..d50dd74c3e 100644 --- a/docs/devel/index-process.rst +++ b/docs/devel/index-process.rst @@ -8,6 +8,7 @@ Notes about how to interact with the community and how and where to submit patch code-of-conduct conflict-resolution + maintainers style submitting-a-patch trivial-patches diff --git a/docs/devel/maintainers.rst b/docs/devel/maintainers.rst new file mode 100644 index 0000000000..5c907d901c --- /dev/null +++ b/docs/devel/maintainers.rst @@ -0,0 +1,107 @@ +.. _maintainers: + +The Role of Maintainers +======================= + +Maintainers are a critical part of the project's contributor ecosystem. +They come from a wide range of backgrounds from unpaid hobbyists +working in their spare time to employees who work on the project as +part of their job. Maintainer activities include: + + - reviewing patches and suggesting changes + - collecting patches and preparing pull requests + - tending to the long term health of their area + - participating in other project activities + +They are also human and subject to the same pressures as everyone else +including overload and burnout. Like everyone else they are subject +to project's :ref:`code_of_conduct` and should also be exemplars of +excellent community collaborators. + +The MAINTAINERS file +-------------------- + +The `MAINTAINERS +`__ +file contains the canonical list of who is a maintainer. The file +is machine readable so an appropriately configured git (see +:ref:`cc_the_relevant_maintainer`) can automatically Cc them on +patches that touch their area of code. + +The file also describes the status of the area of code to give an idea +of how actively that section is maintained. + +.. list-table:: Meaning of support status in MAINTAINERS + :widths: 25 75 + :header-rows: 1 + + * - Status + - Meaning + * - Supported + - Someone is actually paid to look after this. + * - Maintained + - Someone actually looks after it. + * - Odd Fixes + - It has a maintainer but they don't have time to do + much other than throw the odd patch in. + * - Orphan + - No current maintainer. + * - Obsolete + - Old obsolete code, should use something else. + +Please bear in mind that even if someone is paid to support something +it does not mean they are paid to support you. This is open source and +the code comes with no warranty and the project makes no guarantees +about dealing with bugs or features requests. + + + +Becoming a reviewer +------------------- + +Most maintainers start by becoming subsystem reviewers. While anyone +is welcome to review code on the mailing list getting added to the +MAINTAINERS file with a line like:: + + R: Random Hacker + +marks you as a 'designated reviewer' - expected to provide regular +spontaneous feedback. This will ensure that patches touching a given +subsystem will automatically be CC'd to you. + +Becoming a maintainer +--------------------- + +Maintainers are volunteers who put themselves forward or have been +asked by others to keep an eye on an area of code. They have generally +demonstrated to the community, usually via contributions and code +reviews, that they have a good understanding of the subsystem. They +are also trusted to make a positive contribution to the project and +work well with the other contributors. + +The process is simple - simply send a patch to the list that updates +the ``MAINTAINERS`` file. Sometimes this is done as part of a larger +series when a new sub-system is being added to the code base. This can +also be done by a retiring maintainer who nominates their replacement +after discussion with other contributors. + +Once the patch is reviewed and merged the only other step is to make +sure your GPG key is signed. + +.. _maintainer_keys: + +Maintainer GPG Keys +~~~~~~~~~~~~~~~~~~~ + +GPG is used to sign pull requests so they can be identified as really +coming from the maintainer. If your key is not already signed by +members of the QEMU community, you should make arrangements to attend +a `KeySigningParty `__ (for +example at KVM Forum) or make alternative arrangements to have your +key signed by an attendee. Key signing requires meeting another +community member **in person** [#]_ so please make appropriate +arrangements. + +.. [#] In recent pandemic times we have had to exercise some + flexibility here. Maintainers still need to sign their pull + requests though. diff --git a/docs/devel/submitting-a-pull-request.rst b/docs/devel/submitting-a-pull-request.rst index c9d1e8afd9..a4cd7ebbb6 100644 --- a/docs/devel/submitting-a-pull-request.rst +++ b/docs/devel/submitting-a-pull-request.rst @@ -53,14 +53,10 @@ series) and that "make check" passes before sending out the pull request. As a submaintainer you're one of QEMU's lines of defense against bad code, so double check the details. -**All pull requests must be signed**. If your key is not already signed -by members of the QEMU community, you should make arrangements to attend -a `KeySigningParty `__ (for -example at KVM Forum) or make alternative arrangements to have your key -signed by an attendee. Key signing requires meeting another community -member \*in person\* so please make appropriate arrangements. By -"signed" here we mean that the pullreq email should quote a tag which is -a GPG-signed tag (as created with 'gpg tag -s ...'). +**All pull requests must be signed**. By "signed" here we mean that +the pullreq email should quote a tag which is a GPG-signed tag (as +created with 'gpg tag -s ...'). See :ref:`maintainer_keys` for +details. **Pull requests not for master should say "not for master" and have "PULL SUBSYSTEM whatever" in the subject tag**. If your pull request is -- cgit v1.2.3 From 115847f6b05f7a6a3d475208fd120868627f049c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:24 +0000 Subject: docs/devel: make language a little less code centric MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We welcome all sorts of patches. Signed-off-by: Alex Bennée Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221117172532.538149-6-alex.bennee@linaro.org> --- docs/devel/submitting-a-patch.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst index fec33ce148..1319dfd3c2 100644 --- a/docs/devel/submitting-a-patch.rst +++ b/docs/devel/submitting-a-patch.rst @@ -3,11 +3,11 @@ Submitting a Patch ================== -QEMU welcomes contributions of code (either fixing bugs or adding new -functionality). However, we get a lot of patches, and so we have some -guidelines about submitting patches. If you follow these, you'll help -make our task of code review easier and your patch is likely to be -committed faster. +QEMU welcomes contributions to fix bugs, add functionality or improve +the documentation. However, we get a lot of patches, and so we have +some guidelines about submitting them. If you follow these, you'll +help make our task of contribution review easier and your change is +likely to be accepted and committed faster. This page seems very long, so if you are only trying to post a quick one-shot fix, the bare minimum we ask is that: -- cgit v1.2.3 From ca127fe96ddb827f3ea153610c1e8f6e374708e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:25 +0000 Subject: docs/devel: simplify the minimal checklist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bullet points are quite long and contain process tips. Move those bits of the bullet to the relevant sections and link to them. Use a table for nicer formatting of the checklist. Signed-off-by: Alex Bennée Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221117172532.538149-7-alex.bennee@linaro.org> --- docs/devel/submitting-a-patch.rst | 75 +++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 26 deletions(-) diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst index 1319dfd3c2..b2a162ff4c 100644 --- a/docs/devel/submitting-a-patch.rst +++ b/docs/devel/submitting-a-patch.rst @@ -12,25 +12,18 @@ likely to be accepted and committed faster. This page seems very long, so if you are only trying to post a quick one-shot fix, the bare minimum we ask is that: -- You **must** provide a Signed-off-by: line (this is a hard - requirement because it's how you say "I'm legally okay to contribute - this and happy for it to go into QEMU", modeled after the `Linux kernel - `__ - policy.) ``git commit -s`` or ``git format-patch -s`` will add one. -- All contributions to QEMU must be **sent as patches** to the - qemu-devel `mailing list `__. - Patch contributions should not be posted on the bug tracker, posted on - forums, or externally hosted and linked to. (We have other mailing lists too, - but all patches must go to qemu-devel, possibly with a Cc: to another - list.) ``git send-email`` (`step-by-step setup - guide `__ and `hints and - tips `__) - works best for delivering the patch without mangling it, but - attachments can be used as a last resort on a first-time submission. -- You must read replies to your message, and be willing to act on them. - Note, however, that maintainers are often willing to manually fix up - first-time contributions, since there is a learning curve involved in - making an ideal patch submission. +.. list-table:: Minimal Checklist for Patches + :widths: 35 65 + :header-rows: 1 + + * - Check + - Reason + * - Patches contain Signed-off-by: Real Name + - States you are legally able to contribute the code. See :ref:`patch_emails_must_include_a_signed_off_by_line` + * - Sent as patch emails to ``qemu-devel@nongnu.org`` + - The project uses an email list based workflow. See :ref:`submitting_your_patches` + * - Be prepared to respond to review comments + - Code that doesn't pass review will not get merged. See :ref:`participating_in_code_review` You do not have to subscribe to post (list policy is to reply-to-all to preserve CCs and keep non-subscribers in the loop on the threads they @@ -229,6 +222,19 @@ bisection doesn't land on a known-broken state. Submitting your Patches ----------------------- +The QEMU project uses a public email based workflow for reviewing and +merging patches. As a result all contributions to QEMU must be **sent +as patches** to the qemu-devel `mailing list +`__. Patch +contributions should not be posted on the bug tracker, posted on +forums, or externally hosted and linked to. (We have other mailing +lists too, but all patches must go to qemu-devel, possibly with a Cc: +to another list.) ``git send-email`` (`step-by-step setup guide +`__ and `hints and tips +`__) +works best for delivering the patch without mangling it, but +attachments can be used as a last resort on a first-time submission. + .. _if_you_cannot_send_patch_emails: If you cannot send patch emails @@ -314,10 +320,12 @@ git repository to fetch the original commit. Patch emails must include a ``Signed-off-by:`` line ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -For more information see `SubmittingPatches 1.12 -`__. -This is vital or we will not be able to apply your patch! Please use -your real name to sign a patch (not an alias or acronym). +Your patches **must** include a Signed-off-by: line. This is a hard +requirement because it's how you say "I'm legally okay to contribute +this and happy for it to go into QEMU". The process is modelled after +the `Linux kernel +`__ +policy. If you wrote the patch, make sure your "From:" and "Signed-off-by:" lines use the same spelling. It's okay if you subscribe or contribute to @@ -327,6 +335,11 @@ include a "From:" line in the body of the email (different from your envelope From:) that will give credit to the correct author; but again, that author's Signed-off-by: line is mandatory, with the same spelling. +There are various tooling options for automatically adding these tags +include using ``git commit -s`` or ``git format-patch -s``. For more +information see `SubmittingPatches 1.12 +`__. + .. _include_a_meaningful_cover_letter: Include a meaningful cover letter @@ -397,9 +410,19 @@ Participating in Code Review ---------------------------- All patches submitted to the QEMU project go through a code review -process before they are accepted. Some areas of code that are well -maintained may review patches quickly, lesser-loved areas of code may -have a longer delay. +process before they are accepted. This will often mean a series will +go through a number of iterations before being picked up by +:ref:`maintainers`. You therefore should be prepared to +read replies to your messages and be willing to act on them. + +Maintainers are often willing to manually fix up first-time +contributions, since there is a learning curve involved in making an +ideal patch submission. However for the best results you should +proactively respond to suggestions with changes or justifications for +your current approach. + +Some areas of code that are well maintained may review patches +quickly, lesser-loved areas of code may have a longer delay. .. _stay_around_to_fix_problems_raised_in_code_review: -- cgit v1.2.3 From 73ee4c55f7ff95a835584ae54bf840b9281b11d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:26 +0000 Subject: docs/devel: try and improve the language around patch review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is important that contributors take the review process seriously and we collaborate in a respectful way while avoiding personal attacks. Try and make this clear in the language. Signed-off-by: Alex Bennée Reviewed-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221117172532.538149-8-alex.bennee@linaro.org> --- docs/devel/submitting-a-patch.rst | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst index b2a162ff4c..c641d948f1 100644 --- a/docs/devel/submitting-a-patch.rst +++ b/docs/devel/submitting-a-patch.rst @@ -434,14 +434,20 @@ developers will identify bugs, or suggest a cleaner approach, or even just point out code style issues or commit message typos. You'll need to respond to these, and then send a second version of your patches with the issues fixed. This takes a little time and effort on your part, but -if you don't do it then your changes will never get into QEMU. It's also -just polite -- it is quite disheartening for a developer to spend time -reviewing your code and suggesting improvements, only to find that -you're not going to do anything further and it was all wasted effort. +if you don't do it then your changes will never get into QEMU. + +Remember that a maintainer is under no obligation to take your +patches. If someone has spent the time reviewing your code and +suggesting improvements and you simply re-post without either +addressing the comment directly or providing additional justification +for the change then it becomes wasted effort. You cannot demand others +merge and then fix up your code after the fact. When replying to comments on your patches **reply to all and not just the sender** -- keeping discussion on the mailing list means everybody -can follow it. +can follow it. Remember the spirit of the :ref:`code_of_conduct` and +keep discussions respectful and collaborative and avoid making +personal comments. .. _pay_attention_to_review_comments: -- cgit v1.2.3 From 5d25e1e02c5bd297197968668d4351806c65893e Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 17 Nov 2022 17:25:27 +0000 Subject: tests/avocado: Raise timeout for boot_linux.py:BootLinuxPPC64.test_pseries_tcg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On my machine, a debug build of QEMU takes about 260 seconds to complete this test, so with the current timeout value of 180 seconds it always times out. Double the timeout value to 360 so the test definitely has enough time to complete. Signed-off-by: Peter Maydell Signed-off-by: Alex Bennée Message-Id: <20221110142901.3832318-1-peter.maydell@linaro.org> Message-Id: <20221117172532.538149-9-alex.bennee@linaro.org> --- tests/avocado/boot_linux.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py index 32adae6ff6..8c8a63ec2e 100644 --- a/tests/avocado/boot_linux.py +++ b/tests/avocado/boot_linux.py @@ -116,7 +116,7 @@ class BootLinuxPPC64(LinuxTest): :avocado: tags=arch:ppc64 """ - timeout = 180 + timeout = 360 def test_pseries_tcg(self): """ -- cgit v1.2.3 From ba5d1f23f71c6c3c5ff34963bc52f82406ea9f2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:28 +0000 Subject: tests/avocado: introduce alpine virt test for CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The boot_linux tests download and run a full cloud image boot and start a full distro. While the ability to test the full boot chain is worthwhile it is perhaps a little too heavy weight and causes issues in CI. Fix this by introducing a new alpine linux ISO boot in machine_aarch64_virt. This boots a fully loaded -cpu max with all the bells and whistles in 31s on my machine. A full debug build takes around 180s on my machine so we set a more generous timeout to cover that. We don't add a test for lesser GIC versions although there is some coverage for that already in the boot_xen.py tests. If we want to introduce more comprehensive testing we can do it with a custom kernel and initrd rather than a full distro boot. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221117172532.538149-10-alex.bennee@linaro.org> --- tests/avocado/machine_aarch64_virt.py | 46 ++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/avocado/machine_aarch64_virt.py b/tests/avocado/machine_aarch64_virt.py index 21848cba70..c2b2ba2cf8 100644 --- a/tests/avocado/machine_aarch64_virt.py +++ b/tests/avocado/machine_aarch64_virt.py @@ -1,4 +1,5 @@ -# Functional test that boots a Linux kernel and checks the console +# Functional test that boots a various Linux systems and checks the +# console output. # # Copyright (c) 2022 Linaro Ltd. # @@ -8,19 +9,62 @@ # SPDX-License-Identifier: GPL-2.0-or-later import time +import os from avocado_qemu import QemuSystemTest from avocado_qemu import wait_for_console_pattern from avocado_qemu import exec_command +from avocado_qemu import BUILD_DIR class Aarch64VirtMachine(QemuSystemTest): KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 ' + timeout = 360 def wait_for_console_pattern(self, success_message, vm=None): wait_for_console_pattern(self, success_message, failure_message='Kernel panic - not syncing', vm=vm) + # This tests the whole boot chain from EFI to Userspace + # We only boot a whole OS for the current top level CPU and GIC + # Other test profiles should use more minimal boots + def test_alpine_virt_tcg_gic_max(self): + """ + :avocado: tags=arch:aarch64 + :avocado: tags=machine:virt + :avocado: tags=accel:tcg + """ + iso_url = ('https://dl-cdn.alpinelinux.org/' + 'alpine/v3.16/releases/aarch64/' + 'alpine-virt-3.16.3-aarch64.iso') + + # Alpine use sha256 so I recalculated this myself + iso_sha1 = '0683bc089486d55c91bf6607d5ecb93925769bc0' + iso_path = self.fetch_asset(iso_url, asset_hash=iso_sha1) + + self.vm.set_console() + kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE + + 'console=ttyAMA0') + self.require_accelerator("tcg") + + self.vm.add_args("-accel", "tcg") + self.vm.add_args("-cpu", "max,pauth-impdef=on") + self.vm.add_args("-machine", + "virt,acpi=on," + "virtualization=on," + "mte=on," + "gic-version=max,iommu=smmuv3") + self.vm.add_args("-smp", "2", "-m", "1024") + self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios', + 'edk2-aarch64-code.fd')) + self.vm.add_args("-drive", f"file={iso_path},format=raw") + self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0') + self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom') + + self.vm.launch() + self.wait_for_console_pattern('Welcome to Alpine Linux 3.16') + + def test_aarch64_virt(self): """ :avocado: tags=arch:aarch64 -- cgit v1.2.3 From f22a80727fe99f3119a860cf0ad43f4885836d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:29 +0000 Subject: tests/avocado: skip aarch64 cloud TCG tests in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now have a much lighter weight test in machine_aarch64_virt which tests the full boot chain in less time. Rename the tests while we are at it to make it clear it is a Fedora cloud image. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20221117172532.538149-11-alex.bennee@linaro.org> --- tests/avocado/boot_linux.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/avocado/boot_linux.py b/tests/avocado/boot_linux.py index 8c8a63ec2e..b3e58fa309 100644 --- a/tests/avocado/boot_linux.py +++ b/tests/avocado/boot_linux.py @@ -58,6 +58,9 @@ class BootLinuxX8664(LinuxTest): self.launch_and_wait(set_up_ssh_connection=False) +# For Aarch64 we only boot KVM tests in CI as the TCG tests are very +# heavyweight. There are lighter weight distros which we use in the +# machine_aarch64_virt.py tests. class BootLinuxAarch64(LinuxTest): """ :avocado: tags=arch:aarch64 @@ -73,7 +76,8 @@ class BootLinuxAarch64(LinuxTest): self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0') self.vm.add_args('-object', 'rng-random,id=rng0,filename=/dev/urandom') - def test_virt_tcg_gicv2(self): + @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') + def test_fedora_cloud_tcg_gicv2(self): """ :avocado: tags=accel:tcg :avocado: tags=cpu:max @@ -86,7 +90,8 @@ class BootLinuxAarch64(LinuxTest): self.add_common_args() self.launch_and_wait(set_up_ssh_connection=False) - def test_virt_tcg_gicv3(self): + @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab') + def test_fedora_cloud_tcg_gicv3(self): """ :avocado: tags=accel:tcg :avocado: tags=cpu:max -- cgit v1.2.3 From 5544d33d4b3683861315c73eb956492ed8891ce8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 17 Nov 2022 17:25:30 +0000 Subject: gitlab: integrate coverage report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should hopefully give is nice coverage information about what our tests (or at least the subset we are running) have hit. Ideally we would want a way to trigger coverage on tests likely to be affected by the current commit. Signed-off-by: Alex Bennée Acked-by: Stefan Hajnoczi Message-Id: <20221117172532.538149-12-alex.bennee@linaro.org> --- .gitlab-ci.d/buildtest.yml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 7173749c52..d21b4a1fd4 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -494,7 +494,17 @@ check-gprof-gcov: IMAGE: ubuntu2004 MAKE_CHECK_ARGS: check after_script: - - ${CI_PROJECT_DIR}/scripts/ci/coverage-summary.sh + - cd build + - gcovr --xml-pretty --exclude-unreachable-branches --print-summary + -o coverage.xml --root ${CI_PROJECT_DIR} . *.p + coverage: /^\s*lines:\s*\d+.\d+\%/ + artifacts: + name: ${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA} + expire_in: 2 days + reports: + coverage_report: + coverage_format: cobertura + path: build/coverage.xml build-oss-fuzz: extends: .native_build_job_template -- cgit v1.2.3