Alex Bennée [Wed, 1 Jul 2020 13:56:50 +0000 (14:56 +0100)]
containers.yml: build with docker.py tooling
Instead of building the docker files directly use the same docker.py
scripting as we do for building locally. This should help ensure we
use the exact same steps and allow us to cache properly when building
locally.
To get this working you have to have a fairly recent docker binary
otherwise you will see the error message:
=> ERROR importing cache manifest from registry.gitlab....
So far docker 19.03.12 works (from the docker apt repos) but 18.09.1,
build
4c52b90 which is packaged in Debian Buster fails.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200701135652.1366-39-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:49 +0000 (14:56 +0100)]
gitlab: limit re-builds of the containers
Most of the time we are just rebuilding the same things. We can skip
this although currently there is no mechanism for picking up new
distro releases.
Rather than try to be too fine grained allow any change to trigger all
the images being rebuilt.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-38-alex.bennee@linaro.org>
Daniel P. Berrangé [Fri, 10 Jul 2020 06:07:19 +0000 (08:07 +0200)]
tests: improve performance of device-introspect-test
Total execution time with "-m slow" and x86_64 QEMU, drops from 3
minutes 15 seconds, down to 54 seconds.
Individual tests drop from 17-20 seconds, down to 3-4 seconds.
The cost of this change is that any QOM bugs resulting in the test
failure will not be directly associated with the device that caused
the failure. The test case is not frequently identifying such bugs
though, and the cause is likely easily visible in the patch series
that causes the failure. So overall the shorter running time is
considered the more important factor.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
[thuth: Add the tree check to test_device_intro_none() and
test_device_intro_abstract(), too, just to be sure...]
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200710060719.22386-1-thuth@redhat.com>
Alex Bennée [Wed, 1 Jul 2020 13:56:47 +0000 (14:56 +0100)]
gitlab: add avocado asset caching
These can be quite big so lets cache them. I couldn't find any nots on
ccache in the gitlab docs so I've just ignored it for now.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-36-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:46 +0000 (14:56 +0100)]
gitlab: enable check-tcg for linux-user tests
Switch to building in the new debian-all-test-cross image which has
most of the cross compilers inline.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <
20200701135652.1366-35-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:45 +0000 (14:56 +0100)]
linux-user/elfload: use MAP_FIXED_NOREPLACE in pgb_reserved_va
Given we assert the requested address matches what we asked we should
also make that clear in the mmap flags. Otherwise we see failures in
the GitLab environment for some currently unknown but allowable
reason. We use MAP_FIXED_NOREPLACE if we can so we don't just clobber
an existing mapping. Also include the strerror string for a bit more
info on failure.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200701135652.1366-34-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:44 +0000 (14:56 +0100)]
tests/docker: add a linux-user testing focused image
We happily use all the cross images for both cross-building QEMU as
well as building the linux-user tests. However calling docker from
within docker seems not to work. As we can build in Debian anyway why
not include an image that has all the compilers available for
non-docker invocation.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-33-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:43 +0000 (14:56 +0100)]
tests/tcg: add more default compilers to configure.sh
We were missing a bunch of compilers which we could use if they were
locally installed. The defaults are based on Debian as they seem to be
the best distro for well distributed cross-build compilers.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200701135652.1366-32-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:42 +0000 (14:56 +0100)]
gitlab: add acceptance testing to system builds
As part of migrating things from Travis to GitLab add the acceptance
tests. To do this:
- rename system1 to system-ubuntu-main
- rename system2 to system-fedora-misc
- split into build/check/acceptance
- remove -j from check stages
- use artifacts to save build stage
- add post acceptance template and use
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-31-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:41 +0000 (14:56 +0100)]
tests/acceptance: skip LinuxInitrd 2gib with v4.16 on GitLab
This fails on GitLab but not when run locally on the same container
image. It's very confusing.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <
20200701135652.1366-30-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:40 +0000 (14:56 +0100)]
tests/acceptance: skip multicore mips_malta tests on GitLab
For some reason these tests fail all the time on GitLab. I can
re-create the hang around 3% of the time locally but it doesn't seem
to be MTTCG related. For now skipIf on GITLAB_CI.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Message-Id: <
20200701135652.1366-29-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:39 +0000 (14:56 +0100)]
tests/acceptance: fix dtb path for machine_rx_gdbsim
The old path doesn't exist but the rx-virt.dtb file has the same
checksum so lets use that.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <
20200701135652.1366-28-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:38 +0000 (14:56 +0100)]
tests/acceptance: skip s390x_ccw_vrtio_tcg on GitLab
Currently the test takes more the 900 seconds on GitLab and then times
out. Running on Travis seems to be OK.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200701135652.1366-27-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:37 +0000 (14:56 +0100)]
tests/docker: add packages needed for check-acceptance
We need additional python packages to run check-acceptance. Add them
to the docker images we will be using later.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <
20200701135652.1366-26-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:36 +0000 (14:56 +0100)]
tests/docker: add --registry support to tooling
This allows us to point the tools towards a registry from which they
can grab pre-built layers instead of doing everything from scratch
each time. To enable this we need to be using the DOCKER_BUILDKIT
engine.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200701135652.1366-25-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:35 +0000 (14:56 +0100)]
gitlab: build containers with buildkit and metadata
According to the documentation to be able to use --cache-from for
remote registries you need to enable both buildkit and inline the
metadata. We want to do this to support pulling from gitlab when users
build their local docker images.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200701135652.1366-24-alex.bennee@linaro.org>
Daniel P. Berrangé [Wed, 1 Jul 2020 13:56:34 +0000 (14:56 +0100)]
gitlab: convert jobs to use custom built containers
Now that we're building standard container images from
dockerfiles in tests/docker/dockerfiles, we can convert
the build jobs to use them. The key benefit of this is
that a contributor can now more easily replicate the CI
environment on their local machine. The container images
are cached too, so we are not spending time waiting for
the apt-get/dnf package installs to complete.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <
20200622153318.751107-4-berrange@redhat.com>
[AJB: tweak naming convention]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <
20200701135652.1366-23-alex.bennee@linaro.org>
Daniel P. Berrangé [Wed, 1 Jul 2020 13:56:33 +0000 (14:56 +0100)]
gitlab: build all container images during CI
We have a number of container images in tests/docker/dockerfiles
that are intended to provide well defined environments for doing
test builds. We want our CI system to use these containers too.
This introduces builds of all of them as the first stage in the
CI, so that the built containers are available for later build
jobs. The containers are setup to use the GitLab container
registry as the cache, so we only pay the penalty of the full
build when the dockerfiles change. The main qemu-project/qemu
repo is used as a second cache, so that users forking QEMU will
see a fast turnaround time on their CI jobs.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <
20200622153318.751107-3-berrange@redhat.com>
[AJB: tweak the tag format]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Thomas Huth <thuth@redhat.com>
Message-Id: <
20200701135652.1366-22-alex.bennee@linaro.org>
Daniel P. Berrangé [Wed, 1 Jul 2020 13:56:32 +0000 (14:56 +0100)]
gitlab: introduce explicit "container" and "build" stages
If no stage is listed, jobs get put in an implicit "test" stage.
Some jobs which create container images to be used by later stages
are currently listed as in a "build" stages.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200622153318.751107-2-berrange@redhat.com>
Message-Id: <
20200701135652.1366-21-alex.bennee@linaro.org>
Thomas Huth [Wed, 1 Jul 2020 13:56:31 +0000 (14:56 +0100)]
gitlab-ci: Fix the change rules after moving the YML files
The edk2.yml and opensbi.yml files have recently been moved/renamed,
but the change has not been reflected in the rules in the YML files
yet.
Fixes: 922febe2af ("Move edk2 and opensbi YAML files to .gitlab-ci.d folder")
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <
20200625151627.24986-1-thuth@redhat.com>
Message-Id: <
20200701135652.1366-20-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:30 +0000 (14:56 +0100)]
.gitignore: un-ignore .gitlab-ci.d
The sooner we deprecate in-tree builds the sooner this mess of regexes
can be thrown away.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-19-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:29 +0000 (14:56 +0100)]
tests/docker: change tag naming scheme of our images
We've been misusing the tag naming scheme for some time by overloading
the post : section with the image type. Really it should be saved for
the revision of that particular build. Move the details to the other
side so we have:
qemu/image-name
with the implied :latest version added by the tooling.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-18-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:28 +0000 (14:56 +0100)]
tests/docker: check for an parameters not empty string
Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-17-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:27 +0000 (14:56 +0100)]
tests/vm: allow us to take advantage of MTTCG
We currently limit TCG guests to -smp 1 but now we have added some
aarch64 guests we can do better when running on x86_64 hardware.
Raise the limit for TCG guests when it is safe to do so.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200701135652.1366-16-alex.bennee@linaro.org>
Alex Bennée [Wed, 1 Jul 2020 13:56:26 +0000 (14:56 +0100)]
tests/vm: switch from optsparse to argparse
optparse has been deprecated since version 3.2 and argparse is the
blessed replacement. Take the opportunity to enhance our help output
showing defaults when called.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Robert Foley <robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-15-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:25 +0000 (14:56 +0100)]
tests/vm: Add workaround to consume console
This adds support to basevm.py so that we always
drain the console chars. This makes use of
support added in an earlier commit that allows
QEMUMachine to use the ConsoleSocket.
This is a workaround we found was needed since
there is a known issue where QEMU will hang waiting
for console characters to be consumed.
We also added the option of logging the console to a file.
LOG_CONSOLE=1 will now log the output to a file.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-10-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-14-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:24 +0000 (14:56 +0100)]
python/qemu: Add ConsoleSocket for optional use in QEMUMachine
We add the ConsoleSocket object, which has a socket interface
and which will consume all arriving characters on the
socket, placing them into an in memory buffer.
This will also provide those chars via recv() as
would a regular socket.
ConsoleSocket also has the option of dumping
the console bytes to a log file.
We also give QEMUMachine the option of using ConsoleSocket
to drain and to use for logging console to a file.
By default QEMUMachine does not use ConsoleSocket.
This is added in preparation for use by basevm.py in a later commit.
This is a workaround we found was needed for basevm.py since
there is a known issue where QEMU will hang waiting
for console characters to be consumed.
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Cleber Rosa <crosa@redhat.com>
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-9-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-13-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:23 +0000 (14:56 +0100)]
tests/vm: change scripts to use self._config
This change converts existing scripts to using for example self.ROOT_PASS,
to self._config['root_pass'].
We made similar changes for GUEST_USER, and GUEST_PASS.
This allows us also to remove the change in basevm.py,
which adds __getattr__ for backwards compatibility.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200601211421.1277-8-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-12-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:22 +0000 (14:56 +0100)]
tests/vm: Added a new script for centos.aarch64.
centos.aarch64 creates a CentOS 8 image.
Also added a new kickstart script used to build the centos.aarch64 image.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-7-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-11-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:21 +0000 (14:56 +0100)]
tests/vm: Added a new script for ubuntu.aarch64.
ubuntu.aarch64 provides a script to create an Ubuntu 18.04 VM.
Another new file is also added aarch64vm.py, which is a module with
common methods used by aarch64 VMs, such as how to create the
flash images.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-6-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-10-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:20 +0000 (14:56 +0100)]
tests/vm: Add common Ubuntu python module
Add a common Ubuntu python module and make use of
it with the ubuntu.i386 script.
This is preparation for adding an Ubuntu script
ubuntu.aarch64. Splitting out the common
logic such as build_image() will reduce duplication.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-5-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-9-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:19 +0000 (14:56 +0100)]
tests/vm: Added configuration file support
Changes to tests/vm/basevm.py to allow accepting a configuration file
as a parameter. Allows for specifying VM options such as
cpu, machine, memory, and arbitrary qemu arguments for specifying options
such as NUMA configuration.
Also added an example conf_example_aarch64.yml and conf_example_x86.yml.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-4-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-8-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:18 +0000 (14:56 +0100)]
tests/vm: Add configuration to basevm.py
Added use of a configuration to tests/vm/basevm.py.
The configuration provides parameters used to configure a VM.
This allows for providing alternate configurations to the VM being
created/launched. cpu, machine, memory, and NUMA configuration are all
examples of configuration which we might want to vary on the VM being created
or launched.
This will for example allow for creating an aarch64 vm.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Peter Puhov <peter.puhov@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-3-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-7-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:17 +0000 (14:56 +0100)]
tests/vm: pass args through to BaseVM's __init__
Adding the args parameter to BaseVM's __init__.
We will shortly need to pass more parameters to the class
so let's just pass args rather than growing the parameter list.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200601211421.1277-2-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-6-alex.bennee@linaro.org>
Robert Foley [Wed, 1 Jul 2020 13:56:16 +0000 (14:56 +0100)]
util/coroutine: Cleanup start_switch_fiber_ for TSAN.
This is a cleanup patch to follow-up the patch which introduced TSAN.
This patch makes separate start_switch_fiber_ functions for TSAN and ASAN.
This does two things:
1. Unrelated ASAN and TSAN code is separate and each function only
has arguments that are actually needed.
2. The co->tsan_caller_fiber and co->tsan_co_fiber fields are only
access from within #ifdef CONFIG_TSAN.
Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <
20200626170001.27017-1-robert.foley@linaro.org>
Message-Id: <
20200701135652.1366-5-alex.bennee@linaro.org>
David Edmondson [Wed, 1 Jul 2020 13:56:15 +0000 (14:56 +0100)]
crypto/linux_keyring: fix 'secret_keyring' configure test
The configure test for 'secret_keyring' incorrectly checked the
'have_keyring' variable.
Fixes: 54e7aac0562452e4fcab65ca5001d030eef2de15
Signed-off-by: David Edmondson <david.edmondson@oracle.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <
20200618092636.71832-1-david.edmondson@oracle.com>
Message-Id: <
20200701135652.1366-4-alex.bennee@linaro.org>
Peter Maydell [Sat, 11 Jul 2020 12:56:03 +0000 (13:56 +0100)]
Merge remote-tracking branch 'remotes/aperard/tags/pull-xen-
20200710' into staging
xen patches
Fixes following harden checks in qdev.
# gpg: Signature made Fri 10 Jul 2020 14:05:46 BST
# gpg: using RSA key
F80C006308E22CFD8A92E7980CF5572FD7FB55AF
# gpg: issuer "anthony.perard@citrix.com"
# gpg: Good signature from "Anthony PERARD <anthony.perard@gmail.com>" [marginal]
# gpg: aka "Anthony PERARD <anthony.perard@citrix.com>" [marginal]
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg: It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 5379 2F71 024C 600F 778A 7161 D8D5 7199 DF83 42C8
# Subkey fingerprint: F80C 0063 08E2 2CFD 8A92 E798 0CF5 572F D7FB 55AF
* remotes/aperard/tags/pull-xen-
20200710:
xen: cleanup unrealized flash devices
xen: Fix xen-legacy-backend qdev types
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 10 Jul 2020 18:29:07 +0000 (19:29 +0100)]
Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging
virtio: bugfix
fixes vdpa on non-kvm platforms
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
# gpg: Signature made Wed 08 Jul 2020 11:17:48 BST
# gpg: using RSA key
5D09FD0871C8F85B94CA8A0D281F0DB8D28D5469
# gpg: issuer "mst@redhat.com"
# gpg: Good signature from "Michael S. Tsirkin <mst@kernel.org>" [full]
# gpg: aka "Michael S. Tsirkin <mst@redhat.com>" [full]
# Primary key fingerprint: 0270 606B 6F3C DF3D 0B17 0970 C350 3912 AFBE 8E67
# Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA 8A0D 281F 0DB8 D28D 5469
* remotes/mst/tags/for_upstream:
vhost-vdpa: fix the compile issue without kvm
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 10 Jul 2020 15:43:40 +0000 (16:43 +0100)]
Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-
20200707' into staging
qemu-openbios queue
# gpg: Signature made Tue 07 Jul 2020 21:57:37 BST
# gpg: using RSA key
CC621AB98E82200D915CC9C45BC2C56FAE0F321F
# gpg: issuer "mark.cave-ayland@ilande.co.uk"
# gpg: Good signature from "Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>" [full]
# Primary key fingerprint: CC62 1AB9 8E82 200D 915C C9C4 5BC2 C56F AE0F 321F
* remotes/mcayland/tags/qemu-openbios-
20200707:
Update OpenBIOS images to
75fbb41d built from submodule.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 10 Jul 2020 13:41:23 +0000 (14:41 +0100)]
Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2020-07-07-v2' into staging
Error reporting patches patches for 2020-07-07
# gpg: Signature made Fri 10 Jul 2020 14:24:42 BST
# gpg: using RSA key
354BC8B3D7EB2A6B68674E5F3870B400EB918653
# gpg: issuer "armbru@redhat.com"
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full]
# gpg: aka "Markus Armbruster <armbru@pond.sub.org>" [full]
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867 4E5F 3870 B400 EB91 8653
* remotes/armbru/tags/pull-error-2020-07-07-v2: (53 commits)
xen: Use ERRP_GUARD()
nbd: Use ERRP_GUARD()
virtio-9p: Use ERRP_GUARD()
fw_cfg: Use ERRP_GUARD()
pflash: Use ERRP_GUARD()
sd: Use ERRP_GUARD()
scripts: Coccinelle script to use ERRP_GUARD()
error: New macro ERRP_GUARD()
hmp: Ignore Error objects where the return value suffices
qdev: Ignore Error objects where the return value suffices
qemu-img: Ignore Error objects where the return value suffices
error: Avoid error_propagate() after migrate_add_blocker()
qapi: Purge error_propagate() from QAPI core
qapi: Smooth visitor error checking in generated code
qapi: Smooth another visitor error checking pattern
block/parallels: Simplify parallels_open() after previous commit
error: Reduce unnecessary error propagation
error: Eliminate error_propagate() manually
error: Eliminate error_propagate() with Coccinelle, part 2
error: Eliminate error_propagate() with Coccinelle, part 1
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:37 +0000 (18:50 +0200)]
xen: Use ERRP_GUARD()
If we want to check error after errp-function call, we need to
introduce local_err and then propagate it to errp. Instead, use
the ERRP_GUARD() macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_GUARD() leaves errp as is if it's not NULL or
&error_fatal, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_GUARD() macro.
Otherwise, this info will not be added when errp == &error_fatal
(the program will exit prior to the error_append_hint() or
error_prepend() call). No such cases are being fixed here.
This commit is generated by command
sed -n '/^X86 Xen CPUs$/,/^$/{s/^F: //p}' MAINTAINERS | \
xargs git ls-files | grep '\.[hc]$' | \
xargs spatch \
--sp-file scripts/coccinelle/errp-guard.cocci \
--macro-file scripts/cocci-macro-file.h \
--in-place --no-show-diff --max-width 80
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-9-armbru@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci. Commit message
tweaked again.]
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:36 +0000 (18:50 +0200)]
nbd: Use ERRP_GUARD()
If we want to check error after errp-function call, we need to
introduce local_err and then propagate it to errp. Instead, use
the ERRP_GUARD() macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_GUARD() leaves errp as is if it's not NULL or
&error_fatal, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_GUARD() macro.
Otherwise, this info will not be added when errp == &error_fatal
(the program will exit prior to the error_append_hint() or
error_prepend() call). Fix several such cases, e.g. in nbd_read().
This commit is generated by command
sed -n '/^Network Block Device (NBD)$/,/^$/{s/^F: //p}' \
MAINTAINERS | \
xargs git ls-files | grep '\.[hc]$' | \
xargs spatch \
--sp-file scripts/coccinelle/errp-guard.cocci \
--macro-file scripts/cocci-macro-file.h \
--in-place --no-show-diff --max-width 80
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-8-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci. Commit message
tweaked again.]
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:35 +0000 (18:50 +0200)]
virtio-9p: Use ERRP_GUARD()
If we want to check error after errp-function call, we need to
introduce local_err and then propagate it to errp. Instead, use
the ERRP_GUARD() macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_GUARD() leaves errp as is if it's not NULL or
&error_fatal, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_GUARD() macro.
Otherwise, this info will not be added when errp == &error_fatal
(the program will exit prior to the error_append_hint() or
error_prepend() call). Fix such a case in
v9fs_device_realize_common().
This commit is generated by command
sed -n '/^virtio-9p$/,/^$/{s/^F: //p}' MAINTAINERS | \
xargs git ls-files | grep '\.[hc]$' | \
xargs spatch \
--sp-file scripts/coccinelle/errp-guard.cocci \
--macro-file scripts/cocci-macro-file.h \
--in-place --no-show-diff --max-width 80
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Acked-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-7-armbru@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci. Commit message
tweaked again.]
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:34 +0000 (18:50 +0200)]
fw_cfg: Use ERRP_GUARD()
If we want to check error after errp-function call, we need to
introduce local_err and then propagate it to errp. Instead, use
the ERRP_GUARD() macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_GUARD() leaves errp as is if it's not NULL or
&error_fatal, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_GUARD() macro.
Otherwise, this info will not be added when errp == &error_fatal
(the program will exit prior to the error_append_hint() or
error_prepend() call). No such cases are being fixed here.
This commit is generated by command
sed -n '/^Firmware configuration (fw_cfg)$/,/^$/{s/^F: //p}' \
MAINTAINERS | \
xargs git ls-files | grep '\.[hc]$' | \
xargs spatch \
--sp-file scripts/coccinelle/errp-guard.cocci \
--macro-file scripts/cocci-macro-file.h \
--in-place --no-show-diff --max-width 80
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-6-armbru@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci. Commit message
tweaked again. Coccinelle script rerun for commit
3203148917
"hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface"]
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:33 +0000 (18:50 +0200)]
pflash: Use ERRP_GUARD()
If we want to check error after errp-function call, we need to
introduce local_err and then propagate it to errp. Instead, use
the ERRP_GUARD() macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_GUARD() leaves errp as is if it's not NULL or
&error_fatal, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_GUARD() macro.
Otherwise, this info will not be added when errp == &error_fatal
(the program will exit prior to the error_append_hint() or
error_prepend() call). No such cases are being fixed here.
This commit is generated by command
sed -n '/^Parallel NOR Flash devices$/,/^$/{s/^F: //p}' \
MAINTAINERS | \
xargs git ls-files | grep '\.[hc]$' | \
xargs spatch \
--sp-file scripts/coccinelle/errp-guard.cocci \
--macro-file scripts/cocci-macro-file.h \
--in-place --no-show-diff --max-width 80
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-5-armbru@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci. Commit message
tweaked again.]
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:32 +0000 (18:50 +0200)]
sd: Use ERRP_GUARD()
If we want to check error after errp-function call, we need to
introduce local_err and then propagate it to errp. Instead, use
the ERRP_GUARD() macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_GUARD() leaves errp as is if it's not NULL or
&error_fatal, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_GUARD() macro.
Otherwise, this info will not be added when errp == &error_fatal
(the program will exit prior to the error_append_hint() or
error_prepend() call). No such cases are being fixed here.
This commit is generated by command
sed -n '/^SD (Secure Card)$/,/^$/{s/^F: //p}' \
MAINTAINERS | \
xargs git ls-files | grep '\.[hc]$' | \
xargs spatch \
--sp-file scripts/coccinelle/errp-guard.cocci \
--macro-file scripts/cocci-macro-file.h \
--in-place --no-show-diff --max-width 80
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-4-armbru@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci. Commit message
tweaked again.]
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:31 +0000 (18:50 +0200)]
scripts: Coccinelle script to use ERRP_GUARD()
Script adds ERRP_GUARD() macro invocations where appropriate and
does corresponding changes in code (look for details in
include/qapi/error.h)
Usage example:
spatch --sp-file scripts/coccinelle/errp-guard.cocci \
--macro-file scripts/cocci-macro-file.h --in-place --no-show-diff \
--max-width 80 FILES...
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[ERRP_AUTO_PROPAGATE() renamed to ERRP_GUARD(), and
auto-propagated-errp.cocci to errp-guard.cocci]
Vladimir Sementsov-Ogievskiy [Tue, 7 Jul 2020 16:50:30 +0000 (18:50 +0200)]
error: New macro ERRP_GUARD()
Introduce a new ERRP_GUARD() macro, to be used at start of functions
with an errp OUT parameter.
It has three goals:
1. Fix issue with error_fatal and error_prepend/error_append_hint: the
user can't see this additional information, because exit() happens in
error_setg earlier than information is added. [Reported by Greg Kurz]
2. Fix issue with error_abort and error_propagate: when we wrap
error_abort by local_err+error_propagate, the resulting coredump will
refer to error_propagate and not to the place where error happened.
(the macro itself doesn't fix the issue, but it allows us to [3.] drop
the local_err+error_propagate pattern, which will definitely fix the
issue) [Reported by Kevin Wolf]
3. Drop local_err+error_propagate pattern, which is used to workaround
void functions with errp parameter, when caller wants to know resulting
status. (Note: actually these functions could be merely updated to
return int error code).
To achieve these goals, later patches will add invocations
of this macro at the start of functions with either use
error_prepend/error_append_hint (solving 1) or which use
local_err+error_propagate to check errors, switching those
functions to use *errp instead (solving 2 and 3).
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Merge comments properly with recent commit "error: Document Error API
usage rules", and edit for clarity. Put ERRP_AUTO_PROPAGATE() before
its helpers, and touch up style. Tweak commit message.]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707165037.
1026246-2-armbru@redhat.com>
[Rename ERRP_AUTO_PROPAGATE() to ERRP_GUARD(), tweak commit message
again]
Markus Armbruster [Tue, 7 Jul 2020 16:06:13 +0000 (18:06 +0200)]
hmp: Ignore Error objects where the return value suffices
qdev_print_props() receives and throws away Error objects just to
check for object_property_get_str() and object_property_print()
failure. Unnecessary, both return suitable values, so use those
instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-46-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:12 +0000 (18:06 +0200)]
qdev: Ignore Error objects where the return value suffices
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-45-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:11 +0000 (18:06 +0200)]
qemu-img: Ignore Error objects where the return value suffices
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-44-armbru@redhat.com>
[One more in img_amend() due to commit
0bc2a50e17 "qemu-option: Use
returned bool to check for failure"]
Markus Armbruster [Tue, 7 Jul 2020 16:06:10 +0000 (18:06 +0200)]
error: Avoid error_propagate() after migrate_add_blocker()
When migrate_add_blocker(blocker, &errp) is followed by
error_propagate(errp, err), we can often just as well do
migrate_add_blocker(..., errp).
Do that with this Coccinelle script:
@@
expression blocker, err, errp;
expression ret;
@@
- ret = migrate_add_blocker(blocker, &err);
- if (err) {
+ ret = migrate_add_blocker(blocker, errp);
+ if (ret < 0) {
... when != err;
- error_propagate(errp, err);
...
}
@@
expression blocker, err, errp;
@@
- migrate_add_blocker(blocker, &err);
- if (err) {
+ if (migrate_add_blocker(blocker, errp) < 0) {
... when != err;
- error_propagate(errp, err);
...
}
Double-check @err is not used afterwards. Dereferencing it would be
use after free, but checking whether it's null would be legitimate.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-43-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:09 +0000 (18:06 +0200)]
qapi: Purge error_propagate() from QAPI core
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-42-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:08 +0000 (18:06 +0200)]
qapi: Smooth visitor error checking in generated code
Use visitor functions' return values to check for failure. Eliminate
error_propagate() that are now unnecessary. Delete @err that are now
unused.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-41-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:07 +0000 (18:06 +0200)]
qapi: Smooth another visitor error checking pattern
Convert
visit_type_FOO(v, ..., &ptr, &err);
...
if (err) {
...
}
to
visit_type_FOO(v, ..., &ptr, errp);
...
if (!ptr) {
...
}
for functions that set @ptr to non-null / null on success / error.
Eliminate error_propagate() that are now unnecessary. Delete @err
that are now unused.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-40-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:06 +0000 (18:06 +0200)]
block/parallels: Simplify parallels_open() after previous commit
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-39-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:05 +0000 (18:06 +0200)]
error: Reduce unnecessary error propagation
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away, even when we need to keep error_propagate() for other
error paths.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-38-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:04 +0000 (18:06 +0200)]
error: Eliminate error_propagate() manually
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away. The previous two commits did that for sufficiently simple
cases with Coccinelle. Do it for several more manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-37-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:03 +0000 (18:06 +0200)]
error: Eliminate error_propagate() with Coccinelle, part 2
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away. The previous commit did that with a Coccinelle script I
consider fairly trustworthy. This commit uses the same script with
the matching of return taken out, i.e. we convert
if (!foo(..., &err)) {
...
error_propagate(errp, err);
...
}
to
if (!foo(..., errp)) {
...
...
}
This is unsound: @err could still be read between afterwards. I don't
know how to express "no read of @err without an intervening write" in
Coccinelle. Instead, I manually double-checked for uses of @err.
Suboptimal line breaks tweaked manually. qdev_realize() simplified
further to placate scripts/checkpatch.pl.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-36-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:02 +0000 (18:06 +0200)]
error: Eliminate error_propagate() with Coccinelle, part 1
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away. Convert
if (!foo(..., &err)) {
...
error_propagate(errp, err);
...
return ...
}
to
if (!foo(..., errp)) {
...
...
return ...
}
where nothing else needs @err. Coccinelle script:
@rule1 forall@
identifier fun, err, errp, lbl;
expression list args, args2;
binary operator op;
constant c1, c2;
symbol false;
@@
if (
(
- fun(args, &err, args2)
+ fun(args, errp, args2)
|
- !fun(args, &err, args2)
+ !fun(args, errp, args2)
|
- fun(args, &err, args2) op c1
+ fun(args, errp, args2) op c1
)
)
{
... when != err
when != lbl:
when strict
- error_propagate(errp, err);
... when != err
(
return;
|
return c2;
|
return false;
)
}
@rule2 forall@
identifier fun, err, errp, lbl;
expression list args, args2;
expression var;
binary operator op;
constant c1, c2;
symbol false;
@@
- var = fun(args, &err, args2);
+ var = fun(args, errp, args2);
... when != err
if (
(
var
|
!var
|
var op c1
)
)
{
... when != err
when != lbl:
when strict
- error_propagate(errp, err);
... when != err
(
return;
|
return c2;
|
return false;
|
return var;
)
}
@depends on rule1 || rule2@
identifier err;
@@
- Error *err = NULL;
... when != err
Not exactly elegant, I'm afraid.
The "when != lbl:" is necessary to avoid transforming
if (fun(args, &err)) {
goto out
}
...
out:
error_propagate(errp, err);
even though other paths to label out still need the error_propagate().
For an actual example, see sclp_realize().
Without the "when strict", Coccinelle transforms vfio_msix_setup(),
incorrectly. I don't know what exactly "when strict" does, only that
it helps here.
The match of return is narrower than what I want, but I can't figure
out how to express "return where the operand doesn't use @err". For
an example where it's too narrow, see vfio_intx_enable().
Silently fails to convert hw/arm/armsse.c, because Coccinelle gets
confused by ARMSSE being used both as typedef and function-like macro
there. Converted manually.
Line breaks tidied up manually. One nested declaration of @local_err
deleted manually. Preexisting unwanted blank line dropped in
hw/riscv/sifive_e.c.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-35-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:01 +0000 (18:06 +0200)]
error: Avoid unnecessary error_propagate() after error_setg()
Replace
error_setg(&err, ...);
error_propagate(errp, err);
by
error_setg(errp, ...);
Related pattern:
if (...) {
error_setg(&err, ...);
goto out;
}
...
out:
error_propagate(errp, err);
return;
When all paths to label out are that way, replace by
if (...) {
error_setg(errp, ...);
return;
}
and delete the label along with the error_propagate().
When we have at most one other path that actually needs to propagate,
and maybe one at the end that where propagation is unnecessary, e.g.
foo(..., &err);
if (err) {
goto out;
}
...
bar(..., &err);
out:
error_propagate(errp, err);
return;
move the error_propagate() to where it's needed, like
if (...) {
foo(..., &err);
error_propagate(errp, err);
return;
}
...
bar(..., errp);
return;
and transform the error_setg() as above.
In some places, the transformation results in obviously unnecessary
error_propagate(). The next few commits will eliminate them.
Bonus: the elimination of gotos will make later patches in this series
easier to review.
Candidates for conversion tracked down with this Coccinelle script:
@@
identifier err, errp;
expression list args;
@@
- error_setg(&err, args);
+ error_setg(errp, args);
... when != err
error_propagate(errp, err);
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-34-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:06:00 +0000 (18:06 +0200)]
qdev: Use returned bool to check for failure, Coccinelle part
The previous commit enables conversion of
qdev_prop_set_drive_err(..., &err);
if (err) {
...
}
to
if (!qdev_prop_set_drive_err(..., errp)) {
...
}
Coccinelle script:
@@
identifier fun = qdev_prop_set_drive_err;
expression list args;
typedef Error;
Error *err;
@@
- fun(args, &err);
- if (err)
+ if (!fun(args, &err))
{
...
}
One line break tidied up manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <
20200707160613.848843-33-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:59 +0000 (18:05 +0200)]
qdev: Make functions taking Error ** return bool, not void
See recent commit "error: Document Error API usage rules" for
rationale.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-32-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:58 +0000 (18:05 +0200)]
qom: Make functions taking Error ** return bool, not 0/-1
Just for consistency. Also fix the example in object_set_props()'s
documentation.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-31-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:57 +0000 (18:05 +0200)]
qom: Use returned bool to check for failure, manual part
The previous commit used Coccinelle to convert from checking the Error
object to checking the return value. Convert a few more manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-30-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:56 +0000 (18:05 +0200)]
qom: Use returned bool to check for failure, Coccinelle part
The previous commit enables conversion of
foo(..., &err);
if (err) {
...
}
to
if (!foo(..., errp)) {
...
}
for QOM functions that now return true / false on success / error.
Coccinelle script:
@@
identifier fun = {
object_apply_global_props, object_initialize_child_with_props,
object_initialize_child_with_propsv, object_property_get,
object_property_get_bool, object_property_parse, object_property_set,
object_property_set_bool, object_property_set_int,
object_property_set_link, object_property_set_qobject,
object_property_set_str, object_property_set_uint, object_set_props,
object_set_propv, user_creatable_add_dict,
user_creatable_complete, user_creatable_del
};
expression list args, args2;
typedef Error;
Error *err;
@@
- fun(args, &err, args2);
- if (err)
+ if (!fun(args, &err, args2))
{
...
}
Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
ARMSSE being used both as typedef and function-like macro there.
Convert manually.
Line breaks tidied up manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-29-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:55 +0000 (18:05 +0200)]
qom: Make functions taking Error ** return bool, not void
See recent commit "error: Document Error API usage rules" for
rationale.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-28-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:54 +0000 (18:05 +0200)]
qom: Put name parameter before value / visitor parameter
The object_property_set_FOO() setters take property name and value in
an unusual order:
void object_property_set_FOO(Object *obj, FOO_TYPE value,
const char *name, Error **errp)
Having to pass value before name feels grating. Swap them.
Same for object_property_set(), object_property_get(), and
object_property_parse().
Convert callers with this Coccinelle script:
@@
identifier fun = {
object_property_get, object_property_parse, object_property_set_str,
object_property_set_link, object_property_set_bool,
object_property_set_int, object_property_set_uint, object_property_set,
object_property_set_qobject
};
expression obj, v, name, errp;
@@
- fun(obj, v, name, errp)
+ fun(obj, name, v, errp)
Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error
message "no position information". Convert that one manually.
Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
ARMSSE being used both as typedef and function-like macro there.
Convert manually.
Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused
by RXCPU being used both as typedef and function-like macro there.
Convert manually. The other files using RXCPU that way don't need
conversion.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-27-armbru@redhat.com>
[Straightforwad conflict with commit
2336172d9b "audio: set default
value for pcspk.iobase property" resolved]
Markus Armbruster [Tue, 7 Jul 2020 16:05:53 +0000 (18:05 +0200)]
qom: Use return values to check for error where that's simpler
When using the Error object to check for error, we need to receive it
into a local variable, then propagate() it to @errp.
Using the return value permits allows receiving it straight to @errp.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-26-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:52 +0000 (18:05 +0200)]
qom: Don't handle impossible object_property_get_link() failure
Don't handle object_property_get_link() failure that can't happen
unless the programmer screwed up, pass &error_abort.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200707160613.848843-25-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:51 +0000 (18:05 +0200)]
qom: Crash more nicely on object_property_get_link() failure
Pass &error_abort instead of NULL where the returned value is
dereferenced or asserted to be non-null. Drop a now redundant
assertion.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-24-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:50 +0000 (18:05 +0200)]
qom: Rename qdev_get_type() to object_get_type()
Commit
2f262e06f0 lifted qdev_get_type() from qdev to object without
renaming it accordingly. Do that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-23-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:49 +0000 (18:05 +0200)]
qom: Use error_reportf_err() instead of g_printerr() in examples
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-22-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:48 +0000 (18:05 +0200)]
s390x/pci: Fix harmless mistake in zpci's property fid's setter
s390_pci_set_fid() sets zpci->fid_defined to true even when
visit_type_uint32() failed. Reproducer: "-device zpci,fid=junk".
Harmless in practice, because qdev_device_add() then fails, throwing
away @zpci. Fix it anyway.
Cc: Matthew Rosato <mjrosato@linux.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <
20200707160613.848843-21-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:47 +0000 (18:05 +0200)]
qapi: Use returned bool to check for failure, manual part
The previous commit used Coccinelle to convert from checking the Error
object to checking the return value. Convert a few more manually.
Also tweak control flow in places to conform to the conventional "if
error bail out" pattern.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-20-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:46 +0000 (18:05 +0200)]
qapi: Use returned bool to check for failure, Coccinelle part
The previous commit enables conversion of
visit_foo(..., &err);
if (err) {
...
}
to
if (!visit_foo(..., errp)) {
...
}
for visitor functions that now return true / false on success / error.
Coccinelle script:
@@
identifier fun =~ "check_list|input_type_enum|lv_start_struct|lv_type_bool|lv_type_int64|lv_type_str|lv_type_uint64|output_type_enum|parse_type_bool|parse_type_int64|parse_type_null|parse_type_number|parse_type_size|parse_type_str|parse_type_uint64|print_type_bool|print_type_int64|print_type_null|print_type_number|print_type_size|print_type_str|print_type_uint64|qapi_clone_start_alternate|qapi_clone_start_list|qapi_clone_start_struct|qapi_clone_type_bool|qapi_clone_type_int64|qapi_clone_type_null|qapi_clone_type_number|qapi_clone_type_str|qapi_clone_type_uint64|qapi_dealloc_start_list|qapi_dealloc_start_struct|qapi_dealloc_type_anything|qapi_dealloc_type_bool|qapi_dealloc_type_int64|qapi_dealloc_type_null|qapi_dealloc_type_number|qapi_dealloc_type_str|qapi_dealloc_type_uint64|qobject_input_check_list|qobject_input_check_struct|qobject_input_start_alternate|qobject_input_start_list|qobject_input_start_struct|qobject_input_type_any|qobject_input_type_bool|qobject_input_type_bool_keyval|qobject_input_type_int64|qobject_input_type_int64_keyval|qobject_input_type_null|qobject_input_type_number|qobject_input_type_number_keyval|qobject_input_type_size_keyval|qobject_input_type_str|qobject_input_type_str_keyval|qobject_input_type_uint64|qobject_input_type_uint64_keyval|qobject_output_start_list|qobject_output_start_struct|qobject_output_type_any|qobject_output_type_bool|qobject_output_type_int64|qobject_output_type_null|qobject_output_type_number|qobject_output_type_str|qobject_output_type_uint64|start_list|visit_check_list|visit_check_struct|visit_start_alternate|visit_start_list|visit_start_struct|visit_type_.*";
expression list args;
typedef Error;
Error *err;
@@
- fun(args, &err);
- if (err)
+ if (!fun(args, &err))
{
...
}
A few line breaks tidied up manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-19-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:45 +0000 (18:05 +0200)]
qapi: Make visitor functions taking Error ** return bool, not void
See recent commit "error: Document Error API usage rules" for
rationale.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-18-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:44 +0000 (18:05 +0200)]
hmp: Eliminate a variable in hmp_migrate_set_parameter()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-17-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:43 +0000 (18:05 +0200)]
block: Avoid error accumulation in bdrv_img_create()
When creating an image fails because the format doesn't support option
"backing_file" or "backing_fmt", bdrv_img_create() first has
qemu_opt_set() put a generic error into @local_err, then puts the real
error into @errp with error_setg(), and then propagates the former to
the latter, which throws away the generic error. A bit complicated,
but works.
Now that qemu_opt_set() returns a useful value, we can simply ignore
the generic error instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-16-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:42 +0000 (18:05 +0200)]
qemu-option: Use returned bool to check for failure
The previous commit enables conversion of
foo(..., &err);
if (err) {
...
}
to
if (!foo(..., &err)) {
...
}
for QemuOpts functions that now return true / false on success /
error. Coccinelle script:
@@
identifier fun = {
opts_do_parse, parse_option_bool, parse_option_number,
parse_option_size, qemu_opt_parse, qemu_opt_rename, qemu_opt_set,
qemu_opt_set_bool, qemu_opt_set_number, qemu_opts_absorb_qdict,
qemu_opts_do_parse, qemu_opts_from_qdict_entry, qemu_opts_set,
qemu_opts_validate
};
expression list args, args2;
typedef Error;
Error *err;
@@
- fun(args, &err, args2);
- if (err)
+ if (!fun(args, &err, args2))
{
...
}
A few line breaks tidied up manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-15-armbru@redhat.com>
[Conflict with commit
0b6786a9c1 "block/amend: refactor qcow2 amend
options" resolved by rerunning Coccinelle on master's version]
Markus Armbruster [Tue, 7 Jul 2020 16:05:41 +0000 (18:05 +0200)]
qemu-option: Make functions taking Error ** return bool, not void
See recent commit "error: Document Error API usage rules" for
rationale.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-14-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:40 +0000 (18:05 +0200)]
qemu-option: Replace opt_set() by cleaner opt_validate()
opt_set() frees its argument @value on failure. Slightly unclean;
functions ideally do nothing on failure.
To tidy this up, move opt_create() from opt_set() into its callers,
along with the cleanup. Rename opt_set() to opt_validate(), noting
its similarity to qemu_opts_validate(). Drop redundant parameter
@opts; use opt->opts instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-13-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:39 +0000 (18:05 +0200)]
qemu-option: Factor out helper opt_create()
There is just one use so far. The next commit will add more.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-12-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:38 +0000 (18:05 +0200)]
qemu-option: Simplify around find_default_by_name()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <
20200707160613.848843-11-armbru@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Markus Armbruster [Tue, 7 Jul 2020 16:05:37 +0000 (18:05 +0200)]
qemu-option: Factor out helper find_default_by_name()
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <
20200707160613.848843-10-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:36 +0000 (18:05 +0200)]
qemu-option: Make uses of find_desc_by_name() more similar
This is to make the next commit easier to review.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <
20200707160613.848843-9-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:35 +0000 (18:05 +0200)]
qemu-option: Check return value instead of @err where convenient
Convert uses like
opts = qemu_opts_create(..., &err);
if (err) {
...
}
to
opts = qemu_opts_create(..., errp);
if (!opts) {
...
}
Eliminate error_propagate() that are now unnecessary. Delete @err
that are now unused.
Note that we can't drop parallels_open()'s error_propagate() here. We
continue to execute it even in the converted case. It's a no-op then:
local_err is null.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <
20200707160613.848843-8-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:34 +0000 (18:05 +0200)]
virtio-crypto-pci: Tidy up virtio_crypto_pci_realize()
virtio_crypto_pci_realize() continues after realization of its
"virtio-crypto-device" fails. Only an object_property_set_link()
follows; looks harmless to me. Tidy up anyway: return after failure,
just like virtio_rng_pci_realize() does.
Cc: "Gonglei (Arei)" <arei.gonglei@huawei.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Gonglei < arei.gonglei@huawei.com>
Message-Id: <
20200707160613.848843-7-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:33 +0000 (18:05 +0200)]
macio: Tidy up error handling in macio_newworld_realize()
macio_newworld_realize() effectively ignores ns->gpio realization
errors, leaking the Error object. Fortunately, macio_gpio_realize()
can't actually fail. Tidy up.
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <
20200707160613.848843-6-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:32 +0000 (18:05 +0200)]
qdev: Use returned bool to check for qdev_realize() etc. failure
Convert
foo(..., &err);
if (err) {
...
}
to
if (!foo(..., &err)) {
...
}
for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their
wrappers isa_realize_and_unref(), pci_realize_and_unref(),
sysbus_realize(), sysbus_realize_and_unref(), usb_realize_and_unref().
Coccinelle script:
@@
identifier fun = {
isa_realize_and_unref, pci_realize_and_unref, qbus_realize,
qdev_realize, qdev_realize_and_unref, sysbus_realize,
sysbus_realize_and_unref, usb_realize_and_unref
};
expression list args, args2;
typedef Error;
Error *err;
@@
- fun(args, &err, args2);
- if (err)
+ if (!fun(args, &err, args2))
{
...
}
Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error
message "no position information". Nothing to convert there; skipped.
Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by
ARMSSE being used both as typedef and function-like macro there.
Converted manually.
A few line breaks tidied up manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <
20200707160613.848843-5-armbru@redhat.com>
Markus Armbruster [Tue, 7 Jul 2020 16:05:31 +0000 (18:05 +0200)]
error: Document Error API usage rules
This merely codifies existing practice, with one exception: the rule
advising against returning void, where existing practice is mixed.
When the Error API was created, we adopted the (unwritten) rule to
return void when the function returns no useful value on success,
unlike GError, which recommends to return true on success and false on
error then.
When a function returns a distinct error value, say false, a checked
call that passes the error up looks like
if (!frobnicate(..., errp)) {
handle the error...
}
When it returns void, we need
Error *err = NULL;
frobnicate(..., &err);
if (err) {
handle the error...
error_propagate(errp, err);
}
Not only is this more verbose, it also creates an Error object even
when @errp is null, &error_abort or &error_fatal.
People got tired of the additional boilerplate, and started to ignore
the unwritten rule. The result is confusion among developers about
the preferred usage.
Make the rule advising against returning void official by putting it
in writing. This will hopefully reduce confusion.
Update the examples accordingly.
The remainder of this series will update a substantial amount of code
to honor the rule.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <
20200707160613.848843-4-armbru@redhat.com>
[Tweak prose as per advice from Eric]
Markus Armbruster [Tue, 7 Jul 2020 16:05:30 +0000 (18:05 +0200)]
error: Improve error.h's big comment
Add headlines to the big comment.
Explain examples for NULL, &error_abort and &error_fatal argument
better.
Tweak rationale for error_propagate_prepend().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <
20200707160613.848843-3-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Markus Armbruster [Tue, 7 Jul 2020 16:05:29 +0000 (18:05 +0200)]
error: Fix examples in error.h's big comment
Mark a bad example more clearly. Fix the error_propagate_prepend()
example. Add a missing declaration and a second error pileup example.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <
20200707160613.848843-2-armbru@redhat.com>
Paul Durrant [Wed, 24 Jun 2020 12:18:41 +0000 (13:18 +0100)]
xen: cleanup unrealized flash devices
The generic pc_machine_initfn() calls pc_system_flash_create() which creates
'system.flash0' and 'system.flash1' devices. These devices are then realized
by pc_system_flash_map() which is called from pc_system_firmware_init() which
itself is called via pc_memory_init(). The latter however is not called when
xen_enable() is true and hence the following assertion fails:
qemu-system-i386: hw/core/qdev.c:439: qdev_assert_realized_properly:
Assertion `dev->realized' failed
These flash devices are unneeded when using Xen so this patch avoids the
assertion by simply removing them using pc_system_flash_cleanup_unused().
Reported-by: Jason Andryuk <jandryuk@gmail.com>
Fixes: ebc29e1beab0 ("pc: Support firmware configuration with -blockdev")
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <
20200624121841.17971-3-paul@xen.org>
Fixes: dfe8c79c4468 ("qdev: Assert onboard devices all get realized properly")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Jason Andryuk [Wed, 24 Jun 2020 12:19:39 +0000 (08:19 -0400)]
xen: Fix xen-legacy-backend qdev types
xen-sysdev is a TYPE_SYS_BUS_DEVICE. bus_type should not be changed so
that it can plug into the System bus. Otherwise this assert triggers:
qemu-system-i386: hw/core/qdev.c:102: qdev_set_parent_bus: Assertion
`dc->bus_type && object_dynamic_cast(OBJECT(bus), dc->bus_type)'
failed.
TYPE_XENBACKEND attaches to TYPE_XENSYSBUS, so its class_init needs to
be set accordingly to attach the qdev. Otherwise the following assert
triggers:
qemu-system-i386: hw/core/qdev.c:102: qdev_set_parent_bus: Assertion
`dc->bus_type && object_dynamic_cast(OBJECT(bus), dc->bus_type)'
failed.
TYPE_XENBACKEND is not a subclass of XEN_XENSYSDEV, so it's parent
is just TYPE_DEVICE. Change that.
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Paul Durrant <pdurrant@amazon.com>
Fixes: 81cb05732efb ("qdev: Assert devices are plugged into a bus that can take them")
Message-Id: <
20200624121939.10282-1-jandryuk@gmail.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Peter Maydell [Fri, 10 Jul 2020 08:01:28 +0000 (09:01 +0100)]
Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging
Pull request
Fix for a LTTng Userspace Tracer header problem.
# gpg: Signature made Tue 07 Jul 2020 16:10:04 BST
# gpg: using RSA key
8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full]
# gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" [full]
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35 775A 9CA4 ABB3 81AB 73C8
* remotes/stefanha/tags/tracing-pull-request:
tracetool: work around ust <sys/sdt.h> include conflict
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Thu, 9 Jul 2020 19:01:43 +0000 (20:01 +0100)]
Merge remote-tracking branch 'remotes/philmd-gitlab/tags/fw_cfg-
20200704' into staging
firmware (and crypto) patches
- add the tls-cipher-suites object,
- add the ability to QOM objects to produce data consumable
by the fw_cfg device,
- let the tls-cipher-suites object implement the
FW_CFG_DATA_GENERATOR interface.
This is required by EDK2 'HTTPS Boot' feature of OVMF to tell
the guest which TLS ciphers it can use.
CI jobs results:
https://travis-ci.org/github/philmd/qemu/builds/
704724619
https://gitlab.com/philmd/qemu/-/pipelines/
162938106
https://cirrus-ci.com/build/
4682977303068672
# gpg: Signature made Sat 04 Jul 2020 17:37:08 BST
# gpg: using RSA key
FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE
* remotes/philmd-gitlab/tags/fw_cfg-
20200704:
crypto/tls-cipher-suites: Produce fw_cfg consumable blob
softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace
softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface
crypto: Add tls-cipher-suites object
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Thu, 9 Jul 2020 16:02:29 +0000 (17:02 +0100)]
Merge remote-tracking branch 'remotes/kraxel/tags/modules-
20200707-pull-request' into staging
qom: add support for qom objects in modules.
build some devices (qxl, virtio-gpu, ccid, usb-redir) as modules.
build braille chardev as module.
v2: more verbose comment for "build: fix device module builds" patch.
note: qemu doesn't rebuild objects on cflags changes (specifically
-fPIC being added when code is switched from builtin to module).
Workaround for resulting build errors: "make clean", rebuild.
# gpg: Signature made Tue 07 Jul 2020 14:42:16 BST
# gpg: using RSA key
4CB6D8EED3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <kraxel@redhat.com>" [full]
# gpg: aka "Gerd Hoffmann <gerd@kraxel.org>" [full]
# gpg: aka "Gerd Hoffmann (private) <kraxel@gmail.com>" [full]
# Primary key fingerprint: A032 8CFF B93A 17A7 9901 FE7D 4CB6 D8EE D3E8 7138
* remotes/kraxel/tags/modules-
20200707-pull-request:
chardev: enable modules, use for braille
vga: build virtio-gpu as module
vga: build virtio-gpu only once
vga: build qxl as module
usb: build usb-redir as module
ccid: build smartcard as module
build: fix device module builds
qdev: device module support
object: qom module support
module: qom module support
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Thu, 9 Jul 2020 13:13:19 +0000 (14:13 +0100)]
Merge remote-tracking branch 'remotes/vivier2/tags/trivial-branch-for-5.1-pull-request' into staging
trivial branch patches
20200707
# gpg: Signature made Tue 07 Jul 2020 11:52:06 BST
# gpg: using RSA key
CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C
# gpg: issuer "laurent@vivier.eu"
# gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full]
# gpg: aka "Laurent Vivier <laurent@vivier.eu>" [full]
# gpg: aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full]
# Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F 5173 F30C 38BD 3F2F BE3C
* remotes/vivier2/tags/trivial-branch-for-5.1-pull-request:
net/tap-solaris.c: Include qemu-common.h for TFR macro
intel_iommu: "aw-bits" error message still refers to "x-aw-bits"
util/qemu-option: Document the get_opt_value() function
MAINTAINERS: Update Radoslaw Biernacki email address
.mailmap: Update Alexander Graf email address
trivial: Respect alphabetical order of .o files in Makefile.objs
fix the prototype of muls64/mulu64
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Wed, 8 Jul 2020 20:38:47 +0000 (21:38 +0100)]
Merge remote-tracking branch 'remotes/vivier/tags/m68k-next-pull-request' into staging
m68k pull-request
20200706
disable floatx80_invalid_encoding() for m68k
fix m68k_cpu_get_phys_page_debug()
# gpg: Signature made Mon 06 Jul 2020 21:05:33 BST
# gpg: using RSA key
CD2F75DDC8E3A4DC2E4F5173F30C38BD3F2FBE3C
# gpg: issuer "laurent@vivier.eu"
# gpg: Good signature from "Laurent Vivier <lvivier@redhat.com>" [full]
# gpg: aka "Laurent Vivier <laurent@vivier.eu>" [full]
# gpg: aka "Laurent Vivier (Red Hat) <lvivier@redhat.com>" [full]
# Primary key fingerprint: CD2F 75DD C8E3 A4DC 2E4F 5173 F30C 38BD 3F2F BE3C
* remotes/vivier/tags/m68k-next-pull-request:
softfloat,m68k: disable floatx80_invalid_encoding() for m68k
target/m68k: consolidate physical translation offset into get_physical_address()
target/m68k: fix physical address translation in m68k_cpu_get_phys_page_debug()
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>