From afc3a8f9f1df09c091f9903eaef82b35c152cacf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 28 Nov 2019 16:35:24 +0100 Subject: [PATCH 01/25] configure: allow disable of cross compilation containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our docker infrastructure isn't quite as multiarch as we would wish so lets allow the user to disable it if they want. This will allow us to use still run check-tcg on non-x86 CI setups. Signed-off-by: Alex Bennée Reviewed-by: Stefan Weil Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Tested-by: Richard Henderson --- configure | 8 +++++++- tests/tcg/configure.sh | 6 ++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 84b413dbfc..e0c66ee9b6 100755 --- a/configure +++ b/configure @@ -302,6 +302,7 @@ audio_win_int="" libs_qga="" debug_info="yes" stack_protector="" +use_containers="yes" if test -e "$source_path/.git" then @@ -1534,6 +1535,10 @@ for opt do ;; --disable-plugins) plugins="no" ;; + --enable-containers) use_containers="yes" + ;; + --disable-containers) use_containers="no" + ;; *) echo "ERROR: unknown option $opt" echo "Try '$0 --help' for more information" @@ -1717,6 +1722,7 @@ Advanced options (experts only): track the maximum stack usage of stacks created by qemu_alloc_stack --enable-plugins enable plugins via shared library loading + --disable-containers don't use containers for cross-building Optional features, enabled with --enable-FEATURE and disabled with --disable-FEATURE, default is enabled if available: @@ -7992,7 +7998,7 @@ done (for i in $cross_cc_vars; do export $i done -export target_list source_path +export target_list source_path use_containers $source_path/tests/tcg/configure.sh) # temporary config to build submodules diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh index 6c4a471aea..210e68396f 100755 --- a/tests/tcg/configure.sh +++ b/tests/tcg/configure.sh @@ -36,8 +36,10 @@ TMPC="${TMPDIR1}/qemu-conf.c" TMPE="${TMPDIR1}/qemu-conf.exe" container="no" -if has "docker" || has "podman"; then - container=$($python $source_path/tests/docker/docker.py probe) +if test $use_containers = "yes"; then + if has "docker" || has "podman"; then + container=$($python $source_path/tests/docker/docker.py probe) + fi fi # cross compilers defaults, can be overridden with --cross-cc-ARCH From 1e48931c0c0c29a30342614edb772fad8e4cff98 Mon Sep 17 00:00:00 2001 From: Wainer dos Santos Moschetta Date: Thu, 14 Nov 2019 08:42:46 -0500 Subject: [PATCH 02/25] tests/vm: Allow to set qemu-img path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit By default VM build test use qemu-img from system's PATH to create the image disk. Due the lack of qemu-img on the system or the desire to simply use a version built with QEMU, it would be nice to allow one to set its path. So this patch makes that possible by reading the path to qemu-img from QEMU_IMG if set, otherwise it fallback to default behavior. Signed-off-by: Wainer dos Santos Moschetta Message-Id: <20191114134246.12073-2-wainersm@redhat.com> Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé --- docs/devel/testing.rst | 6 ++++-- tests/vm/Makefile.include | 1 + tests/vm/basevm.py | 5 +++++ tests/vm/centos | 2 +- tests/vm/fedora | 4 +--- tests/vm/freebsd | 3 +-- tests/vm/netbsd | 3 +-- tests/vm/openbsd | 3 +-- tests/vm/ubuntu.i386 | 2 +- 9 files changed, 16 insertions(+), 13 deletions(-) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 27f286858a..ab5be0c729 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -418,13 +418,15 @@ access, so they SHOULD NOT be exposed to external interfaces if you are concerned about attackers taking control of the guest and potentially exploiting a QEMU security bug to compromise the host. -QEMU binary ------------ +QEMU binaries +------------- By default, qemu-system-x86_64 is searched in $PATH to run the guest. If there isn't one, or if it is older than 2.10, the test won't work. In this case, provide the QEMU binary in env var: ``QEMU=/path/to/qemu-2.10+``. +Likewise the path to qemu-img can be set in QEMU_IMG environment variable. + Make jobs --------- diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include index fea348e845..9e7c46a473 100644 --- a/tests/vm/Makefile.include +++ b/tests/vm/Makefile.include @@ -34,6 +34,7 @@ vm-help vm-test: @echo " DEBUG=1 - Enable verbose output on host and interactive debugging" @echo " V=1 - Enable verbose ouput on host and guest commands" @echo " QEMU=/path/to/qemu - Change path to QEMU binary" + @echo " QEMU_IMG=/path/to/qemu-img - Change path to qemu-img tool" vm-build-all: $(addprefix vm-build-, $(IMAGES)) diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 53b9515ee2..ed5dd4f3d0 100755 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -152,6 +152,11 @@ class BaseVM(object): def build_image(self, img): raise NotImplementedError + def exec_qemu_img(self, *args): + cmd = [os.environ.get("QEMU_IMG", "qemu-img")] + cmd.extend(list(args)) + subprocess.check_call(cmd) + def add_source_dir(self, src_dir): name = "data-" + hashlib.sha1(src_dir.encode("utf-8")).hexdigest()[:5] tarfile = os.path.join(self._tmpdir, name + ".tar") diff --git a/tests/vm/centos b/tests/vm/centos index b9e851f2d3..f2f0befd84 100755 --- a/tests/vm/centos +++ b/tests/vm/centos @@ -68,7 +68,7 @@ class CentosVM(basevm.BaseVM): sys.stderr.write("Extracting the image...\n") subprocess.check_call(["ln", "-f", cimg, img_tmp + ".xz"]) subprocess.check_call(["xz", "--keep", "-dvf", img_tmp + ".xz"]) - subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"]) + self.exec_qemu_img("resize", img_tmp, "50G") self.boot(img_tmp, extra_args = ["-cdrom", self._gen_cloud_init_iso()]) self.wait_ssh() self.ssh_root_check("touch /etc/cloud/cloud-init.disabled") diff --git a/tests/vm/fedora b/tests/vm/fedora index 7fec1479fb..8e270fc0f0 100755 --- a/tests/vm/fedora +++ b/tests/vm/fedora @@ -74,9 +74,7 @@ class FedoraVM(basevm.BaseVM): self.print_step("Preparing iso and disk image") subprocess.check_call(["cp", "-f", cimg, iso]) - subprocess.check_call(["qemu-img", "create", "-f", "qcow2", - img_tmp, self.size]) - + self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size) self.print_step("Booting installer") self.boot(img_tmp, extra_args = [ "-bios", "pc-bios/bios-256k.bin", diff --git a/tests/vm/freebsd b/tests/vm/freebsd index 2a19461a90..1825cc5821 100755 --- a/tests/vm/freebsd +++ b/tests/vm/freebsd @@ -82,8 +82,7 @@ class FreeBSDVM(basevm.BaseVM): self.print_step("Preparing iso and disk image") subprocess.check_call(["cp", "-f", cimg, iso_xz]) subprocess.check_call(["xz", "-dvf", iso_xz]) - subprocess.check_call(["qemu-img", "create", "-f", "qcow2", - img_tmp, self.size]) + self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size) self.print_step("Booting installer") self.boot(img_tmp, extra_args = [ diff --git a/tests/vm/netbsd b/tests/vm/netbsd index 611e6cc5b5..ec6f3563b2 100755 --- a/tests/vm/netbsd +++ b/tests/vm/netbsd @@ -77,8 +77,7 @@ class NetBSDVM(basevm.BaseVM): self.print_step("Preparing iso and disk image") subprocess.check_call(["ln", "-f", cimg, iso]) - subprocess.check_call(["qemu-img", "create", "-f", "qcow2", - img_tmp, self.size]) + self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size) self.print_step("Booting installer") self.boot(img_tmp, extra_args = [ diff --git a/tests/vm/openbsd b/tests/vm/openbsd index b92c39f89a..6df5162dbf 100755 --- a/tests/vm/openbsd +++ b/tests/vm/openbsd @@ -73,8 +73,7 @@ class OpenBSDVM(basevm.BaseVM): self.print_step("Preparing iso and disk image") subprocess.check_call(["cp", "-f", cimg, iso]) - subprocess.check_call(["qemu-img", "create", "-f", "qcow2", - img_tmp, self.size]) + self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size) self.print_step("Booting installer") self.boot(img_tmp, extra_args = [ diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386 index f611bebdc9..3834cd7a8d 100755 --- a/tests/vm/ubuntu.i386 +++ b/tests/vm/ubuntu.i386 @@ -70,7 +70,7 @@ class UbuntuX86VM(basevm.BaseVM): sha256sum="28969840626d1ea80bb249c08eef1a4533e8904aa51a327b40f37ac4b4ff04ef") img_tmp = img + ".tmp" subprocess.check_call(["cp", "-f", cimg, img_tmp]) - subprocess.check_call(["qemu-img", "resize", img_tmp, "50G"]) + self.exec_qemu_img("resize", img_tmp, "50G") self.boot(img_tmp, extra_args = ["-cdrom", self._gen_cloud_init_iso()]) self.wait_ssh() self.ssh_root_check("touch /etc/cloud/cloud-init.disabled") From 3edaa995e60d46d70fcef13f7d485a5037bd91f2 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 4 Dec 2019 09:31:33 +0100 Subject: [PATCH 03/25] travis.yml: Run tcg tests with tci MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far we only have compile coverage for tci. But since commit 2f160e0f9797c7522bfd0d09218d0c9340a5137c ("tci: Add implementation for INDEX_op_ld16u_i64") has been included now, we can also run the "tcg" and "qtest" tests with tci, so let's enable them in Travis now. Since we don't gain much additional test coverage by compiling all targets, and TCI is broken e.g. with the Sparc targets, we also limit the target list to a reasonable subset now (which should still get us test coverage by tests/boot-serial-test for example). Tested-by: Stefan Weil Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth Message-Id: <20191204083133.6198-1-thuth@redhat.com> [AJB: just --enable-debug-tcg] Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson --- .travis.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6cb8af6fa5..15946293ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -214,10 +214,11 @@ matrix: - TEST_CMD="" - # We manually include builds which we disable "make check" for + # Check the TCG interpreter (TCI) - env: - - CONFIG="--enable-debug --enable-tcg-interpreter" - - TEST_CMD="" + - CONFIG="--enable-debug-tcg --enable-tcg-interpreter --disable-kvm --disable-containers + --target-list=alpha-softmmu,arm-softmmu,hppa-softmmu,m68k-softmmu,microblaze-softmmu,moxie-softmmu,ppc-softmmu,s390x-softmmu,x86_64-softmmu" + - TEST_CMD="make check-qtest check-tcg V=1" # We don't need to exercise every backend with every front-end From 30729ae93b7e123e472a2d42792134ae39bf9df0 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 4 Dec 2019 16:46:12 +0100 Subject: [PATCH 04/25] iotests: Provide a function for checking the creation of huge files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some tests create huge (but sparse) files, and to be able to run those tests in certain limited environments (like CI containers), we have to check for the possibility to create such files first. Thus let's introduce a common function to check for large files, and replace the already existing checks in the iotests 005 and 220 with this function. Reviewed-by: Alex Bennée Signed-off-by: Thomas Huth Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20191204154618.23560-2-thuth@redhat.com> Signed-off-by: Alex Bennée --- tests/qemu-iotests/005 | 5 +---- tests/qemu-iotests/220 | 6 ++---- tests/qemu-iotests/common.rc | 10 ++++++++++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005 index 58442762fe..b6d03ac37d 100755 --- a/tests/qemu-iotests/005 +++ b/tests/qemu-iotests/005 @@ -59,10 +59,7 @@ fi # Sanity check: For raw, we require a file system that permits the creation # of a HUGE (but very sparse) file. Check we can create it before continuing. if [ "$IMGFMT" = "raw" ]; then - if ! truncate --size=5T "$TEST_IMG"; then - _notrun "file system on $TEST_DIR does not support large enough files" - fi - rm "$TEST_IMG" + _require_large_file 5T fi echo diff --git a/tests/qemu-iotests/220 b/tests/qemu-iotests/220 index 2d62c5dcac..15159270d3 100755 --- a/tests/qemu-iotests/220 +++ b/tests/qemu-iotests/220 @@ -42,10 +42,8 @@ echo "== Creating huge file ==" # Sanity check: We require a file system that permits the creation # of a HUGE (but very sparse) file. tmpfs works, ext4 does not. -if ! truncate --size=513T "$TEST_IMG"; then - _notrun "file system on $TEST_DIR does not support large enough files" -fi -rm "$TEST_IMG" +_require_large_file 513T + IMGOPTS='cluster_size=2M,refcount_bits=1' _make_test_img 513T echo "== Populating refcounts ==" diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 0cc8acc9ed..6f0582c79a 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -643,5 +643,15 @@ _require_drivers() done } +# Check that we have a file system that allows huge (but very sparse) files +# +_require_large_file() +{ + if ! truncate --size="$1" "$TEST_IMG"; then + _notrun "file system on $TEST_DIR does not support large enough files" + fi + rm "$TEST_IMG" +} + # make sure this script returns success true From 24eba765192c968fcafab86ad9ce2da124357376 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 4 Dec 2019 16:46:13 +0100 Subject: [PATCH 05/25] iotests: Skip test 060 if it is not possible to create large files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test 060 fails in the arm64, s390x and ppc64le LXD containers on Travis (which we will hopefully enable in our CI soon). These containers apparently do not allow large files to be created. The repair process in test 060 creates a file of 64 GiB, so test first whether such large files are possible and skip the test if that's not the case. Signed-off-by: Thomas Huth Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20191204154618.23560-3-thuth@redhat.com> Signed-off-by: Alex Bennée --- tests/qemu-iotests/060 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index b91d8321bb..d96f17a484 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -49,6 +49,9 @@ _supported_fmt qcow2 _supported_proto file _supported_os Linux +# The repair process will create a large file - so check for availability first +_require_large_file 64G + rt_offset=65536 # 0x10000 (XXX: just an assumption) rb_offset=131072 # 0x20000 (XXX: just an assumption) l1_offset=196608 # 0x30000 (XXX: just an assumption) From 178d383f10e15f5e5a7e7c17992a2dbb88a174f1 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 4 Dec 2019 16:46:14 +0100 Subject: [PATCH 06/25] iotests: Skip test 079 if it is not possible to create large files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test 079 fails in the arm64, s390x and ppc64le LXD containers on Travis (which we will hopefully enable in our CI soon). These containers apparently do not allow large files to be created. Test 079 tries to create a 4G sparse file, which is apparently already too big for these containers, so check first whether we can really create such files before executing the test. Signed-off-by: Thomas Huth Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20191204154618.23560-4-thuth@redhat.com> --- tests/qemu-iotests/079 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079 index 81f0c21f53..78536d3bbf 100755 --- a/tests/qemu-iotests/079 +++ b/tests/qemu-iotests/079 @@ -39,6 +39,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto file nfs +# Some containers (e.g. non-x86 on Travis) do not allow large files +_require_large_file 4G + echo "=== Check option preallocation and cluster_size ===" echo cluster_sizes="16384 32768 65536 131072 262144 524288 1048576 2097152 4194304" From a0d6d7454adb7255a4be33ae0d6a0ca81991273f Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 4 Dec 2019 16:46:15 +0100 Subject: [PATCH 07/25] tests/hd-geo-test: Skip test when images can not be created MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In certain environments like restricted containers, we can not create huge test images. To be able to use "make check" in such container environments, too, let's skip the hd-geo-test instead of failing when the test images could not be created. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alex Bennée Signed-off-by: Thomas Huth Signed-off-by: Alex Bennée Message-Id: <20191204154618.23560-5-thuth@redhat.com> --- tests/hd-geo-test.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c index 7e86c5416c..a249800544 100644 --- a/tests/hd-geo-test.c +++ b/tests/hd-geo-test.c @@ -34,8 +34,13 @@ static char *create_test_img(int secs) fd = mkstemp(template); g_assert(fd >= 0); ret = ftruncate(fd, (off_t)secs * 512); - g_assert(ret == 0); close(fd); + + if (ret) { + free(template); + template = NULL; + } + return template; } @@ -934,6 +939,10 @@ int main(int argc, char **argv) for (i = 0; i < backend_last; i++) { if (img_secs[i] >= 0) { img_file_name[i] = create_test_img(img_secs[i]); + if (!img_file_name[i]) { + g_test_message("Could not create test images."); + goto test_add_done; + } } else { img_file_name[i] = NULL; } @@ -965,6 +974,7 @@ int main(int argc, char **argv) "skipping hd-geo/override/* tests"); } +test_add_done: ret = g_test_run(); for (i = 0; i < backend_last; i++) { From 4f370b109876e0482d46dbe969f093d7ccf6269e Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 4 Dec 2019 16:46:16 +0100 Subject: [PATCH 08/25] tests/test-util-filemonitor: Skip test on non-x86 Travis containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test-util-filemonitor fails in restricted non-x86 Travis containers since they apparently blacklisted some required system calls there. Let's simply skip the test if we detect such an environment. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alex Bennée Signed-off-by: Thomas Huth Signed-off-by: Alex Bennée Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Message-Id: <20191204154618.23560-6-thuth@redhat.com> --- tests/test-util-filemonitor.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c index 301cd2db61..45009c69f4 100644 --- a/tests/test-util-filemonitor.c +++ b/tests/test-util-filemonitor.c @@ -406,10 +406,21 @@ test_file_monitor_events(void) char *pathdst = NULL; QFileMonitorTestData data; GHashTable *ids = g_hash_table_new(g_int64_hash, g_int64_equal); + char *travis_arch; qemu_mutex_init(&data.lock); data.records = NULL; + /* + * This test does not work on Travis LXD containers since some + * syscalls are blocked in that environment. + */ + travis_arch = getenv("TRAVIS_ARCH"); + if (travis_arch && !g_str_equal(travis_arch, "x86_64")) { + g_test_skip("Test does not work on non-x86 Travis containers."); + return; + } + /* * The file monitor needs the main loop running in * order to receive events from inotify. We must From 9c5941a96a55cb945b664961024070149ed76465 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 4 Dec 2019 16:46:18 +0100 Subject: [PATCH 09/25] travis.yml: Enable builds on arm64, ppc64le and s390x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Travis recently added the possibility to test on these architectures, too, so let's enable them in our travis.yml file to extend our test coverage. Unfortunately, the libssh in this Ubuntu version (bionic) is in a pretty unusable Frankenstein state and libspice-server-dev is not available here, so we can not use the global list of packages to install, but have to provide individual package lists instead. Also, some of the iotests crash when using "dist: bionic" on arm64 and ppc64le, thus these two builders have to use "dist: xenial" until the problem is understood / fixed. Signed-off-by: Thomas Huth Acked-by: David Gibson Message-Id: <20191204154618.23560-8-thuth@redhat.com> Signed-off-by: Alex Bennée --- .travis.yml | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/.travis.yml b/.travis.yml index 15946293ff..b68566b1fe 100644 --- a/.travis.yml +++ b/.travis.yml @@ -354,6 +354,92 @@ matrix: - TEST_CMD="make -j3 check-tcg V=1" - CACHE_NAME="${TRAVIS_BRANCH}-linux-gcc-debug-tcg" + - arch: arm64 + dist: xenial + addons: + apt_packages: + - libaio-dev + - libattr1-dev + - libbrlapi-dev + - libcap-ng-dev + - libgcrypt20-dev + - libgnutls28-dev + - libgtk-3-dev + - libiscsi-dev + - liblttng-ust-dev + - libncurses5-dev + - libnfs-dev + - libnss3-dev + - libpixman-1-dev + - libpng-dev + - librados-dev + - libsdl2-dev + - libseccomp-dev + - liburcu-dev + - libusb-1.0-0-dev + - libvdeplug-dev + - libvte-2.91-dev + env: + - TEST_CMD="make check check-tcg V=1" + - CONFIG="--disable-containers --target-list=${MAIN_SOFTMMU_TARGETS}" + + - arch: ppc64le + dist: xenial + addons: + apt_packages: + - libaio-dev + - libattr1-dev + - libbrlapi-dev + - libcap-ng-dev + - libgcrypt20-dev + - libgnutls28-dev + - libgtk-3-dev + - libiscsi-dev + - liblttng-ust-dev + - libncurses5-dev + - libnfs-dev + - libnss3-dev + - libpixman-1-dev + - libpng-dev + - librados-dev + - libsdl2-dev + - libseccomp-dev + - liburcu-dev + - libusb-1.0-0-dev + - libvdeplug-dev + - libvte-2.91-dev + env: + - TEST_CMD="make check check-tcg V=1" + - CONFIG="--disable-containers --target-list=${MAIN_SOFTMMU_TARGETS},ppc64le-linux-user" + + - arch: s390x + dist: bionic + addons: + apt_packages: + - libaio-dev + - libattr1-dev + - libbrlapi-dev + - libcap-ng-dev + - libgcrypt20-dev + - libgnutls28-dev + - libgtk-3-dev + - libiscsi-dev + - liblttng-ust-dev + - libncurses5-dev + - libnfs-dev + - libnss3-dev + - libpixman-1-dev + - libpng-dev + - librados-dev + - libsdl2-dev + - libseccomp-dev + - liburcu-dev + - libusb-1.0-0-dev + - libvdeplug-dev + - libvte-2.91-dev + env: + - TEST_CMD="make check check-tcg V=1" + - CONFIG="--disable-containers --target-list=${MAIN_SOFTMMU_TARGETS},s390x-linux-user" # Release builds # The make-release script expect a QEMU version, so our tag must start with a 'v'. From bc4486fb233573e77b6e9ad6d6379afb5e37ad8c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 11 Dec 2019 15:33:49 +0100 Subject: [PATCH 10/25] ci: build out-of-tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most developers are using out-of-tree builds and it was discussed in the past to only allow those. To prepare for the transition, use out-of-tree builds in all continuous integration jobs. Based on a patch by Marc-André Lureau. Signed-off-by: Paolo Bonzini Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth Reviewed-by: Li-Wen Hsu Message-Id: <1576074829-56711-1-git-send-email-pbonzini@redhat.com> --- .cirrus.yml | 8 ++++++-- .gitlab-ci.yml | 28 +++++++++++++++++++++------- .shippable.yml | 4 +++- .travis.yml | 13 ++++++++----- configure | 1 + 5 files changed, 39 insertions(+), 15 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 27efc48619..90645fede6 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -22,7 +22,9 @@ macos_task: install_script: - brew install pkg-config python gnu-sed glib pixman make sdl2 script: - - ./configure --python=/usr/local/bin/python3 || { cat config.log; exit 1; } + - mkdir build + - cd build + - ../configure --python=/usr/local/bin/python3 || { cat config.log; exit 1; } - gmake -j$(sysctl -n hw.ncpu) - gmake check -j$(sysctl -n hw.ncpu) @@ -33,6 +35,8 @@ macos_xcode_task: install_script: - brew install pkg-config gnu-sed glib pixman make sdl2 script: - - ./configure --cc=clang || { cat config.log; exit 1; } + - mkdir build + - cd build + - ../configure --cc=clang || { cat config.log; exit 1; } - gmake -j$(sysctl -n hw.ncpu) - gmake check -j$(sysctl -n hw.ncpu) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e3db1b847c..ebcef0ebe9 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -6,7 +6,9 @@ build-system1: script: - apt-get install -y -qq libgtk-3-dev libvte-dev nettle-dev libcacard-dev libusb-dev libvde-dev libspice-protocol-dev libgl1-mesa-dev libvdeplug-dev - - ./configure --enable-werror --target-list="aarch64-softmmu alpha-softmmu + - mkdir build + - cd build + - ../configure --enable-werror --target-list="aarch64-softmmu alpha-softmmu cris-softmmu hppa-softmmu lm32-softmmu moxie-softmmu microblazeel-softmmu mips64el-softmmu m68k-softmmu ppc-softmmu riscv64-softmmu sparc-softmmu" - make -j2 @@ -16,7 +18,9 @@ build-system2: script: - apt-get install -y -qq libsdl2-dev libgcrypt-dev libbrlapi-dev libaio-dev libfdt-dev liblzo2-dev librdmacm-dev libibverbs-dev libibumad-dev - - ./configure --enable-werror --target-list="tricore-softmmu unicore32-softmmu + - mkdir build + - cd build + - ../configure --enable-werror --target-list="tricore-softmmu unicore32-softmmu microblaze-softmmu mips-softmmu riscv32-softmmu s390x-softmmu sh4-softmmu sparc64-softmmu x86_64-softmmu xtensa-softmmu nios2-softmmu or1k-softmmu" - make -j2 @@ -24,7 +28,9 @@ build-system2: build-disabled: script: - - ./configure --enable-werror --disable-rdma --disable-slirp --disable-curl + - mkdir build + - cd build + - ../configure --enable-werror --disable-rdma --disable-slirp --disable-curl --disable-capstone --disable-live-block-migration --disable-glusterfs --disable-replication --disable-coroutine-pool --disable-smartcard --disable-guest-agent --disable-curses --disable-libxml2 --disable-tpm @@ -37,7 +43,9 @@ build-disabled: build-tcg-disabled: script: - apt-get install -y -qq clang libgtk-3-dev libusb-dev - - ./configure --cc=clang --enable-werror --disable-tcg --audio-drv-list="" + - mkdir build + - cd build + - ../configure --cc=clang --enable-werror --disable-tcg --audio-drv-list="" - make -j2 - make check-unit - make check-qapi-schema @@ -52,7 +60,9 @@ build-tcg-disabled: build-user: script: - - ./configure --enable-werror --disable-system --disable-guest-agent + - mkdir build + - cd build + - ../configure --enable-werror --disable-system --disable-guest-agent --disable-capstone --disable-slirp --disable-fdt - make -j2 - make run-tcg-tests-i386-linux-user run-tcg-tests-x86_64-linux-user @@ -61,7 +71,9 @@ build-clang: script: - apt-get install -y -qq clang libsdl2-dev libattr1-dev libcap-ng-dev xfslibs-dev libiscsi-dev libnfs-dev libseccomp-dev gnutls-dev librbd-dev - - ./configure --cc=clang --cxx=clang++ --enable-werror + - mkdir build + - cd build + - ../configure --cc=clang --cxx=clang++ --enable-werror --target-list="alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu ppc-softmmu s390x-softmmu x86_64-softmmu arm-linux-user" - make -j2 @@ -70,7 +82,9 @@ build-clang: build-tci: script: - TARGETS="aarch64 alpha arm hppa m68k microblaze moxie ppc64 s390x x86_64" - - ./configure --enable-tcg-interpreter + - mkdir build + - cd build + - ../configure --enable-tcg-interpreter --target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu '; done)" - make -j2 - make tests/boot-serial-test tests/cdrom-test tests/pxe-test diff --git a/.shippable.yml b/.shippable.yml index f74a3de3ff..83aae08bb4 100644 --- a/.shippable.yml +++ b/.shippable.yml @@ -35,5 +35,7 @@ build: options: "-e HOME=/root" ci: - unset CC - - ./configure ${QEMU_CONFIGURE_OPTS} --target-list=${TARGET_LIST} + - mkdir build + - cd build + - ../configure ${QEMU_CONFIGURE_OPTS} --target-list=${TARGET_LIST} - make -j$(($(getconf _NPROCESSORS_ONLN) + 1)) diff --git a/.travis.yml b/.travis.yml index b68566b1fe..d673ee219e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -73,8 +73,8 @@ notifications: env: global: - - SRC_DIR="." - - BUILD_DIR="." + - SRC_DIR=".." + - BUILD_DIR="build" - BASE_CONFIG="--disable-docs --disable-tools" - TEST_CMD="make check V=1" # This is broadly a list of "mainline" softmmu targets which have support across the major distros @@ -191,7 +191,8 @@ matrix: - CACHE_NAME="${TRAVIS_BRANCH}-linux-clang-sanitize" compiler: clang before_script: - - ./configure ${CONFIG} --extra-cflags="-fsanitize=undefined -Werror" || { cat config.log && exit 1; } + - mkdir -p ${BUILD_DIR} && cd ${BUILD_DIR} + - ${SRC_DIR}/configure ${CONFIG} --extra-cflags="-fsanitize=undefined -Werror" || { cat config.log && exit 1; } - env: @@ -323,7 +324,8 @@ matrix: - CONFIG="--cc=gcc-9 --cxx=g++-9 --disable-pie --disable-linux-user" - TEST_CMD="" before_script: - - ./configure ${CONFIG} --extra-cflags="-g3 -O0 -Wno-error=stringop-truncation -fsanitize=thread -fuse-ld=gold" || { cat config.log && exit 1; } + - mkdir -p ${BUILD_DIR} && cd ${BUILD_DIR} + - ${SRC_DIR}/configure ${CONFIG} --extra-cflags="-g3 -O0 -Wno-error=stringop-truncation -fsanitize=thread -fuse-ld=gold" || { cat config.log && exit 1; } # Run check-tcg against linux-user @@ -460,5 +462,6 @@ matrix: - make -C ${SRC_DIR} qemu-${QEMU_VERSION}.tar.bz2 - ls -l ${SRC_DIR}/qemu-${QEMU_VERSION}.tar.bz2 - tar -xf ${SRC_DIR}/qemu-${QEMU_VERSION}.tar.bz2 && cd qemu-${QEMU_VERSION} - - ./configure ${BASE_CONFIG} ${CONFIG} || { cat config.log && exit 1; } + - mkdir -p release-build && cd release-build + - ../configure ${BASE_CONFIG} ${CONFIG} || { cat config.log && exit 1; } - make install diff --git a/configure b/configure index e0c66ee9b6..93625811a0 100755 --- a/configure +++ b/configure @@ -6401,6 +6401,7 @@ else echo "local state directory queried at runtime" echo "Windows SDK $win_sdk" fi +echo "Build directory $(pwd)" echo "Source path $source_path" echo "GIT binary $git" echo "GIT submodules $git_submodules" From 0f516ca4767042aec8716369d6d62436fa10593a Mon Sep 17 00:00:00 2001 From: Robert Foley Date: Mon, 18 Nov 2019 16:15:23 -0500 Subject: [PATCH 11/25] Fix double free issue in qemu_set_log_filename(). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After freeing the logfilename, we set logfilename to NULL, in case of an error which returns without setting logfilename. Signed-off-by: Robert Foley Reviewed-by: Alex Bennée Signed-off-by: Alex Bennée Message-Id: <20191118211528.3221-2-robert.foley@linaro.org> --- util/log.c | 1 + 1 file changed, 1 insertion(+) diff --git a/util/log.c b/util/log.c index 1ca13059ee..4316fe74ee 100644 --- a/util/log.c +++ b/util/log.c @@ -113,6 +113,7 @@ void qemu_set_log_filename(const char *filename, Error **errp) { char *pidstr; g_free(logfilename); + logfilename = NULL; pidstr = strstr(filename, "%"); if (pidstr) { From 045e8861df8ec0922b1c47929d23dd61d8b75f8c Mon Sep 17 00:00:00 2001 From: Robert Foley Date: Mon, 18 Nov 2019 16:15:24 -0500 Subject: [PATCH 12/25] Cleaned up flow of code in qemu_set_log(), to simplify and clarify. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also added some explanation of the reasoning behind the branches. Signed-off-by: Robert Foley Reviewed-by: Alex Bennée Signed-off-by: Alex Bennée Message-Id: <20191118211528.3221-3-robert.foley@linaro.org> --- util/log.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/util/log.c b/util/log.c index 4316fe74ee..417d16ec66 100644 --- a/util/log.c +++ b/util/log.c @@ -54,12 +54,25 @@ static bool log_uses_own_buffers; /* enable or disable low levels log */ void qemu_set_log(int log_flags) { + bool need_to_open_file = false; qemu_loglevel = log_flags; #ifdef CONFIG_TRACE_LOG qemu_loglevel |= LOG_TRACE; #endif - if (!qemu_logfile && - (is_daemonized() ? logfilename != NULL : qemu_loglevel)) { + /* + * In all cases we only log if qemu_loglevel is set. + * Also: + * If not daemonized we will always log either to stderr + * or to a file (if there is a logfilename). + * If we are daemonized, + * we will only log if there is a logfilename. + */ + if (qemu_loglevel && (!is_daemonized() || logfilename)) { + need_to_open_file = true; + } + if (qemu_logfile && !need_to_open_file) { + qemu_log_close(); + } else if (!qemu_logfile && need_to_open_file) { if (logfilename) { qemu_logfile = fopen(logfilename, log_append ? "a" : "w"); if (!qemu_logfile) { @@ -93,10 +106,6 @@ void qemu_set_log(int log_flags) log_append = 1; } } - if (qemu_logfile && - (is_daemonized() ? logfilename == NULL : !qemu_loglevel)) { - qemu_log_close(); - } } void qemu_log_needs_buffers(void) From b8121fe7c005dc3d56e82a0a5ce00563a5fd49d0 Mon Sep 17 00:00:00 2001 From: Robert Foley Date: Mon, 18 Nov 2019 16:15:25 -0500 Subject: [PATCH 13/25] Add a mutex to guarantee single writer to qemu_logfile handle. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also added qemu_logfile_init() for initializing the logfile mutex. Note that inside qemu_set_log() we needed to add a pair of qemu_mutex_unlock() calls in order to avoid a double lock in qemu_log_close(). This unavoidable temporary ugliness will be cleaned up in a later patch in this series. Signed-off-by: Robert Foley Reviewed-by: Alex Bennée Signed-off-by: Alex Bennée Message-Id: <20191118211528.3221-4-robert.foley@linaro.org> --- util/log.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/util/log.c b/util/log.c index 417d16ec66..953a66b5a8 100644 --- a/util/log.c +++ b/util/log.c @@ -24,8 +24,10 @@ #include "qapi/error.h" #include "qemu/cutils.h" #include "trace/control.h" +#include "qemu/thread.h" static char *logfilename; +static QemuMutex qemu_logfile_mutex; FILE *qemu_logfile; int qemu_loglevel; static int log_append = 0; @@ -49,6 +51,11 @@ int qemu_log(const char *fmt, ...) return ret; } +static void __attribute__((__constructor__)) qemu_logfile_init(void) +{ + qemu_mutex_init(&qemu_logfile_mutex); +} + static bool log_uses_own_buffers; /* enable or disable low levels log */ @@ -70,7 +77,9 @@ void qemu_set_log(int log_flags) if (qemu_loglevel && (!is_daemonized() || logfilename)) { need_to_open_file = true; } + qemu_mutex_lock(&qemu_logfile_mutex); if (qemu_logfile && !need_to_open_file) { + qemu_mutex_unlock(&qemu_logfile_mutex); qemu_log_close(); } else if (!qemu_logfile && need_to_open_file) { if (logfilename) { @@ -105,6 +114,7 @@ void qemu_set_log(int log_flags) #endif log_append = 1; } + qemu_mutex_unlock(&qemu_logfile_mutex); } } @@ -240,12 +250,14 @@ void qemu_log_flush(void) /* Close the log file */ void qemu_log_close(void) { + qemu_mutex_lock(&qemu_logfile_mutex); if (qemu_logfile) { if (qemu_logfile != stderr) { fclose(qemu_logfile); } qemu_logfile = NULL; } + qemu_mutex_unlock(&qemu_logfile_mutex); } const QEMULogItem qemu_log_items[] = { From fc59d2d870caddf5cd9c85836ee4a8c59ffe7617 Mon Sep 17 00:00:00 2001 From: Robert Foley Date: Mon, 18 Nov 2019 16:15:26 -0500 Subject: [PATCH 14/25] qemu_log_lock/unlock now preserves the qemu_logfile handle. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qemu_log_lock() now returns a handle and qemu_log_unlock() receives a handle to unlock. This allows for changing the handle during logging and ensures the lock() and unlock() are for the same file. Also in target/tilegx/translate.c removed the qemu_log_lock()/unlock() calls (and the log("\n")), since the translator can longjmp out of the loop if it attempts to translate an instruction in an inaccessible page. Signed-off-by: Robert Foley Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson Signed-off-by: Alex Bennée Message-Id: <20191118211528.3221-5-robert.foley@linaro.org> --- accel/tcg/cpu-exec.c | 4 ++-- accel/tcg/translate-all.c | 4 ++-- accel/tcg/translator.c | 4 ++-- exec.c | 4 ++-- hw/net/can/can_sja1000.c | 4 ++-- include/qemu/log.h | 9 ++++++--- net/can/can_socketcan.c | 5 ++--- target/cris/translate.c | 4 ++-- target/i386/translate.c | 5 +++-- target/lm32/translate.c | 4 ++-- target/microblaze/translate.c | 4 ++-- target/nios2/translate.c | 4 ++-- target/tilegx/translate.c | 6 ------ target/unicore32/translate.c | 4 ++-- tcg/tcg.c | 16 ++++++++-------- 15 files changed, 39 insertions(+), 42 deletions(-) diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index c01f59c743..62068d10c3 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -156,7 +156,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb) #if defined(DEBUG_DISAS) if (qemu_loglevel_mask(CPU_LOG_TB_CPU) && qemu_log_in_addr_range(itb->pc)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); int flags = 0; if (qemu_loglevel_mask(CPU_LOG_TB_FPU)) { flags |= CPU_DUMP_FPU; @@ -165,7 +165,7 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb) flags |= CPU_DUMP_CCOP; #endif log_cpu_state(cpu, flags); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif /* DEBUG_DISAS */ diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 9f48da9472..bb325a2bc4 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -1804,7 +1804,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, #ifdef DEBUG_DISAS if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM) && qemu_log_in_addr_range(tb->pc)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("OUT: [size=%d]\n", gen_code_size); if (tcg_ctx->data_gen_ptr) { size_t code_size = tcg_ctx->data_gen_ptr - tb->tc.ptr; @@ -1829,7 +1829,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu, } qemu_log("\n"); qemu_log_flush(); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index f977682be7..603d17ff83 100644 --- a/accel/tcg/translator.c +++ b/accel/tcg/translator.c @@ -138,11 +138,11 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, #ifdef DEBUG_DISAS if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) && qemu_log_in_addr_range(db->pc_first)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("----------------\n"); ops->disas_log(db, cpu); qemu_log("\n"); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif } diff --git a/exec.c b/exec.c index a34c348184..edafdebeec 100644 --- a/exec.c +++ b/exec.c @@ -1225,13 +1225,13 @@ void cpu_abort(CPUState *cpu, const char *fmt, ...) fprintf(stderr, "\n"); cpu_dump_state(cpu, stderr, CPU_DUMP_FPU | CPU_DUMP_CCOP); if (qemu_log_separate()) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("qemu: fatal: "); qemu_log_vprintf(fmt, ap2); qemu_log("\n"); log_cpu_state(cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP); qemu_log_flush(); - qemu_log_unlock(); + qemu_log_unlock(logfile); qemu_log_close(); } va_end(ap2); diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c index 1f81341554..39c78faf9b 100644 --- a/hw/net/can/can_sja1000.c +++ b/hw/net/can/can_sja1000.c @@ -247,8 +247,8 @@ int can_sja_accept_filter(CanSJA1000State *s, static void can_display_msg(const char *prefix, const qemu_can_frame *msg) { int i; + FILE *logfile = qemu_log_lock(); - qemu_log_lock(); qemu_log("%s%03X [%01d] %s %s", prefix, msg->can_id & QEMU_CAN_EFF_MASK, @@ -261,7 +261,7 @@ static void can_display_msg(const char *prefix, const qemu_can_frame *msg) } qemu_log("\n"); qemu_log_flush(); - qemu_log_unlock(); + qemu_log_unlock(logfile); } static void buff2frame_pel(const uint8_t *buff, qemu_can_frame *frame) diff --git a/include/qemu/log.h b/include/qemu/log.h index a91105b2ad..a7c5b01571 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -53,14 +53,17 @@ static inline bool qemu_log_separate(void) * qemu_loglevel is never set when qemu_logfile is unset. */ -static inline void qemu_log_lock(void) +static inline FILE *qemu_log_lock(void) { qemu_flockfile(qemu_logfile); + return logfile->fd; } -static inline void qemu_log_unlock(void) +static inline void qemu_log_unlock(FILE *fd) { - qemu_funlockfile(qemu_logfile); + if (fd) { + qemu_funlockfile(fd); + } } /* Logging functions: */ diff --git a/net/can/can_socketcan.c b/net/can/can_socketcan.c index 8a6ffad40c..29bfacd4f8 100644 --- a/net/can/can_socketcan.c +++ b/net/can/can_socketcan.c @@ -76,8 +76,7 @@ QEMU_BUILD_BUG_ON(offsetof(qemu_can_frame, data) static void can_host_socketcan_display_msg(struct qemu_can_frame *msg) { int i; - - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("[cansocketcan]: %03X [%01d] %s %s", msg->can_id & QEMU_CAN_EFF_MASK, msg->can_dlc, @@ -89,7 +88,7 @@ static void can_host_socketcan_display_msg(struct qemu_can_frame *msg) } qemu_log("\n"); qemu_log_flush(); - qemu_log_unlock(); + qemu_log_unlock(logfile); } static void can_host_socketcan_read(void *opaque) diff --git a/target/cris/translate.c b/target/cris/translate.c index e752bd0609..cb57516a44 100644 --- a/target/cris/translate.c +++ b/target/cris/translate.c @@ -3273,11 +3273,11 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) #if !DISAS_CRIS if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) && qemu_log_in_addr_range(pc_start)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("--------------\n"); qemu_log("IN: %s\n", lookup_symbol(pc_start)); log_target_disas(cs, pc_start, dc->pc - pc_start); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif #endif diff --git a/target/i386/translate.c b/target/i386/translate.c index 77e932d827..7c99ef1385 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -2502,14 +2502,15 @@ static void gen_unknown_opcode(CPUX86State *env, DisasContext *s) gen_illegal_opcode(s); if (qemu_loglevel_mask(LOG_UNIMP)) { + FILE *logfile = qemu_log_lock(); target_ulong pc = s->pc_start, end = s->pc; - qemu_log_lock(); + qemu_log("ILLOPC: " TARGET_FMT_lx ":", pc); for (; pc < end; ++pc) { qemu_log(" %02x", cpu_ldub_code(env, pc)); } qemu_log("\n"); - qemu_log_unlock(); + qemu_log_unlock(logfile); } } diff --git a/target/lm32/translate.c b/target/lm32/translate.c index 778cae1e81..73db9654d6 100644 --- a/target/lm32/translate.c +++ b/target/lm32/translate.c @@ -1137,10 +1137,10 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) #ifdef DEBUG_DISAS if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) && qemu_log_in_addr_range(pc_start)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("\n"); log_target_disas(cs, pc_start, dc->pc - pc_start); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif } diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c index bdc7d5326a..525115b041 100644 --- a/target/microblaze/translate.c +++ b/target/microblaze/translate.c @@ -1765,10 +1765,10 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) #if !SIM_COMPAT if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) && qemu_log_in_addr_range(pc_start)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("--------------\n"); log_target_disas(cs, pc_start, dc->pc - pc_start); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif #endif diff --git a/target/nios2/translate.c b/target/nios2/translate.c index e17656e66f..82107bf270 100644 --- a/target/nios2/translate.c +++ b/target/nios2/translate.c @@ -892,11 +892,11 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) #ifdef DEBUG_DISAS if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) && qemu_log_in_addr_range(tb->pc)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("IN: %s\n", lookup_symbol(tb->pc)); log_target_disas(cs, tb->pc, dc->pc - tb->pc); qemu_log("\n"); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif } diff --git a/target/tilegx/translate.c b/target/tilegx/translate.c index 68dd4aa2d8..abce7e1c75 100644 --- a/target/tilegx/translate.c +++ b/target/tilegx/translate.c @@ -2388,7 +2388,6 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) dc->zero = NULL; if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) { - qemu_log_lock(); qemu_log("IN: %s\n", lookup_symbol(pc_start)); } gen_tb_start(tb); @@ -2417,11 +2416,6 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb, int max_insns) gen_tb_end(tb, num_insns); tb->size = dc->pc - pc_start; tb->icount = num_insns; - - if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) { - qemu_log("\n"); - qemu_log_unlock(); - } } void restore_state_to_opc(CPUTLGState *env, TranslationBlock *tb, diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c index 0e01f35856..0f6891b8aa 100644 --- a/target/unicore32/translate.c +++ b/target/unicore32/translate.c @@ -1994,12 +1994,12 @@ done_generating: #ifdef DEBUG_DISAS if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) && qemu_log_in_addr_range(pc_start)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("----------------\n"); qemu_log("IN: %s\n", lookup_symbol(pc_start)); log_target_disas(cs, pc_start, dc->pc - pc_start); qemu_log("\n"); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif tb->size = dc->pc - pc_start; diff --git a/tcg/tcg.c b/tcg/tcg.c index 5475d49ed1..0511266d85 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1085,7 +1085,7 @@ void tcg_prologue_init(TCGContext *s) #ifdef DEBUG_DISAS if (qemu_loglevel_mask(CPU_LOG_TB_OUT_ASM)) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("PROLOGUE: [size=%zu]\n", prologue_size); if (s->data_gen_ptr) { size_t code_size = s->data_gen_ptr - buf0; @@ -1110,7 +1110,7 @@ void tcg_prologue_init(TCGContext *s) } qemu_log("\n"); qemu_log_flush(); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif @@ -4041,11 +4041,11 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb) #ifdef DEBUG_DISAS if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP) && qemu_log_in_addr_range(tb->pc))) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("OP:\n"); tcg_dump_ops(s, false); qemu_log("\n"); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif @@ -4086,11 +4086,11 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb) #ifdef DEBUG_DISAS if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_IND) && qemu_log_in_addr_range(tb->pc))) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("OP before indirect lowering:\n"); tcg_dump_ops(s, false); qemu_log("\n"); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif /* Replace indirect temps with direct temps. */ @@ -4107,11 +4107,11 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb) #ifdef DEBUG_DISAS if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP_OPT) && qemu_log_in_addr_range(tb->pc))) { - qemu_log_lock(); + FILE *logfile = qemu_log_lock(); qemu_log("OP after optimization and liveness analysis:\n"); tcg_dump_ops(s, true); qemu_log("\n"); - qemu_log_unlock(); + qemu_log_unlock(logfile); } #endif From 7606488c0efa8f631d31ab9ff8d33b7cf3e2a4c9 Mon Sep 17 00:00:00 2001 From: Robert Foley Date: Mon, 18 Nov 2019 16:15:27 -0500 Subject: [PATCH 15/25] Add use of RCU for qemu_logfile. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This now allows changing the logfile while logging is active, and also solves the issue of a seg fault while changing the logfile. Any read access to the qemu_logfile handle will use the rcu_read_lock()/unlock() around the use of the handle. To fetch the handle we will use atomic_rcu_read(). We also in many cases do a check for validity of the logfile handle before using it to deal with the case where the file is closed and set to NULL. The cases where we write to the qemu_logfile will use atomic_rcu_set(). Writers will also use call_rcu() with a newly added qemu_logfile_free function for freeing/closing when readers have finished. Signed-off-by: Robert Foley Reviewed-by: Alex Bennée Signed-off-by: Alex Bennée Message-Id: <20191118211528.3221-6-robert.foley@linaro.org> --- include/exec/log.h | 33 ++++++++++++++++++--- include/qemu/log.h | 41 ++++++++++++++++++++++---- tcg/tcg.c | 12 ++++++-- util/log.c | 72 ++++++++++++++++++++++++++++++++-------------- 4 files changed, 125 insertions(+), 33 deletions(-) diff --git a/include/exec/log.h b/include/exec/log.h index e2cfd436e6..9bd1e4aa20 100644 --- a/include/exec/log.h +++ b/include/exec/log.h @@ -15,8 +15,15 @@ */ static inline void log_cpu_state(CPUState *cpu, int flags) { + QemuLogFile *logfile; + if (qemu_log_enabled()) { - cpu_dump_state(cpu, qemu_logfile, flags); + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + cpu_dump_state(cpu, logfile->fd, flags); + } + rcu_read_unlock(); } } @@ -40,19 +47,37 @@ static inline void log_cpu_state_mask(int mask, CPUState *cpu, int flags) static inline void log_target_disas(CPUState *cpu, target_ulong start, target_ulong len) { - target_disas(qemu_logfile, cpu, start, len); + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + target_disas(logfile->fd, cpu, start, len); + } + rcu_read_unlock(); } static inline void log_disas(void *code, unsigned long size) { - disas(qemu_logfile, code, size); + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + disas(logfile->fd, code, size); + } + rcu_read_unlock(); } #if defined(CONFIG_USER_ONLY) /* page_dump() output to the log file: */ static inline void log_page_dump(void) { - page_dump(qemu_logfile); + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + page_dump(logfile->fd); + } + rcu_read_unlock(); } #endif #endif diff --git a/include/qemu/log.h b/include/qemu/log.h index a7c5b01571..e0f4e40628 100644 --- a/include/qemu/log.h +++ b/include/qemu/log.h @@ -3,9 +3,16 @@ /* A small part of this API is split into its own header */ #include "qemu/log-for-trace.h" +#include "qemu/rcu.h" + +typedef struct QemuLogFile { + struct rcu_head rcu; + FILE *fd; +} QemuLogFile; /* Private global variable, don't use */ -extern FILE *qemu_logfile; +extern QemuLogFile *qemu_logfile; + /* * The new API: @@ -25,7 +32,16 @@ static inline bool qemu_log_enabled(void) */ static inline bool qemu_log_separate(void) { - return qemu_logfile != NULL && qemu_logfile != stderr; + QemuLogFile *logfile; + bool res = false; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile && logfile->fd != stderr) { + res = true; + } + rcu_read_unlock(); + return res; } #define CPU_LOG_TB_OUT_ASM (1 << 0) @@ -55,8 +71,15 @@ static inline bool qemu_log_separate(void) static inline FILE *qemu_log_lock(void) { - qemu_flockfile(qemu_logfile); - return logfile->fd; + QemuLogFile *logfile; + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + qemu_flockfile(logfile->fd); + return logfile->fd; + } else { + return NULL; + } } static inline void qemu_log_unlock(FILE *fd) @@ -64,6 +87,7 @@ static inline void qemu_log_unlock(FILE *fd) if (fd) { qemu_funlockfile(fd); } + rcu_read_unlock(); } /* Logging functions: */ @@ -73,9 +97,14 @@ static inline void qemu_log_unlock(FILE *fd) static inline void GCC_FMT_ATTR(1, 0) qemu_log_vprintf(const char *fmt, va_list va) { - if (qemu_logfile) { - vfprintf(qemu_logfile, fmt, va); + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + vfprintf(logfile->fd, fmt, va); } + rcu_read_unlock(); } /* log only if a bit is set on the current loglevel mask: diff --git a/tcg/tcg.c b/tcg/tcg.c index 0511266d85..4f616ba38b 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -2114,9 +2114,17 @@ static void tcg_dump_ops(TCGContext *s, bool have_prefs) } if (have_prefs || op->life) { - for (; col < 40; ++col) { - putc(' ', qemu_logfile); + + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + for (; col < 40; ++col) { + putc(' ', logfile->fd); + } } + rcu_read_unlock(); } if (op->life) { diff --git a/util/log.c b/util/log.c index 953a66b5a8..867264da8d 100644 --- a/util/log.c +++ b/util/log.c @@ -28,7 +28,7 @@ static char *logfilename; static QemuMutex qemu_logfile_mutex; -FILE *qemu_logfile; +QemuLogFile *qemu_logfile; int qemu_loglevel; static int log_append = 0; static GArray *debug_regions; @@ -37,10 +37,14 @@ static GArray *debug_regions; int qemu_log(const char *fmt, ...) { int ret = 0; - if (qemu_logfile) { + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { va_list ap; va_start(ap, fmt); - ret = vfprintf(qemu_logfile, fmt, ap); + ret = vfprintf(logfile->fd, fmt, ap); va_end(ap); /* Don't pass back error results. */ @@ -48,6 +52,7 @@ int qemu_log(const char *fmt, ...) ret = 0; } } + rcu_read_unlock(); return ret; } @@ -56,12 +61,24 @@ static void __attribute__((__constructor__)) qemu_logfile_init(void) qemu_mutex_init(&qemu_logfile_mutex); } +static void qemu_logfile_free(QemuLogFile *logfile) +{ + g_assert(logfile); + + if (logfile->fd != stderr) { + fclose(logfile->fd); + } + g_free(logfile); +} + static bool log_uses_own_buffers; /* enable or disable low levels log */ void qemu_set_log(int log_flags) { bool need_to_open_file = false; + QemuLogFile *logfile; + qemu_loglevel = log_flags; #ifdef CONFIG_TRACE_LOG qemu_loglevel |= LOG_TRACE; @@ -79,43 +96,47 @@ void qemu_set_log(int log_flags) } qemu_mutex_lock(&qemu_logfile_mutex); if (qemu_logfile && !need_to_open_file) { - qemu_mutex_unlock(&qemu_logfile_mutex); - qemu_log_close(); + logfile = qemu_logfile; + atomic_rcu_set(&qemu_logfile, NULL); + call_rcu(logfile, qemu_logfile_free, rcu); } else if (!qemu_logfile && need_to_open_file) { + logfile = g_new0(QemuLogFile, 1); if (logfilename) { - qemu_logfile = fopen(logfilename, log_append ? "a" : "w"); - if (!qemu_logfile) { + logfile->fd = fopen(logfilename, log_append ? "a" : "w"); + if (!logfile->fd) { + g_free(logfile); perror(logfilename); _exit(1); } /* In case we are a daemon redirect stderr to logfile */ if (is_daemonized()) { - dup2(fileno(qemu_logfile), STDERR_FILENO); - fclose(qemu_logfile); + dup2(fileno(logfile->fd), STDERR_FILENO); + fclose(logfile->fd); /* This will skip closing logfile in qemu_log_close() */ - qemu_logfile = stderr; + logfile->fd = stderr; } } else { /* Default to stderr if no log file specified */ assert(!is_daemonized()); - qemu_logfile = stderr; + logfile->fd = stderr; } /* must avoid mmap() usage of glibc by setting a buffer "by hand" */ if (log_uses_own_buffers) { static char logfile_buf[4096]; - setvbuf(qemu_logfile, logfile_buf, _IOLBF, sizeof(logfile_buf)); + setvbuf(logfile->fd, logfile_buf, _IOLBF, sizeof(logfile_buf)); } else { #if defined(_WIN32) /* Win32 doesn't support line-buffering, so use unbuffered output. */ - setvbuf(qemu_logfile, NULL, _IONBF, 0); + setvbuf(logfile->fd, NULL, _IONBF, 0); #else - setvbuf(qemu_logfile, NULL, _IOLBF, 0); + setvbuf(logfile->fd, NULL, _IOLBF, 0); #endif log_append = 1; } - qemu_mutex_unlock(&qemu_logfile_mutex); + atomic_rcu_set(&qemu_logfile, logfile); } + qemu_mutex_unlock(&qemu_logfile_mutex); } void qemu_log_needs_buffers(void) @@ -244,18 +265,27 @@ out: /* fflush() the log file */ void qemu_log_flush(void) { - fflush(qemu_logfile); + QemuLogFile *logfile; + + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + if (logfile) { + fflush(logfile->fd); + } + rcu_read_unlock(); } /* Close the log file */ void qemu_log_close(void) { + QemuLogFile *logfile; + qemu_mutex_lock(&qemu_logfile_mutex); - if (qemu_logfile) { - if (qemu_logfile != stderr) { - fclose(qemu_logfile); - } - qemu_logfile = NULL; + logfile = qemu_logfile; + + if (logfile) { + atomic_rcu_set(&qemu_logfile, NULL); + call_rcu(logfile, qemu_logfile_free, rcu); } qemu_mutex_unlock(&qemu_logfile_mutex); } From fb47fc69246591a75555f9b15e87213a7f66bc5e Mon Sep 17 00:00:00 2001 From: Robert Foley Date: Mon, 18 Nov 2019 16:15:28 -0500 Subject: [PATCH 16/25] Added tests for close and change of logfile. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One test ensures that the logfile handle is still valid even if the logfile is changed during logging. The other test validates that the logfile handle remains valid under the logfile lock even if the logfile is closed. Signed-off-by: Robert Foley Reviewed-by: Alex Bennée Signed-off-by: Alex Bennée Message-Id: <20191118211528.3221-7-robert.foley@linaro.org> --- tests/test-logging.c | 80 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tests/test-logging.c b/tests/test-logging.c index a12585f70a..1e646f045d 100644 --- a/tests/test-logging.c +++ b/tests/test-logging.c @@ -108,6 +108,82 @@ static void test_parse_path(gconstpointer data) error_free_or_abort(&err); } +static void test_logfile_write(gconstpointer data) +{ + QemuLogFile *logfile; + QemuLogFile *logfile2; + gchar const *dir = data; + Error *err = NULL; + g_autofree gchar *file_path; + g_autofree gchar *file_path1; + FILE *orig_fd; + + /* + * Before starting test, set log flags, to ensure the file gets + * opened below with the call to qemu_set_log_filename(). + * In cases where a logging backend other than log is used, + * this is needed. + */ + qemu_set_log(CPU_LOG_TB_OUT_ASM); + file_path = g_build_filename(dir, "qemu_test_log_write0.log", NULL); + file_path1 = g_build_filename(dir, "qemu_test_log_write1.log", NULL); + + /* + * Test that even if an open file handle is changed, + * our handle remains valid due to RCU. + */ + qemu_set_log_filename(file_path, &err); + g_assert(!err); + rcu_read_lock(); + logfile = atomic_rcu_read(&qemu_logfile); + orig_fd = logfile->fd; + g_assert(logfile && logfile->fd); + fprintf(logfile->fd, "%s 1st write to file\n", __func__); + fflush(logfile->fd); + + /* Change the logfile and ensure that the handle is still valid. */ + qemu_set_log_filename(file_path1, &err); + g_assert(!err); + logfile2 = atomic_rcu_read(&qemu_logfile); + g_assert(logfile->fd == orig_fd); + g_assert(logfile2->fd != logfile->fd); + fprintf(logfile->fd, "%s 2nd write to file\n", __func__); + fflush(logfile->fd); + rcu_read_unlock(); +} + +static void test_logfile_lock(gconstpointer data) +{ + FILE *logfile; + gchar const *dir = data; + Error *err = NULL; + g_autofree gchar *file_path; + + file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL); + + /* + * Test the use of the logfile lock, such + * that even if an open file handle is closed, + * our handle remains valid for use due to RCU. + */ + qemu_set_log_filename(file_path, &err); + logfile = qemu_log_lock(); + g_assert(logfile); + fprintf(logfile, "%s 1st write to file\n", __func__); + fflush(logfile); + + /* + * Initiate a close file and make sure our handle remains + * valid since we still have the logfile lock. + */ + qemu_log_close(); + fprintf(logfile, "%s 2nd write to file\n", __func__); + fflush(logfile); + qemu_log_unlock(logfile); + + g_assert(!err); +} + /* Remove a directory and all its entries (non-recursive). */ static void rmdir_full(gchar const *root) { @@ -134,6 +210,10 @@ int main(int argc, char **argv) g_test_add_func("/logging/parse_range", test_parse_range); g_test_add_data_func("/logging/parse_path", tmp_path, test_parse_path); + g_test_add_data_func("/logging/logfile_write_path", + tmp_path, test_logfile_write); + g_test_add_data_func("/logging/logfile_lock_path", + tmp_path, test_logfile_lock); rc = g_test_run(); From 7ff5c1fa27c6c7a14089c786e73f2b8dc1423e62 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 18 Dec 2019 02:30:11 +0100 Subject: [PATCH 17/25] docker: gtester is no longer used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are using tap-driver.pl, do not require anymore gtester to be installed to run the testsuite in docker-based tests. Signed-off-by: Paolo Bonzini Reviewed-by: Thomas Huth Signed-off-by: Alex Bennée Message-Id: <1576632611-55032-1-git-send-email-pbonzini@redhat.com> --- tests/docker/common.rc | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/tests/docker/common.rc b/tests/docker/common.rc index 512202b0a1..02cd67a8c5 100755 --- a/tests/docker/common.rc +++ b/tests/docker/common.rc @@ -53,12 +53,7 @@ check_qemu() INVOCATION="$@" fi - if command -v gtester > /dev/null 2>&1 && \ - gtester --version > /dev/null 2>&1; then - make $MAKEFLAGS $INVOCATION - else - echo "No working gtester, skipping make $INVOCATION" - fi + make $MAKEFLAGS $INVOCATION } test_fail() From 88893f7c942ab1470af3e2890044a4228b540cd5 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Tue, 19 Nov 2019 10:21:47 +0100 Subject: [PATCH 18/25] travis.yml: Remove the redundant clang-with-MAIN_SOFTMMU_TARGETS entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We test clang with the MAIN_SOFTMMU_TARGETS twice, once without sanitizers and once with sanitizers enabled. That's somewhat redundant since if compilation and tests succeeded with sanitizers enabled, it should also work fine without sanitizers. Thus remove the clang entry without sanitizers to speed up the CI testing a little bit. Signed-off-by: Thomas Huth Reviewed-by: Wainer dos Santos Moschetta Signed-off-by: Alex Bennée Message-Id: <20191119092147.4260-1-thuth@redhat.com> --- .travis.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index d673ee219e..376b7d6dfa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -180,12 +180,6 @@ matrix: compiler: clang - - env: - - CONFIG="--disable-user --target-list=${MAIN_SOFTMMU_TARGETS}" - - CACHE_NAME="${TRAVIS_BRANCH}-linux-clang-default" - compiler: clang - - - env: - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS} " - CACHE_NAME="${TRAVIS_BRANCH}-linux-clang-sanitize" From 11d9605623b43c2006dbf8f5135b4a5a3a8fb9e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 5 Dec 2019 12:25:12 +0000 Subject: [PATCH 19/25] linux-user: convert target_mprotect debug to tracepoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is a pain to re-compile when you need to debug and tracepoints are a fairly low impact way to instrument QEMU. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Laurent Vivier Message-Id: <20191205122518.10010-2-alex.bennee@linaro.org> --- linux-user/mmap.c | 10 ++-------- linux-user/trace-events | 3 +++ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 46a6e3a761..26a83e7406 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -17,7 +17,7 @@ * along with this program; if not, see . */ #include "qemu/osdep.h" - +#include "trace.h" #include "qemu.h" //#define DEBUG_MMAP @@ -66,13 +66,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot) abi_ulong end, host_start, host_end, addr; int prot1, ret; -#ifdef DEBUG_MMAP - printf("mprotect: start=0x" TARGET_ABI_FMT_lx - "len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len, - prot & PROT_READ ? 'r' : '-', - prot & PROT_WRITE ? 'w' : '-', - prot & PROT_EXEC ? 'x' : '-'); -#endif + trace_target_mprotect(start, len, prot); if ((start & ~TARGET_PAGE_MASK) != 0) return -TARGET_EINVAL; diff --git a/linux-user/trace-events b/linux-user/trace-events index 6df234bbb6..8419243de4 100644 --- a/linux-user/trace-events +++ b/linux-user/trace-events @@ -11,3 +11,6 @@ user_handle_signal(void *env, int target_sig) "env=%p signal %d" user_host_signal(void *env, int host_sig, int target_sig) "env=%p signal %d (target %d(" user_queue_signal(void *env, int target_sig) "env=%p signal %d" user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_addr) "env=%p frame psw.addr 0x%"PRIx64 " current psw.addr 0x%"PRIx64 + +# mmap.c +target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x" From 5a67bb96b0c4ea9f089b97edf88f3f06a4611850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 5 Dec 2019 12:25:13 +0000 Subject: [PATCH 20/25] linux-user: convert target_mmap debug to tracepoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is a pain to re-compile when you need to debug and tracepoints are a fairly low impact way to instrument QEMU. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Laurent Vivier Message-Id: <20191205122518.10010-3-alex.bennee@linaro.org> --- linux-user/mmap.c | 27 +-------------------------- linux-user/trace-events | 1 + 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 26a83e7406..f4f10deaea 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -363,32 +363,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len; mmap_lock(); -#ifdef DEBUG_MMAP - { - printf("mmap: start=0x" TARGET_ABI_FMT_lx - " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=", - start, len, - prot & PROT_READ ? 'r' : '-', - prot & PROT_WRITE ? 'w' : '-', - prot & PROT_EXEC ? 'x' : '-'); - if (flags & MAP_FIXED) - printf("MAP_FIXED "); - if (flags & MAP_ANONYMOUS) - printf("MAP_ANON "); - switch(flags & MAP_TYPE) { - case MAP_PRIVATE: - printf("MAP_PRIVATE "); - break; - case MAP_SHARED: - printf("MAP_SHARED "); - break; - default: - printf("[MAP_TYPE=0x%x] ", flags & MAP_TYPE); - break; - } - printf("fd=%d offset=" TARGET_ABI_FMT_lx "\n", fd, offset); - } -#endif + trace_target_mmap(start, len, prot, flags, fd, offset); if (!len) { errno = EINVAL; diff --git a/linux-user/trace-events b/linux-user/trace-events index 8419243de4..8d8d4c3c68 100644 --- a/linux-user/trace-events +++ b/linux-user/trace-events @@ -14,3 +14,4 @@ user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_add # mmap.c target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x" +target_mmap(uint64_t start, uint64_t len, int pflags, int mflags, int fd, uint64_t offset) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x flags=0x%x fd=%d offset=0x%"PRIx64 From d0e165ae2bdf107f3c63bf11138097869d91ef2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 5 Dec 2019 12:25:14 +0000 Subject: [PATCH 21/25] linux-user: add target_mmap_complete tracepoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For full details we also want to see where the mmaps end up. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Laurent Vivier Message-Id: <20191205122518.10010-4-alex.bennee@linaro.org> --- linux-user/mmap.c | 2 +- linux-user/trace-events | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index f4f10deaea..0b1b43ac3c 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -538,8 +538,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, the_end1: page_set_flags(start, start + len, prot | PAGE_VALID); the_end: + trace_target_mmap_complete(start); #ifdef DEBUG_MMAP - printf("ret=0x" TARGET_ABI_FMT_lx "\n", start); page_dump(stdout); printf("\n"); #endif diff --git a/linux-user/trace-events b/linux-user/trace-events index 8d8d4c3c68..6d6aeef7b5 100644 --- a/linux-user/trace-events +++ b/linux-user/trace-events @@ -15,3 +15,4 @@ user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_add # mmap.c target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x" target_mmap(uint64_t start, uint64_t len, int pflags, int mflags, int fd, uint64_t offset) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x flags=0x%x fd=%d offset=0x%"PRIx64 +target_mmap_complete(uint64_t retaddr) "retaddr=0x%"PRIx64 From 10d0d505de750590c21a78c0652bf5a9c142302a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 5 Dec 2019 12:25:15 +0000 Subject: [PATCH 22/25] linux-user: log page table changes under -d page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CPU_LOG_PAGE flag is woefully underused and could stand to do extra duty tracking page changes. If the user doesn't want to see the details as things change they still have the tracepoints available. We push the locking into log_page_dump and pass a reason for the banner text. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Laurent Vivier Message-Id: <20191205122518.10010-5-alex.bennee@linaro.org> --- bsd-user/main.c | 2 +- include/exec/log.h | 11 +++++------ linux-user/main.c | 2 +- linux-user/mmap.c | 8 ++++---- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/bsd-user/main.c b/bsd-user/main.c index 470a8bf79e..7f4e3cd627 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -963,7 +963,7 @@ int main(int argc, char **argv) if (qemu_loglevel_mask(CPU_LOG_PAGE)) { qemu_log("guest_base 0x%lx\n", guest_base); - log_page_dump(); + log_page_dump("binary load"); qemu_log("start_brk 0x" TARGET_ABI_FMT_lx "\n", info->start_brk); qemu_log("end_code 0x" TARGET_ABI_FMT_lx "\n", info->end_code); diff --git a/include/exec/log.h b/include/exec/log.h index 9bd1e4aa20..fcc7b9e00b 100644 --- a/include/exec/log.h +++ b/include/exec/log.h @@ -69,15 +69,14 @@ static inline void log_disas(void *code, unsigned long size) #if defined(CONFIG_USER_ONLY) /* page_dump() output to the log file: */ -static inline void log_page_dump(void) +static inline void log_page_dump(const char *operation) { - QemuLogFile *logfile; - rcu_read_lock(); - logfile = atomic_rcu_read(&qemu_logfile); + FILE *logfile = qemu_log_lock(); if (logfile) { - page_dump(logfile->fd); + qemu_log("page layout changed following %s\n", operation); + page_dump(logfile); } - rcu_read_unlock(); + qemu_log_unlock(logfile); } #endif #endif diff --git a/linux-user/main.c b/linux-user/main.c index 6ff7851e86..8718d03ee2 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -826,7 +826,7 @@ int main(int argc, char **argv, char **envp) if (qemu_loglevel_mask(CPU_LOG_PAGE)) { qemu_log("guest_base 0x%lx\n", guest_base); - log_page_dump(); + log_page_dump("binary load"); qemu_log("start_brk 0x" TARGET_ABI_FMT_lx "\n", info->start_brk); qemu_log("end_code 0x" TARGET_ABI_FMT_lx "\n", info->end_code); diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 0b1b43ac3c..3d90fa459c 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -18,6 +18,7 @@ */ #include "qemu/osdep.h" #include "trace.h" +#include "exec/log.h" #include "qemu.h" //#define DEBUG_MMAP @@ -539,10 +540,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, page_set_flags(start, start + len, prot | PAGE_VALID); the_end: trace_target_mmap_complete(start); -#ifdef DEBUG_MMAP - page_dump(stdout); - printf("\n"); -#endif + if (qemu_loglevel_mask(CPU_LOG_PAGE)) { + log_page_dump(__func__); + } tb_invalidate_phys_range(start, start + len); mmap_unlock(); return start; From b7b18d2680ea148a4b5e17e935ef7a68b6d391c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 5 Dec 2019 12:25:16 +0000 Subject: [PATCH 23/25] linux-user: convert target_munmap debug to a tracepoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the final bit of DEBUG_MMAP to a tracepoint and remove the last remanents of the #ifdef hackery. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Laurent Vivier Message-Id: <20191205122518.10010-6-alex.bennee@linaro.org> --- linux-user/mmap.c | 9 ++------- linux-user/trace-events | 1 + 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 3d90fa459c..8685f02e7e 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -21,8 +21,6 @@ #include "exec/log.h" #include "qemu.h" -//#define DEBUG_MMAP - static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER; static __thread int mmap_lock_count; @@ -597,11 +595,8 @@ int target_munmap(abi_ulong start, abi_ulong len) abi_ulong end, real_start, real_end, addr; int prot, ret; -#ifdef DEBUG_MMAP - printf("munmap: start=0x" TARGET_ABI_FMT_lx " len=0x" - TARGET_ABI_FMT_lx "\n", - start, len); -#endif + trace_target_munmap(start, len); + if (start & ~TARGET_PAGE_MASK) return -TARGET_EINVAL; len = TARGET_PAGE_ALIGN(len); diff --git a/linux-user/trace-events b/linux-user/trace-events index 6d6aeef7b5..f6de1b8bef 100644 --- a/linux-user/trace-events +++ b/linux-user/trace-events @@ -16,3 +16,4 @@ user_s390x_restore_sigregs(void *env, uint64_t sc_psw_addr, uint64_t env_psw_add target_mprotect(uint64_t start, uint64_t len, int flags) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x" target_mmap(uint64_t start, uint64_t len, int pflags, int mflags, int fd, uint64_t offset) "start=0x%"PRIx64 " len=0x%"PRIx64 " prot=0x%x flags=0x%x fd=%d offset=0x%"PRIx64 target_mmap_complete(uint64_t retaddr) "retaddr=0x%"PRIx64 +target_munmap(uint64_t start, uint64_t len) "start=0x%"PRIx64" len=0x%"PRIx64 From e66eae7a8b8da38daaf0e8a5ac72bd8ee343c2b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Thu, 5 Dec 2019 12:25:17 +0000 Subject: [PATCH 24/25] trace: replace hand-crafted pattern_glob with g_pattern_match_simple MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We already use g_pattern_match elsewhere so remove the duplication. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Stefan Hajnoczi Message-Id: <20191205122518.10010-7-alex.bennee@linaro.org> --- trace/control.c | 35 +---------------------------------- 1 file changed, 1 insertion(+), 34 deletions(-) diff --git a/trace/control.c b/trace/control.c index d9cafc161b..0fb8124160 100644 --- a/trace/control.c +++ b/trace/control.c @@ -98,38 +98,6 @@ TraceEvent *trace_event_name(const char *name) return NULL; } -static bool pattern_glob(const char *pat, const char *ev) -{ - while (*pat != '\0' && *ev != '\0') { - if (*pat == *ev) { - pat++; - ev++; - } - else if (*pat == '*') { - if (pattern_glob(pat, ev+1)) { - return true; - } else if (pattern_glob(pat+1, ev)) { - return true; - } else { - return false; - } - } else { - return false; - } - } - - while (*pat == '*') { - pat++; - } - - if (*pat == '\0' && *ev == '\0') { - return true; - } else { - return false; - } -} - - void trace_event_iter_init(TraceEventIter *iter, const char *pattern) { iter->event = 0; @@ -148,8 +116,7 @@ TraceEvent *trace_event_iter_next(TraceEventIter *iter) iter->group++; } if (!iter->pattern || - pattern_glob(iter->pattern, - trace_event_get_name(ev))) { + g_pattern_match_simple(iter->pattern, trace_event_get_name(ev))) { return ev; } } From 380976f40f909b735acb60d5d424de7eb1b7107e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 11 Dec 2019 17:05:16 +0000 Subject: [PATCH 25/25] tests/tcg: ensure we re-configure if configure.sh is updated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were only doing this if docker was enabled which isn't quite right. Fixes: fc76c56d3f47 Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20191211170520.7747-17-alex.bennee@linaro.org> --- tests/tcg/Makefile.prereqs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tcg/Makefile.prereqs b/tests/tcg/Makefile.prereqs index 7494b31b95..9a29604a83 100644 --- a/tests/tcg/Makefile.prereqs +++ b/tests/tcg/Makefile.prereqs @@ -13,6 +13,6 @@ DOCKER_IMAGE:= ifneq ($(DOCKER_IMAGE),) build-tcg-tests-$(PROBE_TARGET): docker-image-$(DOCKER_IMAGE) +endif $(BUILD_DIR)/tests/tcg/config_$(PROBE_TARGET).mak: config-host.mak config-host.mak: $(SRC_PATH)/tests/tcg/configure.sh -endif