[PULL 00/25] testing and logging updates

Alex Bennée posted 25 patches 4 years, 4 months ago
Only 11 patches received!
docs/devel/testing.rst        |   6 ++-
configure                     |   9 +++-
include/exec/log.h            |  34 +++++++++++--
include/qemu/log.h            |  48 +++++++++++++++---
accel/tcg/cpu-exec.c          |   4 +-
accel/tcg/translate-all.c     |   4 +-
accel/tcg/translator.c        |   4 +-
bsd-user/main.c               |   2 +-
exec.c                        |   4 +-
hw/net/can/can_sja1000.c      |   4 +-
linux-user/main.c             |   2 +-
linux-user/mmap.c             |  56 ++++-----------------
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                     |  28 +++++++----
tests/hd-geo-test.c           |  12 ++++-
tests/test-logging.c          |  80 ++++++++++++++++++++++++++++++
tests/test-util-filemonitor.c |  11 +++++
trace/control.c               |  35 +------------
util/log.c                    | 100 ++++++++++++++++++++++++++++---------
.cirrus.yml                   |   8 ++-
.gitlab-ci.yml                |  28 ++++++++---
.shippable.yml                |   4 +-
.travis.yml                   | 112 ++++++++++++++++++++++++++++++++++++------
linux-user/trace-events       |   6 +++
tests/docker/common.rc        |   7 +--
tests/qemu-iotests/005        |   5 +-
tests/qemu-iotests/060        |   3 ++
tests/qemu-iotests/079        |   3 ++
tests/qemu-iotests/220        |   6 +--
tests/qemu-iotests/common.rc  |  10 ++++
tests/tcg/Makefile.prereqs    |   2 +-
tests/tcg/configure.sh        |   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 +-
47 files changed, 486 insertions(+), 216 deletions(-)
[PULL 00/25] testing and logging updates
Posted by Alex Bennée 4 years, 4 months ago
The following changes since commit aceeaa69d28e6f08a24395d0aa6915b687d0a681:

  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-12-17' into staging (2019-12-17 15:55:20 +0000)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-tesing-and-misc-191219-1

for you to fetch changes up to 380976f40f909b735acb60d5d424de7eb1b7107e:

  tests/tcg: ensure we re-configure if configure.sh is updated (2019-12-19 08:20:16 +0000)

----------------------------------------------------------------
Various testing and logging updates

  - test tci with Travis
  - enable multiarch testing in Travis
  - default to out-of-tree builds
  - make changing logfile safe via RCU
  - remove redundant tests
  - remove gtester test from docker
  - convert DEBUG_MMAP to tracepoints
  - remove hand rolled glob function
  - trigger tcg re-configure when needed

----------------------------------------------------------------
Alex Bennée (8):
      configure: allow disable of cross compilation containers
      linux-user: convert target_mprotect debug to tracepoint
      linux-user: convert target_mmap debug to tracepoint
      linux-user: add target_mmap_complete tracepoint
      linux-user: log page table changes under -d page
      linux-user: convert target_munmap debug to a tracepoint
      trace: replace hand-crafted pattern_glob with g_pattern_match_simple
      tests/tcg: ensure we re-configure if configure.sh is updated

Paolo Bonzini (2):
      ci: build out-of-tree
      docker: gtester is no longer used

Robert Foley (6):
      Fix double free issue in qemu_set_log_filename().
      Cleaned up flow of code in qemu_set_log(), to simplify and clarify.
      Add a mutex to guarantee single writer to qemu_logfile handle.
      qemu_log_lock/unlock now preserves the qemu_logfile handle.
      Add use of RCU for qemu_logfile.
      Added tests for close and change of logfile.

Thomas Huth (8):
      travis.yml: Run tcg tests with tci
      iotests: Provide a function for checking the creation of huge files
      iotests: Skip test 060 if it is not possible to create large files
      iotests: Skip test 079 if it is not possible to create large files
      tests/hd-geo-test: Skip test when images can not be created
      tests/test-util-filemonitor: Skip test on non-x86 Travis containers
      travis.yml: Enable builds on arm64, ppc64le and s390x
      travis.yml: Remove the redundant clang-with-MAIN_SOFTMMU_TARGETS entry

Wainer dos Santos Moschetta (1):
      tests/vm: Allow to set qemu-img path

 docs/devel/testing.rst        |   6 ++-
 configure                     |   9 +++-
 include/exec/log.h            |  34 +++++++++++--
 include/qemu/log.h            |  48 +++++++++++++++---
 accel/tcg/cpu-exec.c          |   4 +-
 accel/tcg/translate-all.c     |   4 +-
 accel/tcg/translator.c        |   4 +-
 bsd-user/main.c               |   2 +-
 exec.c                        |   4 +-
 hw/net/can/can_sja1000.c      |   4 +-
 linux-user/main.c             |   2 +-
 linux-user/mmap.c             |  56 ++++-----------------
 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                     |  28 +++++++----
 tests/hd-geo-test.c           |  12 ++++-
 tests/test-logging.c          |  80 ++++++++++++++++++++++++++++++
 tests/test-util-filemonitor.c |  11 +++++
 trace/control.c               |  35 +------------
 util/log.c                    | 100 ++++++++++++++++++++++++++++---------
 .cirrus.yml                   |   8 ++-
 .gitlab-ci.yml                |  28 ++++++++---
 .shippable.yml                |   4 +-
 .travis.yml                   | 112 ++++++++++++++++++++++++++++++++++++------
 linux-user/trace-events       |   6 +++
 tests/docker/common.rc        |   7 +--
 tests/qemu-iotests/005        |   5 +-
 tests/qemu-iotests/060        |   3 ++
 tests/qemu-iotests/079        |   3 ++
 tests/qemu-iotests/220        |   6 +--
 tests/qemu-iotests/common.rc  |  10 ++++
 tests/tcg/Makefile.prereqs    |   2 +-
 tests/tcg/configure.sh        |   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 +-
 47 files changed, 486 insertions(+), 216 deletions(-)

-- 
2.20.1


Re: [PULL 00/25] testing and logging updates
Posted by Peter Maydell 4 years, 4 months ago
On Thu, 19 Dec 2019 at 10:49, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> The following changes since commit aceeaa69d28e6f08a24395d0aa6915b687d0a681:
>
>   Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-12-17' into staging (2019-12-17 15:55:20 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-tesing-and-misc-191219-1
>
> for you to fetch changes up to 380976f40f909b735acb60d5d424de7eb1b7107e:
>
>   tests/tcg: ensure we re-configure if configure.sh is updated (2019-12-19 08:20:16 +0000)
>
> ----------------------------------------------------------------
> Various testing and logging updates
>
>   - test tci with Travis
>   - enable multiarch testing in Travis
>   - default to out-of-tree builds
>   - make changing logfile safe via RCU
>   - remove redundant tests
>   - remove gtester test from docker
>   - convert DEBUG_MMAP to tracepoints
>   - remove hand rolled glob function
>   - trigger tcg re-configure when needed
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM

[PULL 03/25] travis.yml: Run tcg tests with tci
Posted by Alex Bennée 4 years, 4 months ago
From: Thomas Huth <thuth@redhat.com>

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 <sw@weilnetz.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20191204083133.6198-1-thuth@redhat.com>
[AJB: just --enable-debug-tcg]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

diff --git a/.travis.yml b/.travis.yml
index 6cb8af6fa59..15946293ff3 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
-- 
2.20.1


[PULL 05/25] iotests: Skip test 060 if it is not possible to create large files
Posted by Alex Bennée 4 years, 4 months ago
From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20191204154618.23560-3-thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index b91d8321bb8..d96f17a4846 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)
-- 
2.20.1


[PULL 06/25] iotests: Skip test 079 if it is not possible to create large files
Posted by Alex Bennée 4 years, 4 months ago
From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20191204154618.23560-4-thuth@redhat.com>

diff --git a/tests/qemu-iotests/079 b/tests/qemu-iotests/079
index 81f0c21f530..78536d3bbfa 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"
-- 
2.20.1


[PULL 07/25] tests/hd-geo-test: Skip test when images can not be created
Posted by Alex Bennée 4 years, 4 months ago
From: Thomas Huth <thuth@redhat.com>

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é <philmd@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20191204154618.23560-5-thuth@redhat.com>

diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index 7e86c5416cc..a2498005440 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++) {
-- 
2.20.1


[PULL 08/25] tests/test-util-filemonitor: Skip test on non-x86 Travis containers
Posted by Alex Bennée 4 years, 4 months ago
From: Thomas Huth <thuth@redhat.com>

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é <philmd@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20191204154618.23560-6-thuth@redhat.com>

diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index 301cd2db619..45009c69f41 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
-- 
2.20.1


[PULL 09/25] travis.yml: Enable builds on arm64, ppc64le and s390x
Posted by Alex Bennée 4 years, 4 months ago
From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Message-Id: <20191204154618.23560-8-thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/.travis.yml b/.travis.yml
index 15946293ff3..b68566b1fe9 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'.
-- 
2.20.1


[PULL 11/25] Fix double free issue in qemu_set_log_filename().
Posted by Alex Bennée 4 years, 4 months ago
From: Robert Foley <robert.foley@linaro.org>

After freeing the logfilename, we set logfilename to NULL, in case of an
error which returns without setting logfilename.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20191118211528.3221-2-robert.foley@linaro.org>

diff --git a/util/log.c b/util/log.c
index 1ca13059eef..4316fe74eee 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) {
-- 
2.20.1


[PULL 12/25] Cleaned up flow of code in qemu_set_log(), to simplify and clarify.
Posted by Alex Bennée 4 years, 4 months ago
From: Robert Foley <robert.foley@linaro.org>

Also added some explanation of the reasoning behind the branches.

Signed-off-by: Robert Foley <robert.foley@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20191118211528.3221-3-robert.foley@linaro.org>

diff --git a/util/log.c b/util/log.c
index 4316fe74eee..417d16ec66e 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)
-- 
2.20.1


[PULL 13/25] Add a mutex to guarantee single writer to qemu_logfile handle.
Posted by Alex Bennée 4 years, 4 months ago
From: Robert Foley <robert.foley@linaro.org>

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 <robert.foley@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20191118211528.3221-4-robert.foley@linaro.org>

diff --git a/util/log.c b/util/log.c
index 417d16ec66e..953a66b5a8d 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[] = {
-- 
2.20.1


[PULL 16/25] Added tests for close and change of logfile.
Posted by Alex Bennée 4 years, 4 months ago
From: Robert Foley <robert.foley@linaro.org>

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 <robert.foley@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20191118211528.3221-7-robert.foley@linaro.org>

diff --git a/tests/test-logging.c b/tests/test-logging.c
index a12585f70af..1e646f045dc 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();
 
-- 
2.20.1


[PULL 17/25] docker: gtester is no longer used
Posted by Alex Bennée 4 years, 4 months ago
From: Paolo Bonzini <pbonzini@redhat.com>

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 <pbonzini@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1576632611-55032-1-git-send-email-pbonzini@redhat.com>

diff --git a/tests/docker/common.rc b/tests/docker/common.rc
index 512202b0a19..02cd67a8c5e 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()
-- 
2.20.1


[PULL 18/25] travis.yml: Remove the redundant clang-with-MAIN_SOFTMMU_TARGETS entry
Posted by Alex Bennée 4 years, 4 months ago
From: Thomas Huth <thuth@redhat.com>

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 <thuth@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20191119092147.4260-1-thuth@redhat.com>

diff --git a/.travis.yml b/.travis.yml
index d673ee219e7..376b7d6dfa8 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"
-- 
2.20.1


[PULL 24/25] trace: replace hand-crafted pattern_glob with g_pattern_match_simple
Posted by Alex Bennée 4 years, 4 months ago
We already use g_pattern_match elsewhere so remove the duplication.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20191205122518.10010-7-alex.bennee@linaro.org>

diff --git a/trace/control.c b/trace/control.c
index d9cafc161bb..0fb81241607 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;
         }
     }
-- 
2.20.1


[PULL 25/25] tests/tcg: ensure we re-configure if configure.sh is updated
Posted by Alex Bennée 4 years, 4 months ago
We were only doing this if docker was enabled which isn't quite right.

Fixes: fc76c56d3f47
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191211170520.7747-17-alex.bennee@linaro.org>

diff --git a/tests/tcg/Makefile.prereqs b/tests/tcg/Makefile.prereqs
index 7494b31b952..9a29604a839 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
-- 
2.20.1