[PATCH v2 0/6] Automation of Coverity Scan uploads (via Docker)

[PATCH v2 0/6] Automation of Coverity Scan uploads (via Docker)

[Peter Maydell]

v1 of this series was over a year ago:
https://patchew.org/QEMU/20181113184641.4492-1-peter.maydell@linaro.org/

I dusted it off and fixed some stuff because Paolo reports that the
machine he was previously using for uploads can't run the Coverity
tools any more.

The first four patches are fixes for problems that cause the Coverity
tool not to be able to scan everything.  The first one in particular
meant that every compilation unit failed, which would block uploads. 
The other 3 would reduce the scan coverage but weren't fatal.  (The
only remaining warnings in the log are where Coverity complains about
asm intrinsics system headers.)

With these scripts you can do an upload with
COVERITY_TOKEN=nnnnnnnnn ./scripts/coverity-scan/run-coverity-scan --docker
(where nnnnnnnn is the project's secret token which admins can
get from the Coverity web UI).

I did in fact do an upload to test it, so the currently visible
results on the website are the result of a scan on ce73691e258 plus
this series.

The new upload has +112 defects, which is quite a lot, but I don't
think it's so many that it is "defects we rejected as false positives
coming back again"; my guess is a combination of the fixes in the
first 4 patches increasing coverage plus we haven't run a test in a
while plus maybe the script has some more config options enabled that
Paolo's box did not.  (In the web UI defects that were dismissed as
FPs seem still to be considered present-but-dismissed, so I think
that's OK.)

Not much has changed since v1; I didn't get very much feedback
the first time around[*]. Docker still seems to do the "download
the Coverity tools" part more often than I expect. On the other
hand "actually automated with a script in the tree" beats "not
automated and currently broken" so maybe this patchset as it
stands is good enough, given that basically 1 or 2 people ever
will be running the script ?

[*] Eric will note that yes, the script still uses set -e.

(Like v1 this doesn't try to tie it into Travis, but we could
in theory do that some day, or have some other automated once
a week run of the script.)

thanks
-- PMM

Peter Maydell (6):
  osdep.h: Drop no-longer-needed Coverity workarounds
  thread.h: Fix Coverity version of qemu_cond_timedwait()
  thread.h: Remove trailing semicolons from Coverity qemu_mutex_lock()
    etc
  linux-user/flatload.c: Use "" for include of QEMU header target_flat.h
  scripts/run-coverity-scan: Script to run Coverity Scan build
  scripts/coverity-scan: Add Docker support

 include/qemu/osdep.h                       |  14 -
 include/qemu/thread.h                      |  12 +-
 linux-user/flatload.c                      |   2 +-
 MAINTAINERS                                |   5 +
 scripts/coverity-scan/coverity-scan.docker | 131 +++++++
 scripts/coverity-scan/run-coverity-scan    | 401 +++++++++++++++++++++
 6 files changed, 544 insertions(+), 21 deletions(-)
 create mode 100644 scripts/coverity-scan/coverity-scan.docker
 create mode 100755 scripts/coverity-scan/run-coverity-scan

-- 
2.20.1

[PATCH v2 1/6] osdep.h: Drop no-longer-needed Coverity workarounds

[Peter Maydell]

In commit a1a98357e3fd in 2018 we added some workarounds for Coverity
not being able to handle the _Float* types introduced by recent
glibc.  Newer versions of the Coverity scan tools have support for
these types, and will fail with errors about duplicate typedefs if we
have our workaround.  Remove our copy of the typedefs.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Sadly I don't think there's any way to tell whether we should
or should not provide the typedefs, so anybody with an older
Coverity will presumably find this breaks them.
---
 include/qemu/osdep.h | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 9bd3dcfd136..20f5c5f197d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -33,20 +33,6 @@
 #else
 #include "exec/poison.h"
 #endif
-#ifdef __COVERITY__
-/* Coverity does not like the new _Float* types that are used by
- * recent glibc, and croaks on every single file that includes
- * stdlib.h.  These typedefs are enough to please it.
- *
- * Note that these fix parse errors so they cannot be placed in
- * scripts/coverity-model.c.
- */
-typedef float _Float32;
-typedef double _Float32x;
-typedef double _Float64;
-typedef __float80 _Float64x;
-typedef __float128 _Float128;
-#endif
 
 #include "qemu/compiler.h"
 
-- 
2.20.1

[PATCH v2 2/6] thread.h: Fix Coverity version of qemu_cond_timedwait()

[Peter Maydell]

For Coverity's benefit, we provide simpler versions of functions like
qemu_mutex_lock(), qemu_cond_wait() and qemu_cond_timedwait().  When
we added qemu_cond_timedwait() in commit 3dcc9c6ec4ea, a cut and
paste error meant that the Coverity version of qemu_cond_timedwait()
was using the wrong _impl function, which makes the Coverity parser
complain:

"/qemu/include/qemu/thread.h", line 159: warning #140: too many arguments in
          function call
      return qemu_cond_timedwait(cond, mutex, ms);
             ^

"/qemu/include/qemu/thread.h", line 159: warning #120: return value type does
          not match the function type
      return qemu_cond_timedwait(cond, mutex, ms);
             ^

"/qemu/include/qemu/thread.h", line 156: warning #1563: function
          "qemu_cond_timedwait" not emitted, consider modeling it or review
          parse diagnostics to improve fidelity
  static inline bool (qemu_cond_timedwait)(QemuCond *cond, QemuMutex *mutex,
                      ^

These aren't fatal, but reduce the scope of the analysis. Fix the error.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/thread.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 047db0307e7..10262c63f58 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -67,7 +67,7 @@ extern QemuCondTimedWaitFunc qemu_cond_timedwait_func;
 #define qemu_cond_wait(c, m)                                            \
             qemu_cond_wait_impl(c, m, __FILE__, __LINE__);
 #define qemu_cond_timedwait(c, m, ms)                                   \
-            qemu_cond_wait_impl(c, m, ms, __FILE__, __LINE__);
+            qemu_cond_timedwait_impl(c, m, ms, __FILE__, __LINE__);
 #else
 #define qemu_mutex_lock(m) ({                                           \
             QemuMutexLockFunc _f = atomic_read(&qemu_mutex_lock_func);  \
-- 
2.20.1

[PATCH v2 3/6] thread.h: Remove trailing semicolons from Coverity qemu_mutex_lock() etc

[Peter Maydell]

All the Coverity-specific definitions of qemu_mutex_lock() and friends
have a trailing semicolon. This works fine almost everywhere because
of QEMU's mandatory-braces coding style and because most callsites are
simple, but target/s390x/sigp.c has a use of qemu_mutex_trylock() as
an if() statement, which makes the ';' a syntax error:
"../target/s390x/sigp.c", line 461: warning #18: expected a ")"
      if (qemu_mutex_trylock(&qemu_sigp_mutex)) {
          ^

Remove the bogus semicolons from the macro definitions.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/qemu/thread.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 10262c63f58..d22848138ea 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -57,17 +57,17 @@ extern QemuCondTimedWaitFunc qemu_cond_timedwait_func;
  * hide them.
  */
 #define qemu_mutex_lock(m)                                              \
-            qemu_mutex_lock_impl(m, __FILE__, __LINE__);
+            qemu_mutex_lock_impl(m, __FILE__, __LINE__)
 #define qemu_mutex_trylock(m)                                           \
-            qemu_mutex_trylock_impl(m, __FILE__, __LINE__);
+            qemu_mutex_trylock_impl(m, __FILE__, __LINE__)
 #define qemu_rec_mutex_lock(m)                                          \
-            qemu_rec_mutex_lock_impl(m, __FILE__, __LINE__);
+            qemu_rec_mutex_lock_impl(m, __FILE__, __LINE__)
 #define qemu_rec_mutex_trylock(m)                                       \
-            qemu_rec_mutex_trylock_impl(m, __FILE__, __LINE__);
+            qemu_rec_mutex_trylock_impl(m, __FILE__, __LINE__)
 #define qemu_cond_wait(c, m)                                            \
-            qemu_cond_wait_impl(c, m, __FILE__, __LINE__);
+            qemu_cond_wait_impl(c, m, __FILE__, __LINE__)
 #define qemu_cond_timedwait(c, m, ms)                                   \
-            qemu_cond_timedwait_impl(c, m, ms, __FILE__, __LINE__);
+            qemu_cond_timedwait_impl(c, m, ms, __FILE__, __LINE__)
 #else
 #define qemu_mutex_lock(m) ({                                           \
             QemuMutexLockFunc _f = atomic_read(&qemu_mutex_lock_func);  \
-- 
2.20.1

[PATCH v2 4/6] linux-user/flatload.c: Use "" for include of QEMU header target_flat.h

[Peter Maydell]

The target_flat.h file is a QEMU header, so we should include it using
quotes, not angle brackets.

Coverity otherwise is unable to find the header:

"../linux-user/flatload.c", line 40: error #1712: cannot open source file
          "target_flat.h"
  #include <target_flat.h>
                          ^

because the relevant directory is only on the -iquote path, not the -I path.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I don't know why Coverity in particular has trouble here but
real compilers don't. Still, the "" is the right thing.
---
 linux-user/flatload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 0122ab3afe6..66901f39cc5 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -37,7 +37,7 @@
 
 #include "qemu.h"
 #include "flat.h"
-#include <target_flat.h>
+#include "target_flat.h"
 
 //#define DEBUG
 
-- 
2.20.1

[PATCH v2 5/6] scripts/run-coverity-scan: Script to run Coverity Scan build

[Peter Maydell]

Add a new script to automate the process of running the Coverity
Scan build tools and uploading the resulting tarball to the
website.

This is intended eventually to be driven from Travis,
but it can be run locally, if you are a maintainer of the
QEMU project on the Coverity Scan website and have the secret
upload token.

The script must be run on a Fedora 30 system.  Support for using a
Docker container is added in a following commit.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
changes v1->v2:
 * fix sense of DRYRUN test in check_upload_permissions
 * use nproc rather than hardcoding -j8
 * use $PWD rather than $(pwd)
 * minor tweaks to configure line
 * new --results-tarball option
---
 MAINTAINERS                             |   5 +
 scripts/coverity-scan/run-coverity-scan | 311 ++++++++++++++++++++++++
 2 files changed, 316 insertions(+)
 create mode 100755 scripts/coverity-scan/run-coverity-scan

diff --git a/MAINTAINERS b/MAINTAINERS
index 7364af0d8b0..395534522b6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2003,6 +2003,11 @@ M: Markus Armbruster <armbru@redhat.com>
 S: Supported
 F: scripts/coverity-model.c
 
+Coverity Scan integration
+M: Peter Maydell <peter.maydell@linaro.org>
+S: Maintained
+F: scripts/coverity-scan/
+
 Device Tree
 M: Alistair Francis <alistair.francis@wdc.com>
 R: David Gibson <david@gibson.dropbear.id.au>
diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
new file mode 100755
index 00000000000..d40b51969fa
--- /dev/null
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -0,0 +1,311 @@
+#!/bin/sh -e
+
+# Upload a created tarball to Coverity Scan, as per
+# https://scan.coverity.com/projects/qemu/builds/new
+
+# This work is licensed under the terms of the GNU GPL version 2,
+# or (at your option) any later version.
+# See the COPYING file in the top-level directory.
+#
+# Copyright (c) 2017-2020 Linaro Limited
+# Written by Peter Maydell
+
+# Note that this script will automatically download and
+# run the (closed-source) coverity build tools, so don't
+# use it if you don't trust them!
+
+# This script assumes that you're running it from a QEMU source
+# tree, and that tree is a fresh clean one, because we do an in-tree
+# build. (This is necessary so that the filenames that the Coverity
+# Scan server sees are relative paths that match up with the component
+# regular expressions it uses; an out-of-tree build won't work for this.)
+# The host machine should have as many of QEMU's dependencies
+# installed as possible, for maximum coverity coverage.
+
+# To do an upload you need to be a maintainer in the Coverity online
+# service, and you will need to know the "Coverity token", which is a
+# secret 8 digit hex string. You can find that from the web UI in the
+# project settings, if you have maintainer access there.
+
+# Command line options:
+#   --dry-run : run the tools, but don't actually do the upload
+#   --update-tools-only : update the cached copy of the tools, but don't run them
+#   --tokenfile : file to read Coverity token from
+#   --version ver : specify version being analyzed (default: ask git)
+#   --description desc : specify description of this version (default: ask git)
+#   --srcdir : QEMU source tree to analyze (default: current working dir)
+#   --results-tarball : path to copy the results tarball to (default: don't
+#                       copy it anywhere, just upload it)
+#
+# User-specifiable environment variables:
+#  COVERITY_TOKEN -- Coverity token
+#  COVERITY_EMAIL -- the email address to use for uploads (default:
+#                    looks at your git user.email config)
+#  COVERITY_BUILD_CMD -- make command (default: 'make -jN' where N is
+#                    number of CPUs as determined by 'nproc')
+#  COVERITY_TOOL_BASE -- set to directory to put coverity tools
+#                        (default: /tmp/coverity-tools)
+#
+# You must specify the token, either by environment variable or by
+# putting it in a file and using --tokenfile. Everything else has
+# a reasonable default if this is run from a git tree.
+
+check_upload_permissions() {
+    # Check whether we can do an upload to the server; will exit the script
+    # with status 1 if the check failed (usually a bad token);
+    # will exit the script with status 0 if the check indicated that we
+    # can't upload yet (ie we are at quota)
+    # Assumes that PROJTOKEN, PROJNAME and DRYRUN have been initialized.
+
+    echo "Checking upload permissions..."
+
+    if ! up_perm="$(wget https://scan.coverity.com/api/upload_permitted --post-data "token=$PROJTOKEN&project=$PROJNAME" -q -O -)"; then
+        echo "Coverity Scan API access denied: bad token?"
+        exit 1
+    fi
+
+    # Really up_perm is a JSON response with either
+    # {upload_permitted:true} or {next_upload_permitted_at:<date>}
+    # We do some hacky string parsing instead of properly parsing it.
+    case "$up_perm" in
+        *upload_permitted*true*)
+            echo "Coverity Scan: upload permitted"
+            ;;
+        *next_upload_permitted_at*)
+            if [ "$DRYRUN" = yes ]; then
+                echo "Coverity Scan: upload quota reached, continuing dry run"
+            else
+                echo "Coverity Scan: upload quota reached; stopping here"
+                # Exit success as this isn't a build error.
+                exit 0
+            fi
+            ;;
+        *)
+            echo "Coverity Scan upload check: unexpected result $up_perm"
+            exit 1
+            ;;
+    esac
+}
+
+
+update_coverity_tools () {
+    # Check for whether we need to download the Coverity tools
+    # (either because we don't have a copy, or because it's out of date)
+    # Assumes that COVERITY_TOOL_BASE, PROJTOKEN and PROJNAME are set.
+
+    mkdir -p "$COVERITY_TOOL_BASE"
+    cd "$COVERITY_TOOL_BASE"
+
+    echo "Checking for new version of coverity build tools..."
+    wget https://scan.coverity.com/download/linux64 --post-data "token=$PROJTOKEN&project=$PROJNAME&md5=1" -O coverity_tool.md5.new
+
+    if ! cmp -s coverity_tool.md5 coverity_tool.md5.new; then
+        # out of date md5 or no md5: download new build tool
+        # blow away the old build tool
+        echo "Downloading coverity build tools..."
+        rm -rf coverity_tool coverity_tool.tgz
+        wget https://scan.coverity.com/download/linux64 --post-data "token=$PROJTOKEN&project=$PROJNAME" -O coverity_tool.tgz
+        if ! (cat coverity_tool.md5.new; echo "  coverity_tool.tgz") | md5sum -c --status; then
+            echo "Downloaded tarball didn't match md5sum!"
+            exit 1
+        fi
+        # extract the new one, keeping it corralled in a 'coverity_tool' directory
+        echo "Unpacking coverity build tools..."
+        mkdir -p coverity_tool
+        cd coverity_tool
+        tar xf ../coverity_tool.tgz
+        cd ..
+        mv coverity_tool.md5.new coverity_tool.md5
+    fi
+
+    rm -f coverity_tool.md5.new
+}
+
+
+# Check user-provided environment variables and arguments
+DRYRUN=no
+UPDATE_ONLY=no
+
+while [ "$#" -ge 1 ]; do
+    case "$1" in
+        --dry-run)
+            shift
+            DRYRUN=yes
+            ;;
+        --update-tools-only)
+            shift
+            UPDATE_ONLY=yes
+            ;;
+        --version)
+            shift
+            if [ $# -eq 0 ]; then
+                echo "--version needs an argument"
+                exit 1
+            fi
+            VERSION="$1"
+            shift
+            ;;
+        --description)
+            shift
+            if [ $# -eq 0 ]; then
+                echo "--description needs an argument"
+                exit 1
+            fi
+            DESCRIPTION="$1"
+            shift
+            ;;
+        --tokenfile)
+            shift
+            if [ $# -eq 0 ]; then
+                echo "--tokenfile needs an argument"
+                exit 1
+            fi
+            COVERITY_TOKEN="$(cat "$1")"
+            shift
+            ;;
+        --srcdir)
+            shift
+            if [ $# -eq 0 ]; then
+                echo "--srcdir needs an argument"
+                exit 1
+            fi
+            SRCDIR="$1"
+            shift
+            ;;
+        --results-tarball)
+            shift
+            if [ $# -eq 0 ]; then
+                echo "--results-tarball needs an argument"
+                exit 1
+            fi
+            RESULTSTARBALL="$1"
+            shift
+            ;;
+        *)
+            echo "Unexpected argument '$1'"
+            exit 1
+            ;;
+    esac
+done
+
+if [ -z "$COVERITY_TOKEN" ]; then
+    echo "COVERITY_TOKEN environment variable not set"
+    exit 1
+fi
+
+if [ -z "$COVERITY_BUILD_CMD" ]; then
+    NPROC=$(nproc)
+    COVERITY_BUILD_CMD="make -j$NPROC"
+    echo "COVERITY_BUILD_CMD: using default '$COVERITY_BUILD_CMD'"
+fi
+
+if [ -z "$COVERITY_TOOL_BASE" ]; then
+    echo "COVERITY_TOOL_BASE: using default /tmp/coverity-tools"
+    COVERITY_TOOL_BASE=/tmp/coverity-tools
+fi
+
+if [ -z "$SRCDIR" ]; then
+    SRCDIR="$PWD"
+fi
+
+PROJTOKEN="$COVERITY_TOKEN"
+PROJNAME=QEMU
+TARBALL=cov-int.tar.xz
+
+
+if [ "$UPDATE_ONLY" = yes ]; then
+    # Just do the tools update; we don't need to check whether
+    # we are in a source tree or have upload rights for this,
+    # so do it before some of the command line and source tree checks.
+    update_coverity_tools
+    exit 0
+fi
+
+cd "$SRCDIR"
+
+echo "Checking this is a QEMU source tree..."
+if ! [ -e "$SRCDIR/VERSION" ]; then
+    echo "Not in a QEMU source tree?"
+    exit 1
+fi
+
+# Fill in defaults used by the non-update-only process
+if [ -z "$VERSION" ]; then
+    VERSION="$(git describe --always HEAD)"
+fi
+
+if [ -z "$DESCRIPTION" ]; then
+    DESCRIPTION="$(git rev-parse HEAD)"
+fi
+
+if [ -z "$COVERITY_EMAIL" ]; then
+    COVERITY_EMAIL="$(git config user.email)"
+fi
+
+check_upload_permissions
+
+update_coverity_tools
+
+TOOLBIN="$(cd "$COVERITY_TOOL_BASE" && echo $PWD/coverity_tool/cov-analysis-*/bin)"
+
+if ! test -x "$TOOLBIN/cov-build"; then
+    echo "Couldn't find cov-build in the coverity build-tool directory??"
+    exit 1
+fi
+
+export PATH="$TOOLBIN:$PATH"
+
+cd "$SRCDIR"
+
+echo "Doing make distclean..."
+make distclean
+
+echo "Configuring..."
+# We configure with a fixed set of enables here to ensure that we don't
+# accidentally reduce the scope of the analysis by doing the build on
+# the system that's missing a dependency that we need to build part of
+# the codebase.
+./configure --disable-modules --enable-sdl --enable-gtk \
+    --enable-opengl --enable-vte --enable-gnutls \
+    --enable-nettle --enable-curses --enable-curl \
+    --audio-drv-list=oss,alsa,sdl,pa --enable-virtfs \
+    --enable-vnc --enable-vnc-sasl --enable-vnc-jpeg --enable-vnc-png \
+    --enable-xen --enable-brlapi \
+    --enable-linux-aio --enable-attr \
+    --enable-cap-ng --enable-trace-backends=log --enable-spice --enable-rbd \
+    --enable-xfsctl --enable-libusb --enable-usb-redir \
+    --enable-libiscsi --enable-libnfs --enable-seccomp \
+    --enable-tpm --enable-libssh --enable-lzo --enable-snappy --enable-bzip2 \
+    --enable-numa --enable-rdma --enable-smartcard --enable-virglrenderer \
+    --enable-mpath --enable-libxml2 --enable-glusterfs \
+    --enable-virtfs --enable-zstd
+
+echo "Making libqemustub.a..."
+make libqemustub.a
+
+echo "Running cov-build..."
+rm -rf cov-int
+mkdir cov-int
+cov-build --dir cov-int $COVERITY_BUILD_CMD
+
+echo "Creating results tarball..."
+tar cvf - cov-int | xz > "$TARBALL"
+
+if [ ! -z "$RESULTSTARBALL" ]; then
+    echo "Copying results tarball to $RESULTSTARBALL..."
+    cp "$TARBALL" "$RESULTSTARBALL"
+fi
+
+echo "Uploading results tarball..."
+
+if [ "$DRYRUN" = yes ]; then
+    echo "Dry run only, not uploading $TARBALL"
+    exit 0
+fi
+
+curl --form token="$PROJTOKEN" --form email="$COVERITY_EMAIL" \
+     --form file=@"$TARBALL" --form version="$VERSION" \
+     --form description="$DESCRIPTION" \
+     https://scan.coverity.com/builds?project="$PROJNAME"
+
+echo "Done."
-- 
2.20.1

[PATCH v2 6/6] scripts/coverity-scan: Add Docker support

[Peter Maydell]

Add support for running the Coverity Scan tools inside a Docker
container rather than directly on the host system.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
v1->v2:
 * various bug fixes
 * added --src-tarball rather than putting the whole source
   tree in the 'secrets' directory
 * docker file package list updated
---
 scripts/coverity-scan/coverity-scan.docker | 131 +++++++++++++++++++++
 scripts/coverity-scan/run-coverity-scan    |  90 ++++++++++++++
 2 files changed, 221 insertions(+)
 create mode 100644 scripts/coverity-scan/coverity-scan.docker

diff --git a/scripts/coverity-scan/coverity-scan.docker b/scripts/coverity-scan/coverity-scan.docker
new file mode 100644
index 00000000000..a4f64d12834
--- /dev/null
+++ b/scripts/coverity-scan/coverity-scan.docker
@@ -0,0 +1,131 @@
+# syntax=docker/dockerfile:1.0.0-experimental
+#
+# Docker setup for running the "Coverity Scan" tools over the source
+# tree and uploading them to the website, as per
+# https://scan.coverity.com/projects/qemu/builds/new
+# We do this on a fixed config (currently Fedora 30 with a known
+# set of dependencies and a configure command that enables a specific
+# set of options) so that random changes don't result in our accidentally
+# dropping some files from the scan.
+#
+# We don't build on top of the fedora.docker file because we don't
+# want to accidentally change or break the scan config when that
+# is updated.
+
+# The work of actually doing the build is handled by the
+# run-coverity-scan script.
+
+FROM fedora:30
+ENV PACKAGES \
+    alsa-lib-devel \
+    bc \
+    bison \
+    brlapi-devel \
+    bzip2 \
+    bzip2-devel \
+    ccache \
+    clang \
+    curl \
+    cyrus-sasl-devel \
+    dbus-daemon \
+    device-mapper-multipath-devel \
+    findutils \
+    flex \
+    gcc \
+    gcc-c++ \
+    gettext \
+    git \
+    glib2-devel \
+    glusterfs-api-devel \
+    gnutls-devel \
+    gtk3-devel \
+    hostname \
+    libaio-devel \
+    libasan \
+    libattr-devel \
+    libblockdev-mpath-devel \
+    libcap-devel \
+    libcap-ng-devel \
+    libcurl-devel \
+    libepoxy-devel \
+    libfdt-devel \
+    libgbm-devel \
+    libiscsi-devel \
+    libjpeg-devel \
+    libpmem-devel \
+    libnfs-devel \
+    libpng-devel \
+    librbd-devel \
+    libseccomp-devel \
+    libssh-devel \
+    libubsan \
+    libudev-devel \
+    libusbx-devel \
+    libxml2-devel \
+    libzstd-devel \
+    llvm \
+    lzo-devel \
+    make \
+    mingw32-bzip2 \
+    mingw32-curl \
+    mingw32-glib2 \
+    mingw32-gmp \
+    mingw32-gnutls \
+    mingw32-gtk3 \
+    mingw32-libjpeg-turbo \
+    mingw32-libpng \
+    mingw32-libtasn1 \
+    mingw32-nettle \
+    mingw32-nsis \
+    mingw32-pixman \
+    mingw32-pkg-config \
+    mingw32-SDL2 \
+    mingw64-bzip2 \
+    mingw64-curl \
+    mingw64-glib2 \
+    mingw64-gmp \
+    mingw64-gnutls \
+    mingw64-gtk3 \
+    mingw64-libjpeg-turbo \
+    mingw64-libpng \
+    mingw64-libtasn1 \
+    mingw64-nettle \
+    mingw64-pixman \
+    mingw64-pkg-config \
+    mingw64-SDL2 \
+    ncurses-devel \
+    nettle-devel \
+    nss-devel \
+    numactl-devel \
+    perl \
+    perl-Test-Harness \
+    pixman-devel \
+    pulseaudio-libs-devel \
+    python3 \
+    python3-sphinx \
+    PyYAML \
+    rdma-core-devel \
+    SDL2-devel \
+    snappy-devel \
+    sparse \
+    spice-server-devel \
+    systemd-devel \
+    systemtap-sdt-devel \
+    tar \
+    texinfo \
+    usbredir-devel \
+    virglrenderer-devel \
+    vte291-devel \
+    wget \
+    which \
+    xen-devel \
+    xfsprogs-devel \
+    zlib-devel
+ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
+
+RUN dnf install -y $PACKAGES
+RUN rpm -q $PACKAGES | sort > /packages.txt
+ENV PATH $PATH:/usr/libexec/python3-sphinx/
+ENV COVERITY_TOOL_BASE=/coverity-tools
+COPY run-coverity-scan run-coverity-scan
+RUN --mount=type=secret,id=coverity.token,required ./run-coverity-scan --update-tools-only --tokenfile /run/secrets/coverity.token
diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
index d40b51969fa..2e067ef5cfc 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -29,6 +29,7 @@
 
 # Command line options:
 #   --dry-run : run the tools, but don't actually do the upload
+#   --docker : create and work inside a docker container
 #   --update-tools-only : update the cached copy of the tools, but don't run them
 #   --tokenfile : file to read Coverity token from
 #   --version ver : specify version being analyzed (default: ask git)
@@ -36,6 +37,8 @@
 #   --srcdir : QEMU source tree to analyze (default: current working dir)
 #   --results-tarball : path to copy the results tarball to (default: don't
 #                       copy it anywhere, just upload it)
+#   --src-tarball : tarball to untar into src dir (default: none); this
+#                   is intended mainly for internal use by the Docker support
 #
 # User-specifiable environment variables:
 #  COVERITY_TOKEN -- Coverity token
@@ -125,6 +128,7 @@ update_coverity_tools () {
 # Check user-provided environment variables and arguments
 DRYRUN=no
 UPDATE_ONLY=no
+DOCKER=no
 
 while [ "$#" -ge 1 ]; do
     case "$1" in
@@ -181,6 +185,19 @@ while [ "$#" -ge 1 ]; do
             RESULTSTARBALL="$1"
             shift
             ;;
+        --src-tarball)
+            shift
+            if [ $# -eq 0 ]; then
+                echo "--src-tarball needs an argument"
+                exit 1
+            fi
+            SRCTARBALL="$1"
+            shift
+            ;;
+        --docker)
+            DOCKER=yes
+            shift
+            ;;
         *)
             echo "Unexpected argument '$1'"
             exit 1
@@ -212,6 +229,10 @@ PROJTOKEN="$COVERITY_TOKEN"
 PROJNAME=QEMU
 TARBALL=cov-int.tar.xz
 
+if [ "$UPDATE_ONLY" = yes ] && [ "$DOCKER" = yes ]; then
+    echo "Combining --docker and --update-only is not supported"
+    exit 1
+fi
 
 if [ "$UPDATE_ONLY" = yes ]; then
     # Just do the tools update; we don't need to check whether
@@ -221,8 +242,17 @@ if [ "$UPDATE_ONLY" = yes ]; then
     exit 0
 fi
 
+if [ ! -e "$SRCDIR" ]; then
+    mkdir "$SRCDIR"
+fi
+
 cd "$SRCDIR"
 
+if [ ! -z "$SRCTARBALL" ]; then
+    echo "Untarring source tarball into $SRCDIR..."
+    tar xvf "$SRCTARBALL"
+fi
+
 echo "Checking this is a QEMU source tree..."
 if ! [ -e "$SRCDIR/VERSION" ]; then
     echo "Not in a QEMU source tree?"
@@ -242,6 +272,66 @@ if [ -z "$COVERITY_EMAIL" ]; then
     COVERITY_EMAIL="$(git config user.email)"
 fi
 
+# Run ourselves inside docker if that's what the user wants
+if [ "$DOCKER" = yes ]; then
+    # build docker container including the coverity-scan tools
+    # Put the Coverity token into a temporary file that only
+    # we have read access to, and then pass it to docker build
+    # using --secret. This requires at least Docker 18.09.
+    # Mostly what we are trying to do here is ensure we don't leak
+    # the token into the Docker image.
+    umask 077
+    SECRETDIR=$(mktemp -d)
+    if [ -z "$SECRETDIR" ]; then
+        echo "Failed to create temporary directory"
+        exit 1
+    fi
+    trap 'rm -rf "$SECRETDIR"' INT TERM EXIT
+    echo "Created temporary directory $SECRETDIR"
+    SECRET="$SECRETDIR/token"
+    echo "$COVERITY_TOKEN" > "$SECRET"
+    echo "Building docker container..."
+    # TODO: This re-downloads the tools every time, rather than
+    # caching and reusing the image produced with the downloaded tools.
+    # Not sure why.
+    # TODO: how do you get 'docker build' to print the output of the
+    # commands it is running to its stdout? This would be useful for debug.
+    DOCKER_BUILDKIT=1 docker build -t coverity-scanner \
+                   --secret id=coverity.token,src="$SECRET" \
+                   -f scripts/coverity-scan/coverity-scan.docker \
+                   scripts/coverity-scan
+    echo "Archiving sources to be analyzed..."
+    ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
+    if [ "$DRYRUN" = yes ]; then
+        DRYRUNARG=--dry-run
+    fi
+    echo "Running scanner..."
+    # If we need to capture the output tarball, get the inner run to
+    # save it to the secrets directory so we can copy it out before the
+    # directory is cleaned up.
+    if [ ! -z "$RESULTSTARBALL" ]; then
+        RTARGS="--results-tarball /work/cov-int.tar.xz"
+    else
+        RTARGS=""
+    fi
+    # Arrange for this docker run to get access to the sources with -v.
+    # We pass through all the configuration from the outer script to the inner.
+    export COVERITY_EMAIL COVERITY_BUILD_CMD
+    docker run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
+           -v "$SECRETDIR:/work" coverity-scanner \
+           ./run-coverity-scan --version "$VERSION" \
+           --description "$DESCRIPTION" $DRYRUNARG --tokenfile /work/token \
+           --srcdir /qemu --src-tarball /work/qemu-sources.tgz $RTARGS
+    if [ ! -z "$RESULTSTARBALL" ]; then
+        echo "Copying results tarball to $RESULTSTARBALL..."
+        cp "$SECRETDIR/cov-int.tar.xz" "$RESULTSTARBALL"
+    fi
+    echo "Docker work complete."
+    exit 0
+fi
+
+# Otherwise, continue with the full build and upload process.
+
 check_upload_permissions
 
 update_coverity_tools
-- 
2.20.1

Re: [PATCH v2 1/6] osdep.h: Drop no-longer-needed Coverity workarounds

[Richard Henderson]

On 3/19/20 12:33 PM, Peter Maydell wrote:
> In commit a1a98357e3fd in 2018 we added some workarounds for Coverity
> not being able to handle the _Float* types introduced by recent
> glibc.  Newer versions of the Coverity scan tools have support for
> these types, and will fail with errors about duplicate typedefs if we
> have our workaround.  Remove our copy of the typedefs.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 2/6] thread.h: Fix Coverity version of qemu_cond_timedwait()

[Richard Henderson]

On 3/19/20 12:33 PM, Peter Maydell wrote:
> For Coverity's benefit, we provide simpler versions of functions like
> qemu_mutex_lock(), qemu_cond_wait() and qemu_cond_timedwait().  When
> we added qemu_cond_timedwait() in commit 3dcc9c6ec4ea, a cut and
> paste error meant that the Coverity version of qemu_cond_timedwait()
> was using the wrong _impl function, which makes the Coverity parser
> complain:
> 
> "/qemu/include/qemu/thread.h", line 159: warning #140: too many arguments in
>           function call
>       return qemu_cond_timedwait(cond, mutex, ms);
>              ^
> 
> "/qemu/include/qemu/thread.h", line 159: warning #120: return value type does
>           not match the function type
>       return qemu_cond_timedwait(cond, mutex, ms);
>              ^
> 
> "/qemu/include/qemu/thread.h", line 156: warning #1563: function
>           "qemu_cond_timedwait" not emitted, consider modeling it or review
>           parse diagnostics to improve fidelity
>   static inline bool (qemu_cond_timedwait)(QemuCond *cond, QemuMutex *mutex,
>                       ^
> 
> These aren't fatal, but reduce the scope of the analysis. Fix the error.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/thread.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 3/6] thread.h: Remove trailing semicolons from Coverity qemu_mutex_lock() etc

[Richard Henderson]

On 3/19/20 12:33 PM, Peter Maydell wrote:
> All the Coverity-specific definitions of qemu_mutex_lock() and friends
> have a trailing semicolon. This works fine almost everywhere because
> of QEMU's mandatory-braces coding style and because most callsites are
> simple, but target/s390x/sigp.c has a use of qemu_mutex_trylock() as
> an if() statement, which makes the ';' a syntax error:
> "../target/s390x/sigp.c", line 461: warning #18: expected a ")"
>       if (qemu_mutex_trylock(&qemu_sigp_mutex)) {
>           ^
> 
> Remove the bogus semicolons from the macro definitions.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/qemu/thread.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 4/6] linux-user/flatload.c: Use "" for include of QEMU header target_flat.h

[Richard Henderson]

On 3/19/20 12:33 PM, Peter Maydell wrote:
> The target_flat.h file is a QEMU header, so we should include it using
> quotes, not angle brackets.
> 
> Coverity otherwise is unable to find the header:
> 
> "../linux-user/flatload.c", line 40: error #1712: cannot open source file
>           "target_flat.h"
>   #include <target_flat.h>
>                           ^
> 
> because the relevant directory is only on the -iquote path, not the -I path.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I don't know why Coverity in particular has trouble here but
> real compilers don't. Still, the "" is the right thing.
> ---
>  linux-user/flatload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 6/6] scripts/coverity-scan: Add Docker support

[Paolo Bonzini]

On 19/03/20 20:33, Peter Maydell wrote:
> +    # TODO: how do you get 'docker build' to print the output of the
> +    # commands it is running to its stdout? This would be useful for debug.
> +    DOCKER_BUILDKIT=1 docker build -t coverity-scanner \
> +                   --secret id=coverity.token,src="$SECRET" \
> +                   -f scripts/coverity-scan/coverity-scan.docker \
> +                   scripts/coverity-scan

I'm not sure but tests/docker/docker.py should do it.  I'll test this
next week.

Paolo

Re: [PATCH v2 3/6] thread.h: Remove trailing semicolons from Coverity qemu_mutex_lock() etc

[Philippe Mathieu-Daudé]

On 3/19/20 8:33 PM, Peter Maydell wrote:
> All the Coverity-specific definitions of qemu_mutex_lock() and friends
> have a trailing semicolon. This works fine almost everywhere because
> of QEMU's mandatory-braces coding style and because most callsites are
> simple, but target/s390x/sigp.c has a use of qemu_mutex_trylock() as
> an if() statement, which makes the ';' a syntax error:
> "../target/s390x/sigp.c", line 461: warning #18: expected a ")"
>        if (qemu_mutex_trylock(&qemu_sigp_mutex)) {
>            ^
> 
> Remove the bogus semicolons from the macro definitions.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/qemu/thread.h | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index 10262c63f58..d22848138ea 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -57,17 +57,17 @@ extern QemuCondTimedWaitFunc qemu_cond_timedwait_func;
>    * hide them.
>    */
>   #define qemu_mutex_lock(m)                                              \
> -            qemu_mutex_lock_impl(m, __FILE__, __LINE__);
> +            qemu_mutex_lock_impl(m, __FILE__, __LINE__)
>   #define qemu_mutex_trylock(m)                                           \
> -            qemu_mutex_trylock_impl(m, __FILE__, __LINE__);
> +            qemu_mutex_trylock_impl(m, __FILE__, __LINE__)
>   #define qemu_rec_mutex_lock(m)                                          \
> -            qemu_rec_mutex_lock_impl(m, __FILE__, __LINE__);
> +            qemu_rec_mutex_lock_impl(m, __FILE__, __LINE__)
>   #define qemu_rec_mutex_trylock(m)                                       \
> -            qemu_rec_mutex_trylock_impl(m, __FILE__, __LINE__);
> +            qemu_rec_mutex_trylock_impl(m, __FILE__, __LINE__)
>   #define qemu_cond_wait(c, m)                                            \
> -            qemu_cond_wait_impl(c, m, __FILE__, __LINE__);
> +            qemu_cond_wait_impl(c, m, __FILE__, __LINE__)
>   #define qemu_cond_timedwait(c, m, ms)                                   \
> -            qemu_cond_timedwait_impl(c, m, ms, __FILE__, __LINE__);
> +            qemu_cond_timedwait_impl(c, m, ms, __FILE__, __LINE__)
>   #else
>   #define qemu_mutex_lock(m) ({                                           \
>               QemuMutexLockFunc _f = atomic_read(&qemu_mutex_lock_func);  \
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 4/6] linux-user/flatload.c: Use "" for include of QEMU header target_flat.h

[Philippe Mathieu-Daudé]

On 3/19/20 8:33 PM, Peter Maydell wrote:
> The target_flat.h file is a QEMU header, so we should include it using
> quotes, not angle brackets.
> 
> Coverity otherwise is unable to find the header:
> 
> "../linux-user/flatload.c", line 40: error #1712: cannot open source file
>            "target_flat.h"
>    #include <target_flat.h>
>                            ^
> 
> because the relevant directory is only on the -iquote path, not the -I path.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I don't know why Coverity in particular has trouble here but
> real compilers don't. Still, the "" is the right thing.
> ---
>   linux-user/flatload.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> index 0122ab3afe6..66901f39cc5 100644
> --- a/linux-user/flatload.c
> +++ b/linux-user/flatload.c
> @@ -37,7 +37,7 @@
>   
>   #include "qemu.h"
>   #include "flat.h"
> -#include <target_flat.h>
> +#include "target_flat.h"
>   
>   //#define DEBUG
>   
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 2/6] thread.h: Fix Coverity version of qemu_cond_timedwait()

[Philippe Mathieu-Daudé]

On 3/19/20 8:33 PM, Peter Maydell wrote:
> For Coverity's benefit, we provide simpler versions of functions like
> qemu_mutex_lock(), qemu_cond_wait() and qemu_cond_timedwait().  When
> we added qemu_cond_timedwait() in commit 3dcc9c6ec4ea, a cut and
> paste error meant that the Coverity version of qemu_cond_timedwait()
> was using the wrong _impl function, which makes the Coverity parser
> complain:
> 
> "/qemu/include/qemu/thread.h", line 159: warning #140: too many arguments in
>            function call
>        return qemu_cond_timedwait(cond, mutex, ms);
>               ^
> 
> "/qemu/include/qemu/thread.h", line 159: warning #120: return value type does
>            not match the function type
>        return qemu_cond_timedwait(cond, mutex, ms);
>               ^
> 
> "/qemu/include/qemu/thread.h", line 156: warning #1563: function
>            "qemu_cond_timedwait" not emitted, consider modeling it or review
>            parse diagnostics to improve fidelity
>    static inline bool (qemu_cond_timedwait)(QemuCond *cond, QemuMutex *mutex,
>                        ^
> 
> These aren't fatal, but reduce the scope of the analysis. Fix the error.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/qemu/thread.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qemu/thread.h b/include/qemu/thread.h
> index 047db0307e7..10262c63f58 100644
> --- a/include/qemu/thread.h
> +++ b/include/qemu/thread.h
> @@ -67,7 +67,7 @@ extern QemuCondTimedWaitFunc qemu_cond_timedwait_func;
>   #define qemu_cond_wait(c, m)                                            \
>               qemu_cond_wait_impl(c, m, __FILE__, __LINE__);
>   #define qemu_cond_timedwait(c, m, ms)                                   \
> -            qemu_cond_wait_impl(c, m, ms, __FILE__, __LINE__);
> +            qemu_cond_timedwait_impl(c, m, ms, __FILE__, __LINE__);
>   #else
>   #define qemu_mutex_lock(m) ({                                           \
>               QemuMutexLockFunc _f = atomic_read(&qemu_mutex_lock_func);  \
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 0/6] Automation of Coverity Scan uploads (via Docker)

[Peter Maydell]

What's your view on this series, Paolo? Personally I'd like
to put it into master, because at least then we have something
that we can do Coverity runs on, whereas AIUI at the moment
we don't. But I'd rather not put it in after rc3, which is
tomorrow...

thanks
-- PMM

On Thu, 19 Mar 2020 at 19:33, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> v1 of this series was over a year ago:
> https://patchew.org/QEMU/20181113184641.4492-1-peter.maydell@linaro.org/
>
> I dusted it off and fixed some stuff because Paolo reports that the
> machine he was previously using for uploads can't run the Coverity
> tools any more.
>
> The first four patches are fixes for problems that cause the Coverity
> tool not to be able to scan everything.  The first one in particular
> meant that every compilation unit failed, which would block uploads.
> The other 3 would reduce the scan coverage but weren't fatal.  (The
> only remaining warnings in the log are where Coverity complains about
> asm intrinsics system headers.)
>
> With these scripts you can do an upload with
> COVERITY_TOKEN=nnnnnnnnn ./scripts/coverity-scan/run-coverity-scan --docker
> (where nnnnnnnn is the project's secret token which admins can
> get from the Coverity web UI).
>
> I did in fact do an upload to test it, so the currently visible
> results on the website are the result of a scan on ce73691e258 plus
> this series.
>
> The new upload has +112 defects, which is quite a lot, but I don't
> think it's so many that it is "defects we rejected as false positives
> coming back again"; my guess is a combination of the fixes in the
> first 4 patches increasing coverage plus we haven't run a test in a
> while plus maybe the script has some more config options enabled that
> Paolo's box did not.  (In the web UI defects that were dismissed as
> FPs seem still to be considered present-but-dismissed, so I think
> that's OK.)
>
> Not much has changed since v1; I didn't get very much feedback
> the first time around[*]. Docker still seems to do the "download
> the Coverity tools" part more often than I expect. On the other
> hand "actually automated with a script in the tree" beats "not
> automated and currently broken" so maybe this patchset as it
> stands is good enough, given that basically 1 or 2 people ever
> will be running the script ?
>
> [*] Eric will note that yes, the script still uses set -e.
>
> (Like v1 this doesn't try to tie it into Travis, but we could
> in theory do that some day, or have some other automated once
> a week run of the script.)
>
> thanks
> -- PMM
>
> Peter Maydell (6):
>   osdep.h: Drop no-longer-needed Coverity workarounds
>   thread.h: Fix Coverity version of qemu_cond_timedwait()
>   thread.h: Remove trailing semicolons from Coverity qemu_mutex_lock()
>     etc
>   linux-user/flatload.c: Use "" for include of QEMU header target_flat.h
>   scripts/run-coverity-scan: Script to run Coverity Scan build
>   scripts/coverity-scan: Add Docker support
>
>  include/qemu/osdep.h                       |  14 -
>  include/qemu/thread.h                      |  12 +-
>  linux-user/flatload.c                      |   2 +-
>  MAINTAINERS                                |   5 +
>  scripts/coverity-scan/coverity-scan.docker | 131 +++++++
>  scripts/coverity-scan/run-coverity-scan    | 401 +++++++++++++++++++++
>  6 files changed, 544 insertions(+), 21 deletions(-)
>  create mode 100644 scripts/coverity-scan/coverity-scan.docker
>  create mode 100755 scripts/coverity-scan/run-coverity-scan
>
> --
> 2.20.1

Re: [PATCH v2 0/6] Automation of Coverity Scan uploads (via Docker)

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 3960 bytes --]

Yes, go ahead. I would like to add a docker-coverity Makefile target but I
can do that later.

Il lun 13 apr 2020, 14:13 Peter Maydell <peter.maydell@linaro.org> ha
scritto:

> What's your view on this series, Paolo? Personally I'd like
> to put it into master, because at least then we have something
> that we can do Coverity runs on, whereas AIUI at the moment
> we don't. But I'd rather not put it in after rc3, which is
> tomorrow...
>
> thanks
> -- PMM
>
> On Thu, 19 Mar 2020 at 19:33, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >
> > v1 of this series was over a year ago:
> > https://patchew.org/QEMU/20181113184641.4492-1-peter.maydell@linaro.org/
> >
> > I dusted it off and fixed some stuff because Paolo reports that the
> > machine he was previously using for uploads can't run the Coverity
> > tools any more.
> >
> > The first four patches are fixes for problems that cause the Coverity
> > tool not to be able to scan everything.  The first one in particular
> > meant that every compilation unit failed, which would block uploads.
> > The other 3 would reduce the scan coverage but weren't fatal.  (The
> > only remaining warnings in the log are where Coverity complains about
> > asm intrinsics system headers.)
> >
> > With these scripts you can do an upload with
> > COVERITY_TOKEN=nnnnnnnnn ./scripts/coverity-scan/run-coverity-scan
> --docker
> > (where nnnnnnnn is the project's secret token which admins can
> > get from the Coverity web UI).
> >
> > I did in fact do an upload to test it, so the currently visible
> > results on the website are the result of a scan on ce73691e258 plus
> > this series.
> >
> > The new upload has +112 defects, which is quite a lot, but I don't
> > think it's so many that it is "defects we rejected as false positives
> > coming back again"; my guess is a combination of the fixes in the
> > first 4 patches increasing coverage plus we haven't run a test in a
> > while plus maybe the script has some more config options enabled that
> > Paolo's box did not.  (In the web UI defects that were dismissed as
> > FPs seem still to be considered present-but-dismissed, so I think
> > that's OK.)
> >
> > Not much has changed since v1; I didn't get very much feedback
> > the first time around[*]. Docker still seems to do the "download
> > the Coverity tools" part more often than I expect. On the other
> > hand "actually automated with a script in the tree" beats "not
> > automated and currently broken" so maybe this patchset as it
> > stands is good enough, given that basically 1 or 2 people ever
> > will be running the script ?
> >
> > [*] Eric will note that yes, the script still uses set -e.
> >
> > (Like v1 this doesn't try to tie it into Travis, but we could
> > in theory do that some day, or have some other automated once
> > a week run of the script.)
> >
> > thanks
> > -- PMM
> >
> > Peter Maydell (6):
> >   osdep.h: Drop no-longer-needed Coverity workarounds
> >   thread.h: Fix Coverity version of qemu_cond_timedwait()
> >   thread.h: Remove trailing semicolons from Coverity qemu_mutex_lock()
> >     etc
> >   linux-user/flatload.c: Use "" for include of QEMU header target_flat.h
> >   scripts/run-coverity-scan: Script to run Coverity Scan build
> >   scripts/coverity-scan: Add Docker support
> >
> >  include/qemu/osdep.h                       |  14 -
> >  include/qemu/thread.h                      |  12 +-
> >  linux-user/flatload.c                      |   2 +-
> >  MAINTAINERS                                |   5 +
> >  scripts/coverity-scan/coverity-scan.docker | 131 +++++++
> >  scripts/coverity-scan/run-coverity-scan    | 401 +++++++++++++++++++++
> >  6 files changed, 544 insertions(+), 21 deletions(-)
> >  create mode 100644 scripts/coverity-scan/coverity-scan.docker
> >  create mode 100755 scripts/coverity-scan/run-coverity-scan
> >
> > --
> > 2.20.1
>
>

[-- Attachment #2: Type: text/html, Size: 4998 bytes --]

Re: [PATCH v2 6/6] scripts/coverity-scan: Add Docker support

[Philippe Mathieu-Daudé]

On 3/19/20 8:33 PM, Peter Maydell wrote:
> Add support for running the Coverity Scan tools inside a Docker
> container rather than directly on the host system.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> v1->v2:
>   * various bug fixes
>   * added --src-tarball rather than putting the whole source
>     tree in the 'secrets' directory
>   * docker file package list updated
> ---
>   scripts/coverity-scan/coverity-scan.docker | 131 +++++++++++++++++++++
>   scripts/coverity-scan/run-coverity-scan    |  90 ++++++++++++++
>   2 files changed, 221 insertions(+)
>   create mode 100644 scripts/coverity-scan/coverity-scan.docker
> 
> diff --git a/scripts/coverity-scan/coverity-scan.docker b/scripts/coverity-scan/coverity-scan.docker
> new file mode 100644
> index 00000000000..a4f64d12834
> --- /dev/null
> +++ b/scripts/coverity-scan/coverity-scan.docker
> @@ -0,0 +1,131 @@
> +# syntax=docker/dockerfile:1.0.0-experimental
> +#
> +# Docker setup for running the "Coverity Scan" tools over the source
> +# tree and uploading them to the website, as per
> +# https://scan.coverity.com/projects/qemu/builds/new
> +# We do this on a fixed config (currently Fedora 30 with a known
> +# set of dependencies and a configure command that enables a specific
> +# set of options) so that random changes don't result in our accidentally
> +# dropping some files from the scan.
> +#
> +# We don't build on top of the fedora.docker file because we don't
> +# want to accidentally change or break the scan config when that
> +# is updated.
> +
> +# The work of actually doing the build is handled by the
> +# run-coverity-scan script.
> +
> +FROM fedora:30
> +ENV PACKAGES \
> +    alsa-lib-devel \
> +    bc \
> +    bison \
> +    brlapi-devel \
> +    bzip2 \
> +    bzip2-devel \
> +    ccache \
> +    clang \
> +    curl \
> +    cyrus-sasl-devel \
> +    dbus-daemon \
> +    device-mapper-multipath-devel \
> +    findutils \
> +    flex \
> +    gcc \
> +    gcc-c++ \
> +    gettext \
> +    git \
> +    glib2-devel \
> +    glusterfs-api-devel \
> +    gnutls-devel \
> +    gtk3-devel \
> +    hostname \
> +    libaio-devel \
> +    libasan \
> +    libattr-devel \
> +    libblockdev-mpath-devel \
> +    libcap-devel \
> +    libcap-ng-devel \
> +    libcurl-devel \
> +    libepoxy-devel \
> +    libfdt-devel \
> +    libgbm-devel \
> +    libiscsi-devel \
> +    libjpeg-devel \
> +    libpmem-devel \
> +    libnfs-devel \
> +    libpng-devel \
> +    librbd-devel \
> +    libseccomp-devel \
> +    libssh-devel \
> +    libubsan \
> +    libudev-devel \
> +    libusbx-devel \
> +    libxml2-devel \
> +    libzstd-devel \
> +    llvm \
> +    lzo-devel \
> +    make \
> +    mingw32-bzip2 \
> +    mingw32-curl \
> +    mingw32-glib2 \
> +    mingw32-gmp \
> +    mingw32-gnutls \
> +    mingw32-gtk3 \
> +    mingw32-libjpeg-turbo \
> +    mingw32-libpng \
> +    mingw32-libtasn1 \
> +    mingw32-nettle \
> +    mingw32-nsis \
> +    mingw32-pixman \
> +    mingw32-pkg-config \
> +    mingw32-SDL2 \
> +    mingw64-bzip2 \
> +    mingw64-curl \
> +    mingw64-glib2 \
> +    mingw64-gmp \
> +    mingw64-gnutls \
> +    mingw64-gtk3 \
> +    mingw64-libjpeg-turbo \
> +    mingw64-libpng \
> +    mingw64-libtasn1 \
> +    mingw64-nettle \
> +    mingw64-pixman \
> +    mingw64-pkg-config \
> +    mingw64-SDL2 \
> +    ncurses-devel \
> +    nettle-devel \
> +    nss-devel \
> +    numactl-devel \
> +    perl \
> +    perl-Test-Harness \
> +    pixman-devel \
> +    pulseaudio-libs-devel \
> +    python3 \
> +    python3-sphinx \
> +    PyYAML \
> +    rdma-core-devel \
> +    SDL2-devel \
> +    snappy-devel \
> +    sparse \
> +    spice-server-devel \
> +    systemd-devel \
> +    systemtap-sdt-devel \
> +    tar \
> +    texinfo \
> +    usbredir-devel \
> +    virglrenderer-devel \
> +    vte291-devel \
> +    wget \
> +    which \
> +    xen-devel \
> +    xfsprogs-devel \
> +    zlib-devel
> +ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
> +
> +RUN dnf install -y $PACKAGES
> +RUN rpm -q $PACKAGES | sort > /packages.txt
> +ENV PATH $PATH:/usr/libexec/python3-sphinx/
> +ENV COVERITY_TOOL_BASE=/coverity-tools
> +COPY run-coverity-scan run-coverity-scan
> +RUN --mount=type=secret,id=coverity.token,required ./run-coverity-scan --update-tools-only --tokenfile /run/secrets/coverity.token
> diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
> index d40b51969fa..2e067ef5cfc 100755
> --- a/scripts/coverity-scan/run-coverity-scan
> +++ b/scripts/coverity-scan/run-coverity-scan
> @@ -29,6 +29,7 @@
>   
>   # Command line options:
>   #   --dry-run : run the tools, but don't actually do the upload
> +#   --docker : create and work inside a docker container
>   #   --update-tools-only : update the cached copy of the tools, but don't run them
>   #   --tokenfile : file to read Coverity token from
>   #   --version ver : specify version being analyzed (default: ask git)
> @@ -36,6 +37,8 @@
>   #   --srcdir : QEMU source tree to analyze (default: current working dir)
>   #   --results-tarball : path to copy the results tarball to (default: don't
>   #                       copy it anywhere, just upload it)
> +#   --src-tarball : tarball to untar into src dir (default: none); this
> +#                   is intended mainly for internal use by the Docker support
>   #
>   # User-specifiable environment variables:
>   #  COVERITY_TOKEN -- Coverity token
> @@ -125,6 +128,7 @@ update_coverity_tools () {
>   # Check user-provided environment variables and arguments
>   DRYRUN=no
>   UPDATE_ONLY=no
> +DOCKER=no
>   
>   while [ "$#" -ge 1 ]; do
>       case "$1" in
> @@ -181,6 +185,19 @@ while [ "$#" -ge 1 ]; do
>               RESULTSTARBALL="$1"
>               shift
>               ;;
> +        --src-tarball)
> +            shift
> +            if [ $# -eq 0 ]; then
> +                echo "--src-tarball needs an argument"
> +                exit 1
> +            fi
> +            SRCTARBALL="$1"
> +            shift
> +            ;;
> +        --docker)
> +            DOCKER=yes
> +            shift
> +            ;;
>           *)
>               echo "Unexpected argument '$1'"
>               exit 1
> @@ -212,6 +229,10 @@ PROJTOKEN="$COVERITY_TOKEN"
>   PROJNAME=QEMU
>   TARBALL=cov-int.tar.xz
>   
> +if [ "$UPDATE_ONLY" = yes ] && [ "$DOCKER" = yes ]; then
> +    echo "Combining --docker and --update-only is not supported"
> +    exit 1
> +fi
>   
>   if [ "$UPDATE_ONLY" = yes ]; then
>       # Just do the tools update; we don't need to check whether
> @@ -221,8 +242,17 @@ if [ "$UPDATE_ONLY" = yes ]; then
>       exit 0
>   fi
>   
> +if [ ! -e "$SRCDIR" ]; then
> +    mkdir "$SRCDIR"
> +fi
> +
>   cd "$SRCDIR"
>   
> +if [ ! -z "$SRCTARBALL" ]; then
> +    echo "Untarring source tarball into $SRCDIR..."
> +    tar xvf "$SRCTARBALL"
> +fi
> +
>   echo "Checking this is a QEMU source tree..."
>   if ! [ -e "$SRCDIR/VERSION" ]; then
>       echo "Not in a QEMU source tree?"
> @@ -242,6 +272,66 @@ if [ -z "$COVERITY_EMAIL" ]; then
>       COVERITY_EMAIL="$(git config user.email)"
>   fi
>   
> +# Run ourselves inside docker if that's what the user wants
> +if [ "$DOCKER" = yes ]; then
> +    # build docker container including the coverity-scan tools
> +    # Put the Coverity token into a temporary file that only
> +    # we have read access to, and then pass it to docker build
> +    # using --secret. This requires at least Docker 18.09.
> +    # Mostly what we are trying to do here is ensure we don't leak
> +    # the token into the Docker image.
> +    umask 077
> +    SECRETDIR=$(mktemp -d)
> +    if [ -z "$SECRETDIR" ]; then
> +        echo "Failed to create temporary directory"
> +        exit 1
> +    fi
> +    trap 'rm -rf "$SECRETDIR"' INT TERM EXIT
> +    echo "Created temporary directory $SECRETDIR"
> +    SECRET="$SECRETDIR/token"
> +    echo "$COVERITY_TOKEN" > "$SECRET"
> +    echo "Building docker container..."
> +    # TODO: This re-downloads the tools every time, rather than
> +    # caching and reusing the image produced with the downloaded tools.
> +    # Not sure why.

I remember something similar when using -f and COPY.

My guess is using -f somefile instead of a directory, then COPY from 
outside of the directory, the cache is invalidated (or not used). If the 
file copied and the Dockerfile are in the same directory, it works (for me).

> +    # TODO: how do you get 'docker build' to print the output of the
> +    # commands it is running to its stdout? This would be useful for debug.

Maybe '--progress plain'?

> +    DOCKER_BUILDKIT=1 docker build -t coverity-scanner \
> +                   --secret id=coverity.token,src="$SECRET" \
> +                   -f scripts/coverity-scan/coverity-scan.docker \
> +                   scripts/coverity-scan
> +    echo "Archiving sources to be analyzed..."
> +    ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
> +    if [ "$DRYRUN" = yes ]; then
> +        DRYRUNARG=--dry-run
> +    fi
> +    echo "Running scanner..."
> +    # If we need to capture the output tarball, get the inner run to
> +    # save it to the secrets directory so we can copy it out before the
> +    # directory is cleaned up.
> +    if [ ! -z "$RESULTSTARBALL" ]; then
> +        RTARGS="--results-tarball /work/cov-int.tar.xz"
> +    else
> +        RTARGS=""
> +    fi
> +    # Arrange for this docker run to get access to the sources with -v.
> +    # We pass through all the configuration from the outer script to the inner.
> +    export COVERITY_EMAIL COVERITY_BUILD_CMD
> +    docker run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
> +           -v "$SECRETDIR:/work" coverity-scanner \
> +           ./run-coverity-scan --version "$VERSION" \
> +           --description "$DESCRIPTION" $DRYRUNARG --tokenfile /work/token \
> +           --srcdir /qemu --src-tarball /work/qemu-sources.tgz $RTARGS
> +    if [ ! -z "$RESULTSTARBALL" ]; then
> +        echo "Copying results tarball to $RESULTSTARBALL..."
> +        cp "$SECRETDIR/cov-int.tar.xz" "$RESULTSTARBALL"
> +    fi
> +    echo "Docker work complete."
> +    exit 0
> +fi
> +
> +# Otherwise, continue with the full build and upload process.
> +
>   check_upload_permissions
>   
>   update_coverity_tools
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 6/6] scripts/coverity-scan: Add Docker support

[Philippe Mathieu-Daudé]

On 4/14/20 1:58 PM, Philippe Mathieu-Daudé wrote:
> On 3/19/20 8:33 PM, Peter Maydell wrote:
>> Add support for running the Coverity Scan tools inside a Docker
>> container rather than directly on the host system.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> v1->v2:
>>   * various bug fixes
>>   * added --src-tarball rather than putting the whole source
>>     tree in the 'secrets' directory
>>   * docker file package list updated
>> ---
>>   scripts/coverity-scan/coverity-scan.docker | 131 +++++++++++++++++++++
>>   scripts/coverity-scan/run-coverity-scan    |  90 ++++++++++++++
>>   2 files changed, 221 insertions(+)
>>   create mode 100644 scripts/coverity-scan/coverity-scan.docker
>>
>> diff --git a/scripts/coverity-scan/coverity-scan.docker 
>> b/scripts/coverity-scan/coverity-scan.docker
>> new file mode 100644
>> index 00000000000..a4f64d12834
>> --- /dev/null
>> +++ b/scripts/coverity-scan/coverity-scan.docker
>> @@ -0,0 +1,131 @@
>> +# syntax=docker/dockerfile:1.0.0-experimental
>> +#
>> +# Docker setup for running the "Coverity Scan" tools over the source
>> +# tree and uploading them to the website, as per
>> +# https://scan.coverity.com/projects/qemu/builds/new
>> +# We do this on a fixed config (currently Fedora 30 with a known
>> +# set of dependencies and a configure command that enables a specific
>> +# set of options) so that random changes don't result in our 
>> accidentally
>> +# dropping some files from the scan.
>> +#
>> +# We don't build on top of the fedora.docker file because we don't
>> +# want to accidentally change or break the scan config when that
>> +# is updated.
>> +
>> +# The work of actually doing the build is handled by the
>> +# run-coverity-scan script.
>> +
>> +FROM fedora:30
>> +ENV PACKAGES \
>> +    alsa-lib-devel \
>> +    bc \
>> +    bison \
>> +    brlapi-devel \
>> +    bzip2 \
>> +    bzip2-devel \
>> +    ccache \
>> +    clang \
>> +    curl \
>> +    cyrus-sasl-devel \
>> +    dbus-daemon \
>> +    device-mapper-multipath-devel \
>> +    findutils \
>> +    flex \
>> +    gcc \
>> +    gcc-c++ \
>> +    gettext \
>> +    git \
>> +    glib2-devel \
>> +    glusterfs-api-devel \
>> +    gnutls-devel \
>> +    gtk3-devel \
>> +    hostname \
>> +    libaio-devel \
>> +    libasan \
>> +    libattr-devel \
>> +    libblockdev-mpath-devel \
>> +    libcap-devel \
>> +    libcap-ng-devel \
>> +    libcurl-devel \
>> +    libepoxy-devel \
>> +    libfdt-devel \
>> +    libgbm-devel \
>> +    libiscsi-devel \
>> +    libjpeg-devel \
>> +    libpmem-devel \
>> +    libnfs-devel \
>> +    libpng-devel \
>> +    librbd-devel \
>> +    libseccomp-devel \
>> +    libssh-devel \
>> +    libubsan \
>> +    libudev-devel \
>> +    libusbx-devel \
>> +    libxml2-devel \
>> +    libzstd-devel \
>> +    llvm \
>> +    lzo-devel \
>> +    make \
>> +    mingw32-bzip2 \
>> +    mingw32-curl \
>> +    mingw32-glib2 \
>> +    mingw32-gmp \
>> +    mingw32-gnutls \
>> +    mingw32-gtk3 \
>> +    mingw32-libjpeg-turbo \
>> +    mingw32-libpng \
>> +    mingw32-libtasn1 \
>> +    mingw32-nettle \
>> +    mingw32-nsis \
>> +    mingw32-pixman \
>> +    mingw32-pkg-config \
>> +    mingw32-SDL2 \
>> +    mingw64-bzip2 \
>> +    mingw64-curl \
>> +    mingw64-glib2 \
>> +    mingw64-gmp \
>> +    mingw64-gnutls \
>> +    mingw64-gtk3 \
>> +    mingw64-libjpeg-turbo \
>> +    mingw64-libpng \
>> +    mingw64-libtasn1 \
>> +    mingw64-nettle \
>> +    mingw64-pixman \
>> +    mingw64-pkg-config \
>> +    mingw64-SDL2 \
>> +    ncurses-devel \
>> +    nettle-devel \
>> +    nss-devel \
>> +    numactl-devel \
>> +    perl \
>> +    perl-Test-Harness \
>> +    pixman-devel \
>> +    pulseaudio-libs-devel \
>> +    python3 \
>> +    python3-sphinx \
>> +    PyYAML \
>> +    rdma-core-devel \
>> +    SDL2-devel \
>> +    snappy-devel \
>> +    sparse \
>> +    spice-server-devel \
>> +    systemd-devel \
>> +    systemtap-sdt-devel \
>> +    tar \
>> +    texinfo \
>> +    usbredir-devel \
>> +    virglrenderer-devel \
>> +    vte291-devel \
>> +    wget \
>> +    which \
>> +    xen-devel \
>> +    xfsprogs-devel \
>> +    zlib-devel
>> +ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
>> +
>> +RUN dnf install -y $PACKAGES
>> +RUN rpm -q $PACKAGES | sort > /packages.txt
>> +ENV PATH $PATH:/usr/libexec/python3-sphinx/
>> +ENV COVERITY_TOOL_BASE=/coverity-tools
>> +COPY run-coverity-scan run-coverity-scan
>> +RUN --mount=type=secret,id=coverity.token,required 
>> ./run-coverity-scan --update-tools-only --tokenfile 
>> /run/secrets/coverity.token
>> diff --git a/scripts/coverity-scan/run-coverity-scan 
>> b/scripts/coverity-scan/run-coverity-scan
>> index d40b51969fa..2e067ef5cfc 100755
>> --- a/scripts/coverity-scan/run-coverity-scan
>> +++ b/scripts/coverity-scan/run-coverity-scan
>> @@ -29,6 +29,7 @@
>>   # Command line options:
>>   #   --dry-run : run the tools, but don't actually do the upload
>> +#   --docker : create and work inside a docker container
>>   #   --update-tools-only : update the cached copy of the tools, but 
>> don't run them
>>   #   --tokenfile : file to read Coverity token from
>>   #   --version ver : specify version being analyzed (default: ask git)
>> @@ -36,6 +37,8 @@
>>   #   --srcdir : QEMU source tree to analyze (default: current working 
>> dir)
>>   #   --results-tarball : path to copy the results tarball to 
>> (default: don't
>>   #                       copy it anywhere, just upload it)
>> +#   --src-tarball : tarball to untar into src dir (default: none); this
>> +#                   is intended mainly for internal use by the Docker 
>> support
>>   #
>>   # User-specifiable environment variables:
>>   #  COVERITY_TOKEN -- Coverity token
>> @@ -125,6 +128,7 @@ update_coverity_tools () {
>>   # Check user-provided environment variables and arguments
>>   DRYRUN=no
>>   UPDATE_ONLY=no
>> +DOCKER=no
>>   while [ "$#" -ge 1 ]; do
>>       case "$1" in
>> @@ -181,6 +185,19 @@ while [ "$#" -ge 1 ]; do
>>               RESULTSTARBALL="$1"
>>               shift
>>               ;;
>> +        --src-tarball)
>> +            shift
>> +            if [ $# -eq 0 ]; then
>> +                echo "--src-tarball needs an argument"
>> +                exit 1
>> +            fi
>> +            SRCTARBALL="$1"
>> +            shift
>> +            ;;
>> +        --docker)
>> +            DOCKER=yes
>> +            shift
>> +            ;;
>>           *)
>>               echo "Unexpected argument '$1'"
>>               exit 1
>> @@ -212,6 +229,10 @@ PROJTOKEN="$COVERITY_TOKEN"
>>   PROJNAME=QEMU
>>   TARBALL=cov-int.tar.xz
>> +if [ "$UPDATE_ONLY" = yes ] && [ "$DOCKER" = yes ]; then
>> +    echo "Combining --docker and --update-only is not supported"
>> +    exit 1
>> +fi
>>   if [ "$UPDATE_ONLY" = yes ]; then
>>       # Just do the tools update; we don't need to check whether
>> @@ -221,8 +242,17 @@ if [ "$UPDATE_ONLY" = yes ]; then
>>       exit 0
>>   fi
>> +if [ ! -e "$SRCDIR" ]; then
>> +    mkdir "$SRCDIR"
>> +fi
>> +
>>   cd "$SRCDIR"
>> +if [ ! -z "$SRCTARBALL" ]; then
>> +    echo "Untarring source tarball into $SRCDIR..."
>> +    tar xvf "$SRCTARBALL"
>> +fi
>> +
>>   echo "Checking this is a QEMU source tree..."
>>   if ! [ -e "$SRCDIR/VERSION" ]; then
>>       echo "Not in a QEMU source tree?"
>> @@ -242,6 +272,66 @@ if [ -z "$COVERITY_EMAIL" ]; then
>>       COVERITY_EMAIL="$(git config user.email)"
>>   fi
>> +# Run ourselves inside docker if that's what the user wants
>> +if [ "$DOCKER" = yes ]; then
>> +    # build docker container including the coverity-scan tools
>> +    # Put the Coverity token into a temporary file that only
>> +    # we have read access to, and then pass it to docker build
>> +    # using --secret. This requires at least Docker 18.09.
>> +    # Mostly what we are trying to do here is ensure we don't leak
>> +    # the token into the Docker image.
>> +    umask 077
>> +    SECRETDIR=$(mktemp -d)
>> +    if [ -z "$SECRETDIR" ]; then
>> +        echo "Failed to create temporary directory"
>> +        exit 1
>> +    fi
>> +    trap 'rm -rf "$SECRETDIR"' INT TERM EXIT
>> +    echo "Created temporary directory $SECRETDIR"
>> +    SECRET="$SECRETDIR/token"
>> +    echo "$COVERITY_TOKEN" > "$SECRET"
>> +    echo "Building docker container..."
>> +    # TODO: This re-downloads the tools every time, rather than
>> +    # caching and reusing the image produced with the downloaded tools.
>> +    # Not sure why.
> 
> I remember something similar when using -f and COPY.
> 
> My guess is using -f somefile instead of a directory, then COPY from 
> outside of the directory, the cache is invalidated (or not used). If the 
> file copied and the Dockerfile are in the same directory, it works (for 
> me).
> 
>> +    # TODO: how do you get 'docker build' to print the output of the
>> +    # commands it is running to its stdout? This would be useful for 
>> debug.
> 
> Maybe '--progress plain'?
> 
>> +    DOCKER_BUILDKIT=1 docker build -t coverity-scanner \
>> +                   --secret id=coverity.token,src="$SECRET" \
>> +                   -f scripts/coverity-scan/coverity-scan.docker \
>> +                   scripts/coverity-scan
>> +    echo "Archiving sources to be analyzed..."
>> +    ./scripts/archive-source.sh "$SECRETDIR/qemu-sources.tgz"
>> +    if [ "$DRYRUN" = yes ]; then
>> +        DRYRUNARG=--dry-run
>> +    fi
>> +    echo "Running scanner..."
>> +    # If we need to capture the output tarball, get the inner run to
>> +    # save it to the secrets directory so we can copy it out before the
>> +    # directory is cleaned up.
>> +    if [ ! -z "$RESULTSTARBALL" ]; then
>> +        RTARGS="--results-tarball /work/cov-int.tar.xz"
>> +    else
>> +        RTARGS=""
>> +    fi
>> +    # Arrange for this docker run to get access to the sources with -v.
>> +    # We pass through all the configuration from the outer script to 
>> the inner.
>> +    export COVERITY_EMAIL COVERITY_BUILD_CMD
>> +    docker run -it --env COVERITY_EMAIL --env COVERITY_BUILD_CMD \
>> +           -v "$SECRETDIR:/work" coverity-scanner \
>> +           ./run-coverity-scan --version "$VERSION" \
>> +           --description "$DESCRIPTION" $DRYRUNARG --tokenfile 
>> /work/token \
>> +           --srcdir /qemu --src-tarball /work/qemu-sources.tgz $RTARGS
>> +    if [ ! -z "$RESULTSTARBALL" ]; then
>> +        echo "Copying results tarball to $RESULTSTARBALL..."
>> +        cp "$SECRETDIR/cov-int.tar.xz" "$RESULTSTARBALL"
>> +    fi
>> +    echo "Docker work complete."
>> +    exit 0
>> +fi
>> +
>> +# Otherwise, continue with the full build and upload process.
>> +
>>   check_upload_permissions
>>   update_coverity_tools
>>
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Oh also,
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
until:

#14 0.441 Connecting to scan.coverity.com 
(scan.coverity.com)|45.60.34.99|:443... connected.
#14 0.570 HTTP request sent, awaiting response... 401 Unauthorized
#14 1.033
#14 1.033 Username/Password Authentication Failed.

Re: [PATCH v2 5/6] scripts/run-coverity-scan: Script to run Coverity Scan build

[Philippe Mathieu-Daudé]

On 3/19/20 8:33 PM, Peter Maydell wrote:
> Add a new script to automate the process of running the Coverity
> Scan build tools and uploading the resulting tarball to the
> website.
> 
> This is intended eventually to be driven from Travis,
> but it can be run locally, if you are a maintainer of the
> QEMU project on the Coverity Scan website and have the secret
> upload token.
> 
> The script must be run on a Fedora 30 system.  Support for using a
> Docker container is added in a following commit.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> changes v1->v2:
>   * fix sense of DRYRUN test in check_upload_permissions
>   * use nproc rather than hardcoding -j8
>   * use $PWD rather than $(pwd)
>   * minor tweaks to configure line
>   * new --results-tarball option
> ---
>   MAINTAINERS                             |   5 +
>   scripts/coverity-scan/run-coverity-scan | 311 ++++++++++++++++++++++++
>   2 files changed, 316 insertions(+)
>   create mode 100755 scripts/coverity-scan/run-coverity-scan
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7364af0d8b0..395534522b6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2003,6 +2003,11 @@ M: Markus Armbruster <armbru@redhat.com>
>   S: Supported
>   F: scripts/coverity-model.c
>   
> +Coverity Scan integration
> +M: Peter Maydell <peter.maydell@linaro.org>
> +S: Maintained
> +F: scripts/coverity-scan/
> +
>   Device Tree
>   M: Alistair Francis <alistair.francis@wdc.com>
>   R: David Gibson <david@gibson.dropbear.id.au>
> diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
> new file mode 100755
> index 00000000000..d40b51969fa
> --- /dev/null
> +++ b/scripts/coverity-scan/run-coverity-scan
> @@ -0,0 +1,311 @@
> +#!/bin/sh -e
> +
> +# Upload a created tarball to Coverity Scan, as per
> +# https://scan.coverity.com/projects/qemu/builds/new
> +
> +# This work is licensed under the terms of the GNU GPL version 2,
> +# or (at your option) any later version.
> +# See the COPYING file in the top-level directory.
> +#
> +# Copyright (c) 2017-2020 Linaro Limited
> +# Written by Peter Maydell
> +
> +# Note that this script will automatically download and
> +# run the (closed-source) coverity build tools, so don't
> +# use it if you don't trust them!
> +
> +# This script assumes that you're running it from a QEMU source
> +# tree, and that tree is a fresh clean one, because we do an in-tree
> +# build. (This is necessary so that the filenames that the Coverity
> +# Scan server sees are relative paths that match up with the component
> +# regular expressions it uses; an out-of-tree build won't work for this.)
> +# The host machine should have as many of QEMU's dependencies
> +# installed as possible, for maximum coverity coverage.
> +
> +# To do an upload you need to be a maintainer in the Coverity online
> +# service, and you will need to know the "Coverity token", which is a
> +# secret 8 digit hex string. You can find that from the web UI in the
> +# project settings, if you have maintainer access there.
> +
> +# Command line options:
> +#   --dry-run : run the tools, but don't actually do the upload
> +#   --update-tools-only : update the cached copy of the tools, but don't run them
> +#   --tokenfile : file to read Coverity token from
> +#   --version ver : specify version being analyzed (default: ask git)
> +#   --description desc : specify description of this version (default: ask git)
> +#   --srcdir : QEMU source tree to analyze (default: current working dir)
> +#   --results-tarball : path to copy the results tarball to (default: don't
> +#                       copy it anywhere, just upload it)
> +#
> +# User-specifiable environment variables:
> +#  COVERITY_TOKEN -- Coverity token
> +#  COVERITY_EMAIL -- the email address to use for uploads (default:
> +#                    looks at your git user.email config)
> +#  COVERITY_BUILD_CMD -- make command (default: 'make -jN' where N is
> +#                    number of CPUs as determined by 'nproc')
> +#  COVERITY_TOOL_BASE -- set to directory to put coverity tools
> +#                        (default: /tmp/coverity-tools)
> +#
> +# You must specify the token, either by environment variable or by
> +# putting it in a file and using --tokenfile. Everything else has
> +# a reasonable default if this is run from a git tree.
> +
> +check_upload_permissions() {
> +    # Check whether we can do an upload to the server; will exit the script
> +    # with status 1 if the check failed (usually a bad token);
> +    # will exit the script with status 0 if the check indicated that we
> +    # can't upload yet (ie we are at quota)
> +    # Assumes that PROJTOKEN, PROJNAME and DRYRUN have been initialized.
> +
> +    echo "Checking upload permissions..."
> +
> +    if ! up_perm="$(wget https://scan.coverity.com/api/upload_permitted --post-data "token=$PROJTOKEN&project=$PROJNAME" -q -O -)"; then
> +        echo "Coverity Scan API access denied: bad token?"
> +        exit 1
> +    fi
> +
> +    # Really up_perm is a JSON response with either
> +    # {upload_permitted:true} or {next_upload_permitted_at:<date>}
> +    # We do some hacky string parsing instead of properly parsing it.
> +    case "$up_perm" in
> +        *upload_permitted*true*)
> +            echo "Coverity Scan: upload permitted"
> +            ;;
> +        *next_upload_permitted_at*)
> +            if [ "$DRYRUN" = yes ]; then
> +                echo "Coverity Scan: upload quota reached, continuing dry run"
> +            else
> +                echo "Coverity Scan: upload quota reached; stopping here"
> +                # Exit success as this isn't a build error.
> +                exit 0
> +            fi
> +            ;;
> +        *)
> +            echo "Coverity Scan upload check: unexpected result $up_perm"
> +            exit 1
> +            ;;
> +    esac
> +}
> +
> +
> +update_coverity_tools () {
> +    # Check for whether we need to download the Coverity tools
> +    # (either because we don't have a copy, or because it's out of date)
> +    # Assumes that COVERITY_TOOL_BASE, PROJTOKEN and PROJNAME are set.
> +
> +    mkdir -p "$COVERITY_TOOL_BASE"
> +    cd "$COVERITY_TOOL_BASE"
> +
> +    echo "Checking for new version of coverity build tools..."
> +    wget https://scan.coverity.com/download/linux64 --post-data "token=$PROJTOKEN&project=$PROJNAME&md5=1" -O coverity_tool.md5.new
> +
> +    if ! cmp -s coverity_tool.md5 coverity_tool.md5.new; then
> +        # out of date md5 or no md5: download new build tool
> +        # blow away the old build tool
> +        echo "Downloading coverity build tools..."
> +        rm -rf coverity_tool coverity_tool.tgz
> +        wget https://scan.coverity.com/download/linux64 --post-data "token=$PROJTOKEN&project=$PROJNAME" -O coverity_tool.tgz
> +        if ! (cat coverity_tool.md5.new; echo "  coverity_tool.tgz") | md5sum -c --status; then
> +            echo "Downloaded tarball didn't match md5sum!"
> +            exit 1
> +        fi
> +        # extract the new one, keeping it corralled in a 'coverity_tool' directory
> +        echo "Unpacking coverity build tools..."
> +        mkdir -p coverity_tool
> +        cd coverity_tool
> +        tar xf ../coverity_tool.tgz
> +        cd ..
> +        mv coverity_tool.md5.new coverity_tool.md5
> +    fi
> +
> +    rm -f coverity_tool.md5.new
> +}
> +
> +
> +# Check user-provided environment variables and arguments
> +DRYRUN=no
> +UPDATE_ONLY=no
> +
> +while [ "$#" -ge 1 ]; do
> +    case "$1" in
> +        --dry-run)
> +            shift
> +            DRYRUN=yes
> +            ;;
> +        --update-tools-only)
> +            shift
> +            UPDATE_ONLY=yes
> +            ;;
> +        --version)
> +            shift
> +            if [ $# -eq 0 ]; then
> +                echo "--version needs an argument"
> +                exit 1
> +            fi
> +            VERSION="$1"
> +            shift
> +            ;;
> +        --description)
> +            shift
> +            if [ $# -eq 0 ]; then
> +                echo "--description needs an argument"
> +                exit 1
> +            fi
> +            DESCRIPTION="$1"
> +            shift
> +            ;;
> +        --tokenfile)
> +            shift
> +            if [ $# -eq 0 ]; then
> +                echo "--tokenfile needs an argument"
> +                exit 1
> +            fi
> +            COVERITY_TOKEN="$(cat "$1")"
> +            shift
> +            ;;
> +        --srcdir)
> +            shift
> +            if [ $# -eq 0 ]; then
> +                echo "--srcdir needs an argument"
> +                exit 1
> +            fi
> +            SRCDIR="$1"
> +            shift
> +            ;;
> +        --results-tarball)
> +            shift
> +            if [ $# -eq 0 ]; then
> +                echo "--results-tarball needs an argument"
> +                exit 1
> +            fi
> +            RESULTSTARBALL="$1"
> +            shift
> +            ;;
> +        *)
> +            echo "Unexpected argument '$1'"
> +            exit 1
> +            ;;
> +    esac
> +done
> +
> +if [ -z "$COVERITY_TOKEN" ]; then
> +    echo "COVERITY_TOKEN environment variable not set"
> +    exit 1
> +fi
> +
> +if [ -z "$COVERITY_BUILD_CMD" ]; then
> +    NPROC=$(nproc)
> +    COVERITY_BUILD_CMD="make -j$NPROC"
> +    echo "COVERITY_BUILD_CMD: using default '$COVERITY_BUILD_CMD'"
> +fi
> +
> +if [ -z "$COVERITY_TOOL_BASE" ]; then
> +    echo "COVERITY_TOOL_BASE: using default /tmp/coverity-tools"
> +    COVERITY_TOOL_BASE=/tmp/coverity-tools
> +fi
> +
> +if [ -z "$SRCDIR" ]; then
> +    SRCDIR="$PWD"
> +fi
> +
> +PROJTOKEN="$COVERITY_TOKEN"
> +PROJNAME=QEMU
> +TARBALL=cov-int.tar.xz
> +
> +
> +if [ "$UPDATE_ONLY" = yes ]; then
> +    # Just do the tools update; we don't need to check whether
> +    # we are in a source tree or have upload rights for this,
> +    # so do it before some of the command line and source tree checks.
> +    update_coverity_tools
> +    exit 0
> +fi
> +
> +cd "$SRCDIR"
> +
> +echo "Checking this is a QEMU source tree..."
> +if ! [ -e "$SRCDIR/VERSION" ]; then
> +    echo "Not in a QEMU source tree?"
> +    exit 1
> +fi
> +
> +# Fill in defaults used by the non-update-only process
> +if [ -z "$VERSION" ]; then
> +    VERSION="$(git describe --always HEAD)"
> +fi
> +
> +if [ -z "$DESCRIPTION" ]; then
> +    DESCRIPTION="$(git rev-parse HEAD)"
> +fi
> +
> +if [ -z "$COVERITY_EMAIL" ]; then
> +    COVERITY_EMAIL="$(git config user.email)"
> +fi
> +
> +check_upload_permissions
> +
> +update_coverity_tools
> +
> +TOOLBIN="$(cd "$COVERITY_TOOL_BASE" && echo $PWD/coverity_tool/cov-analysis-*/bin)"
> +
> +if ! test -x "$TOOLBIN/cov-build"; then
> +    echo "Couldn't find cov-build in the coverity build-tool directory??"
> +    exit 1
> +fi
> +
> +export PATH="$TOOLBIN:$PATH"
> +
> +cd "$SRCDIR"
> +
> +echo "Doing make distclean..."
> +make distclean
> +
> +echo "Configuring..."
> +# We configure with a fixed set of enables here to ensure that we don't
> +# accidentally reduce the scope of the analysis by doing the build on
> +# the system that's missing a dependency that we need to build part of
> +# the codebase.
> +./configure --disable-modules --enable-sdl --enable-gtk \
> +    --enable-opengl --enable-vte --enable-gnutls \
> +    --enable-nettle --enable-curses --enable-curl \
> +    --audio-drv-list=oss,alsa,sdl,pa --enable-virtfs \
> +    --enable-vnc --enable-vnc-sasl --enable-vnc-jpeg --enable-vnc-png \
> +    --enable-xen --enable-brlapi \
> +    --enable-linux-aio --enable-attr \
> +    --enable-cap-ng --enable-trace-backends=log --enable-spice --enable-rbd \
> +    --enable-xfsctl --enable-libusb --enable-usb-redir \
> +    --enable-libiscsi --enable-libnfs --enable-seccomp \
> +    --enable-tpm --enable-libssh --enable-lzo --enable-snappy --enable-bzip2 \
> +    --enable-numa --enable-rdma --enable-smartcard --enable-virglrenderer \
> +    --enable-mpath --enable-libxml2 --enable-glusterfs \
> +    --enable-virtfs --enable-zstd
> +
> +echo "Making libqemustub.a..."
> +make libqemustub.a
> +
> +echo "Running cov-build..."
> +rm -rf cov-int
> +mkdir cov-int
> +cov-build --dir cov-int $COVERITY_BUILD_CMD
> +
> +echo "Creating results tarball..."
> +tar cvf - cov-int | xz > "$TARBALL"
> +
> +if [ ! -z "$RESULTSTARBALL" ]; then
> +    echo "Copying results tarball to $RESULTSTARBALL..."
> +    cp "$TARBALL" "$RESULTSTARBALL"
> +fi
> +
> +echo "Uploading results tarball..."
> +
> +if [ "$DRYRUN" = yes ]; then
> +    echo "Dry run only, not uploading $TARBALL"
> +    exit 0
> +fi
> +
> +curl --form token="$PROJTOKEN" --form email="$COVERITY_EMAIL" \
> +     --form file=@"$TARBALL" --form version="$VERSION" \
> +     --form description="$DESCRIPTION" \
> +     https://scan.coverity.com/builds?project="$PROJNAME"
> +
> +echo "Done."
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 6/6] scripts/coverity-scan: Add Docker support

[Peter Maydell]

On Tue, 14 Apr 2020 at 12:58, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 3/19/20 8:33 PM, Peter Maydell wrote:
> > +    # TODO: This re-downloads the tools every time, rather than
> > +    # caching and reusing the image produced with the downloaded tools.
> > +    # Not sure why.
>
> I remember something similar when using -f and COPY.
>
> My guess is using -f somefile instead of a directory, then COPY from
> outside of the directory, the cache is invalidated (or not used). If the
> file copied and the Dockerfile are in the same directory, it works (for me).

The comment turns out to be not entirely accurate -- at least some
of the time it successfully skips re-doing the tools download;
but sometimes it doesn't and I'm not sure what triggers that.



> > +    # TODO: how do you get 'docker build' to print the output of the
> > +    # commands it is running to its stdout? This would be useful for debug.
>
> Maybe '--progress plain'?

Good find, I'll have to try that.

thanks
-- PMM