[Qemu-devel] [PATCH 0/2] Automation for running Coverity Scan builds

[Qemu-devel] [PATCH 0/2] Automation for running Coverity Scan builds

[Peter Maydell]

This patchset is an attempt to automate the process of
running a Coverity Scan build and uploading it to the website.
(I had a go at this last year; the scripts here are based
on my older attempt, with some changes.)

Patch 1 is a script which will do a build-and-upload. It
requires that you have a Fedora 28 host with all the relevant
-devel packages installed, and that you run it in a clean directory.

Patch 2 improves on that by using Docker to set up the
build environment, so you can run it on any host and get
the same results, and it doesn't trash your source tree with
an in-place build.

Patch 1 I'm generally pretty happy with. Paolo, if you could
check the configure options against what you use at the moment
for uploads that would be very helpful. These are the set that
you gave me last time I asked, but I think that was long enough
ago that they may be a little out of date.

Patch 2 is a bit more RFC-ish -- it does work and you can do a
complete build-and-upload with it, but I was thrashing about
trying to get Docker to do what I wanted and I'm pretty sure
there are better ways to do it. In particular I was expecting
that once the script has run once and done the "set up F28,
download the coverity tools and unpack them", that a second
run would reuse a cached copy of that image, but the docker
build seems to repeat the download-and-unpack part.
NB that you need the bleeding-edge Docker 18.09 for the --secret
option which lets us pass the coverity magic token into the
build stage safely. Review from people who know Docker welcomed.


I would eventually like to set up Travis so it does the
build-and-upload automatically, but I think getting to a
point where coverity uploads are done with a process that
can be repeated reliably by anybody with maintainer rights
is useful in itself even if we don't want to do it via Travis.

thanks
-- PMM


Peter Maydell (2):
  scripts/run-coverity-scan: Script to run Coverity Scan build
  scripts/coverity-scan: Add Docker support

 MAINTAINERS                                |   5 +
 scripts/coverity-scan/coverity-scan.docker | 120 +++++++
 scripts/coverity-scan/run-coverity-scan    | 350 +++++++++++++++++++++
 3 files changed, 475 insertions(+)
 create mode 100644 scripts/coverity-scan/coverity-scan.docker
 create mode 100755 scripts/coverity-scan/run-coverity-scan

-- 
2.19.1

[Qemu-devel] [PATCH 1/2] 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 directly on a Fedora 28 system.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 MAINTAINERS                             |   5 +
 scripts/coverity-scan/run-coverity-scan | 292 ++++++++++++++++++++++++
 2 files changed, 297 insertions(+)
 create mode 100755 scripts/coverity-scan/run-coverity-scan

diff --git a/MAINTAINERS b/MAINTAINERS
index 126fe0be7eb..5f107d99061 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1553,6 +1553,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/
+
 CPU
 L: qemu-devel@nongnu.org
 S: Supported
diff --git a/scripts/coverity-scan/run-coverity-scan b/scripts/coverity-scan/run-coverity-scan
new file mode 100755
index 00000000000..99495b04501
--- /dev/null
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -0,0 +1,292 @@
+#!/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, 2018 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)
+#
+# 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 -j8')
+#  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; stopping here"
+                # Exit success as this isn't a build error.
+                exit 0
+            else
+                echo "Coverity Scan: upload quota reached, continuing dry run"
+            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
+            ;;
+        *)
+            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
+    echo "COVERITY_BUILD_CMD: using default 'make -j8'"
+    COVERITY_BUILD_CMD="make -j8"
+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 --with-sdlabi=2.0 --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-bluez \
+    --disable-vde --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-libssh2 --enable-lzo --enable-snappy --enable-bzip2 \
+    --enable-numa --enable-rdma --enable-smartcard --enable-virglrenderer \
+    --enable-mpath --enable-libxml2 --enable-glusterfs
+
+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"
+
+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.19.1

[Qemu-devel] [PATCH 2/2] 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>
---
 scripts/coverity-scan/coverity-scan.docker | 120 +++++++++++++++++++++
 scripts/coverity-scan/run-coverity-scan    |  58 ++++++++++
 2 files changed, 178 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..81f69459954
--- /dev/null
+++ b/scripts/coverity-scan/coverity-scan.docker
@@ -0,0 +1,120 @@
+# 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 28 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.
+# The work of actually doing the build is handled by the
+# run-coverity-scan script.
+
+
+FROM fedora:28
+ENV PACKAGES \
+    alsa-lib-devel \
+    bc \
+    bison \
+    bluez-libs-devel \
+    brlapi-devel \
+    bzip2 \
+    bzip2-devel \
+    ccache \
+    clang \
+    curl \
+    cyrus-sasl-devel \
+    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 \
+    libcap-devel \
+    libcap-ng-devel \
+    libcurl-devel \
+    libepoxy-devel \
+    libfdt-devel \
+    libgbm-devel \
+    libiscsi-devel \
+    libjpeg-devel \
+    libnfs-devel \
+    libpng-devel \
+    librbd-devel \
+    libseccomp-devel \
+    libssh2-devel \
+    libubsan \
+    libudev-devel \
+    libusbx-devel \
+    libxml2-devel \
+    llvm \
+    lzo-devel \
+    make \
+    mingw32-bzip2 \
+    mingw32-curl \
+    mingw32-glib2 \
+    mingw32-gmp \
+    mingw32-gnutls \
+    mingw32-gtk3 \
+    mingw32-libjpeg-turbo \
+    mingw32-libpng \
+    mingw32-libssh2 \
+    mingw32-libtasn1 \
+    mingw32-nettle \
+    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-libssh2 \
+    mingw64-libtasn1 \
+    mingw64-nettle \
+    mingw64-pixman \
+    mingw64-pkg-config \
+    mingw64-SDL2 \
+    ncurses-devel \
+    nettle-devel \
+    nss-devel \
+    numactl-devel \
+    perl \
+    pixman-devel \
+    pulseaudio-libs-devel \
+    python3 \
+    PyYAML \
+    rdma-core-devel \
+    SDL2-devel \
+    snappy-devel \
+    sparse \
+    spice-server-devel \
+    systemtap-sdt-devel \
+    tar \
+    usbredir-devel \
+    virglrenderer-devel \
+    vte3-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 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 99495b04501..e89316c090d 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)
@@ -122,6 +123,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
@@ -169,6 +171,10 @@ while [ "$#" -ge 1 ]; do
             SRCDIR="$1"
             shift
             ;;
+        --docker)
+            DOCKER=yes
+            shift
+            ;;
         *)
             echo "Unexpected argument '$1'"
             exit 1
@@ -199,6 +205,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
@@ -229,6 +239,54 @@ 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"
+    (cd "$SECRETDIR" && mkdir qemu && cd qemu && tar xvf ../qemu-sources.tgz)
+    if [ "$DRYRUN" = yes ]; then
+        DRYRUNARG=--dry-run
+    fi
+    echo "Running scanner..."
+    # 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.
+    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 /work/qemu
+    echo "Docker work complete."
+    exit 0
+fi
+
+# Otherwise, continue with the full build and upload process.
+
 check_upload_permissions
 
 update_coverity_tools
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/2] scripts/run-coverity-scan: Script to run Coverity Scan build

[Eric Blake]

On 11/13/18 12:46 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 directly on a Fedora 28 system.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---

> +++ b/scripts/coverity-scan/run-coverity-scan
> @@ -0,0 +1,292 @@
> +#!/bin/sh -e

set -e...

> +check_upload_permissions() {

...and shell functions do NOT intuitively do what you would think. It's 
almost always better to use explicit error checking than to rely on set 
-e as a crutch, because then you don't get surprises.

> +    # 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; stopping here"
> +                # Exit success as this isn't a build error.
> +                exit 0
> +            else
> +                echo "Coverity Scan: upload quota reached, continuing dry run"
> +            fi

Did you get the logic backwards on this if?  Based on the error message, 
I was guessing the intended condition was [ "$DRYRUN" != yes ]

> +            ;;
> +        *)
> +            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

Assumes GNU tar, with its auto-decompression. But then again, you said 
the entire script assumes Fedora 28, so that's not necessarily bad.

> +        cd ..
> +        mv coverity_tool.md5.new coverity_tool.md5

Here's an example of where 'set -e' could bite - if tar or mv fails 
(perhaps due to ENOSPC), the decision of whether the shell function 
immediately stops or continues on to the next line (without handling the 
error) is dependent on the context that the caller used when calling 
update_coverity_tools (that is, 'update_coverity_tools' and 
'update_coverity_tools || fail' behave differently).

> +    fi
> +
> +    rm -f coverity_tool.md5.new
> +}
> +
> +
> +# Check user-provided environment variables and arguments
> +DRYRUN=no
> +UPDATE_ONLY=no
> +
> +while [ "$#" -ge 1 ]; do

> +done
> +
> +if [ -z "$COVERITY_TOKEN" ]; then
> +    echo "COVERITY_TOKEN environment variable not set"
> +    exit 1
> +fi
> +
> +if [ -z "$COVERITY_BUILD_CMD" ]; then
> +    echo "COVERITY_BUILD_CMD: using default 'make -j8'"
> +    COVERITY_BUILD_CMD="make -j8"

Should this query 'nproc' instead of hard-coding -j8?

> +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)"

Why not save a process, and just use 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 $CDPATH is set and $COVERITY_TOOL_BASE does not contain /, this could 
result in garbage being prepended to TOOLBIN as output from the 'cd'. 
Also, $PWD is nicer than $(pwd); but are you sure the glob in 
cov-analysis-* won't select too many files?

> +
> +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 --with-sdlabi=2.0 --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-bluez \
> +    --disable-vde --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-libssh2 --enable-lzo --enable-snappy --enable-bzip2 \
> +    --enable-numa --enable-rdma --enable-smartcard --enable-virglrenderer \
> +    --enable-mpath --enable-libxml2 --enable-glusterfs
> +
> +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"
> +
> +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."
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/2] scripts/run-coverity-scan: Script to run Coverity Scan build

[Peter Maydell]

On 13 November 2018 at 19:06, Eric Blake <eblake@redhat.com> wrote:
> On 11/13/18 12:46 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 directly on a Fedora 28 system.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---

Thanks for the code review -- my shell scripting has some
bad habits in it.

>
>> +++ b/scripts/coverity-scan/run-coverity-scan
>> @@ -0,0 +1,292 @@
>> +#!/bin/sh -e
>
>
> set -e...
>
>> +check_upload_permissions() {
>
>
> ...and shell functions do NOT intuitively do what you would think. It's
> almost always better to use explicit error checking than to rely on set -e
> as a crutch, because then you don't get surprises.

I think we had this conversation with last year's version
of this script too :-)  As you say, the use of functions
makes it maybe better to use explicit error checking -- but
is there a standard syntax for that that doesn't make
basic
 foo
 bar
 baz
"do these things in order" code look weird and require special care?
At least with set -e you do get error checking, whereas scripts without
it tend to just plough on regardless (look at configure, which doesn't
use set -e but doesn't have explicit checking either).

>> +    # 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; stopping here"
>> +                # Exit success as this isn't a build error.
>> +                exit 0
>> +            else
>> +                echo "Coverity Scan: upload quota reached, continuing dry
>> run"
>> +            fi
>
>
> Did you get the logic backwards on this if?  Based on the error message, I
> was guessing the intended condition was [ "$DRYRUN" != yes ]

Yes, I did (I flipped the way I was doing checks from "unset
means no" to "check if it is yes", and didn't get it right;
I caught another instance of this later, but missed this one.)

>> +done
>> +
>> +if [ -z "$COVERITY_TOKEN" ]; then
>> +    echo "COVERITY_TOKEN environment variable not set"
>> +    exit 1
>> +fi
>> +
>> +if [ -z "$COVERITY_BUILD_CMD" ]; then
>> +    echo "COVERITY_BUILD_CMD: using default 'make -j8'"
>> +    COVERITY_BUILD_CMD="make -j8"
>
>
> Should this query 'nproc' instead of hard-coding -j8?

Probably. Legacy of this thing developing from a local hack
into something a bit more 'production'.

>> +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)"
>
>
> Why not save a process, and just use SRCDIR="$PWD"

I never remember that $PWD exists, because when I'm doing
things on a shell command line I always use 'pwd'. But
it would be better, yes.

>> +TOOLBIN="$(cd "$COVERITY_TOOL_BASE" && echo
>> $(pwd)/coverity_tool/cov-analysis-*/bin)"
>
>
> If $CDPATH is set and $COVERITY_TOOL_BASE does not contain /, this could
> result in garbage being prepended to TOOLBIN as output from the 'cd'. Also,
> $PWD is nicer than $(pwd); but are you sure the glob in cov-analysis-* won't
> select too many files?

The glob is not great, but it is necessary to make the script
robust over new versions of the tools, which put the version
number in the cov-analysis-whatever directory name. If
we do ever get more than one file then the "test -x" below
will fail, and we'll be able to investigate and fix up the script.

CDPATH sounds like a horrific misfeature designed for breaking
scripts, so I'm not very interested in trying to work around it.
We don't seem to worry about this in configure either.
(I suppose we could just unset it at the start of the script.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] scripts/coverity-scan: Add Docker support

[Philippe Mathieu-Daudé]

On 13/11/18 19:46, 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>
> ---
>   scripts/coverity-scan/coverity-scan.docker | 120 +++++++++++++++++++++
>   scripts/coverity-scan/run-coverity-scan    |  58 ++++++++++
>   2 files changed, 178 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..81f69459954
> --- /dev/null
> +++ b/scripts/coverity-scan/coverity-scan.docker
> @@ -0,0 +1,120 @@
> +# 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 28 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.
> +# The work of actually doing the build is handled by the
> +# run-coverity-scan script.
> +
> +
> +FROM fedora:28
> +ENV PACKAGES \
> +    alsa-lib-devel \
> +    bc \
> +    bison \
> +    bluez-libs-devel \
> +    brlapi-devel \
> +    bzip2 \
> +    bzip2-devel \
> +    ccache \
> +    clang \
> +    curl \
> +    cyrus-sasl-devel \
> +    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 \
> +    libcap-devel \
> +    libcap-ng-devel \
> +    libcurl-devel \
> +    libepoxy-devel \
> +    libfdt-devel \
> +    libgbm-devel \
> +    libiscsi-devel \
> +    libjpeg-devel \
> +    libnfs-devel \
> +    libpng-devel \
> +    librbd-devel \
> +    libseccomp-devel \
> +    libssh2-devel \
> +    libubsan \
> +    libudev-devel \
> +    libusbx-devel \
> +    libxml2-devel \
> +    llvm \
> +    lzo-devel \
> +    make \
> +    mingw32-bzip2 \
> +    mingw32-curl \
> +    mingw32-glib2 \
> +    mingw32-gmp \
> +    mingw32-gnutls \
> +    mingw32-gtk3 \
> +    mingw32-libjpeg-turbo \
> +    mingw32-libpng \
> +    mingw32-libssh2 \
> +    mingw32-libtasn1 \
> +    mingw32-nettle \
> +    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-libssh2 \
> +    mingw64-libtasn1 \
> +    mingw64-nettle \
> +    mingw64-pixman \
> +    mingw64-pkg-config \
> +    mingw64-SDL2 \
> +    ncurses-devel \
> +    nettle-devel \
> +    nss-devel \
> +    numactl-devel \
> +    perl \
> +    pixman-devel \
> +    pulseaudio-libs-devel \
> +    python3 \
> +    PyYAML \
> +    rdma-core-devel \
> +    SDL2-devel \
> +    snappy-devel \
> +    sparse \
> +    spice-server-devel \
> +    systemtap-sdt-devel \
> +    tar \
> +    usbredir-devel \
> +    virglrenderer-devel \
> +    vte3-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 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

Calling "make docket-image-fedora" you can reduce this script to:

-- >8 --
FROM qemu:fedora
ENV PACKAGES \
     $PACKAGES \
     alsa-lib-devel \
     curl \
     cyrus-sasl-devel \
     libepoxy-devel \
     libgbm-devel \
     libiscsi-devel \
     libnfs-devel \
     libseccomp-devel \
     libudev-devel \
     pulseaudio-libs-devel \
     rdma-core-devel \
     wget \
     xfsprogs-devel

RUN dnf install -y $PACKAGES
RUN rpm -q $PACKAGES | sort > /packages.txt
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
---

sharing a big docker layer.

Re: [Qemu-devel] [PATCH 1/2] scripts/run-coverity-scan: Script to run Coverity Scan build

[Eric Blake]

On 11/13/18 1:21 PM, Peter Maydell wrote:

>>
>> set -e...
>>
>>> +check_upload_permissions() {
>>
>>
>> ...and shell functions do NOT intuitively do what you would think. It's
>> almost always better to use explicit error checking than to rely on set -e
>> as a crutch, because then you don't get surprises.
> 
> I think we had this conversation with last year's version
> of this script too :-)  As you say, the use of functions
> makes it maybe better to use explicit error checking -- but
> is there a standard syntax for that that doesn't make
> basic
>   foo
>   bar
>   baz
> "do these things in order" code look weird and require special care?
> At least with set -e you do get error checking, whereas scripts without
> it tend to just plough on regardless (look at configure, which doesn't
> use set -e but doesn't have explicit checking either).

I've seen both:

foo &&
bar &&
baz

and

foo || fail
bar || fail
baz || fail

for some variation of a 'fail' function.  But yeah, once you start 
having to worry about checking everything yourself (or realizing which 
lines don't need checking), you find out how much of a crutch 'set -e' 
tries to be, and then wonder how it ever worked (for the number of cases 
where 'set -e' does not actually catch failure, and cannot be re-enabled 
in smaller scopes).

>>> +TOOLBIN="$(cd "$COVERITY_TOOL_BASE" && echo
>>> $(pwd)/coverity_tool/cov-analysis-*/bin)"
>>
>>
>> If $CDPATH is set and $COVERITY_TOOL_BASE does not contain /, this could
>> result in garbage being prepended to TOOLBIN as output from the 'cd'. Also,
>> $PWD is nicer than $(pwd); but are you sure the glob in cov-analysis-* won't
>> select too many files?
> 
> The glob is not great, but it is necessary to make the script
> robust over new versions of the tools, which put the version
> number in the cov-analysis-whatever directory name. If
> we do ever get more than one file then the "test -x" below
> will fail, and we'll be able to investigate and fix up the script.

Yeah, I think you're okay on that front.

> 
> CDPATH sounds like a horrific misfeature designed for breaking
> scripts, so I'm not very interested in trying to work around it.
> We don't seem to worry about this in configure either.
> (I suppose we could just unset it at the start of the script.)

Autoconf 'configure' scripts do just that (unset CDPATH up front).  If 
someone ever complains that it actually broke for them, we'll make the 
fix; until then, I can live with the risk.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/2] scripts/coverity-scan: Add Docker support

[Alex Bennée]

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

> On 13/11/18 19:46, 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>
>> ---
>>   scripts/coverity-scan/coverity-scan.docker | 120 +++++++++++++++++++++
>>   scripts/coverity-scan/run-coverity-scan    |  58 ++++++++++
>>   2 files changed, 178 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..81f69459954
>> --- /dev/null
>> +++ b/scripts/coverity-scan/coverity-scan.docker
>> @@ -0,0 +1,120 @@
>> +# 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 28 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.
>> +# The work of actually doing the build is handled by the
>> +# run-coverity-scan script.
>> +
>> +
>> +FROM fedora:28
>> +ENV PACKAGES \
>> +    alsa-lib-devel \
>> +    bc \
>> +    bison \
>> +    bluez-libs-devel \
>> +    brlapi-devel \
>> +    bzip2 \
>> +    bzip2-devel \
>> +    ccache \
>> +    clang \
>> +    curl \
>> +    cyrus-sasl-devel \
>> +    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 \
>> +    libcap-devel \
>> +    libcap-ng-devel \
>> +    libcurl-devel \
>> +    libepoxy-devel \
>> +    libfdt-devel \
>> +    libgbm-devel \
>> +    libiscsi-devel \
>> +    libjpeg-devel \
>> +    libnfs-devel \
>> +    libpng-devel \
>> +    librbd-devel \
>> +    libseccomp-devel \
>> +    libssh2-devel \
>> +    libubsan \
>> +    libudev-devel \
>> +    libusbx-devel \
>> +    libxml2-devel \
>> +    llvm \
>> +    lzo-devel \
>> +    make \
>> +    mingw32-bzip2 \
>> +    mingw32-curl \
>> +    mingw32-glib2 \
>> +    mingw32-gmp \
>> +    mingw32-gnutls \
>> +    mingw32-gtk3 \
>> +    mingw32-libjpeg-turbo \
>> +    mingw32-libpng \
>> +    mingw32-libssh2 \
>> +    mingw32-libtasn1 \
>> +    mingw32-nettle \
>> +    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-libssh2 \
>> +    mingw64-libtasn1 \
>> +    mingw64-nettle \
>> +    mingw64-pixman \
>> +    mingw64-pkg-config \
>> +    mingw64-SDL2 \
>> +    ncurses-devel \
>> +    nettle-devel \
>> +    nss-devel \
>> +    numactl-devel \
>> +    perl \
>> +    pixman-devel \
>> +    pulseaudio-libs-devel \
>> +    python3 \
>> +    PyYAML \
>> +    rdma-core-devel \
>> +    SDL2-devel \
>> +    snappy-devel \
>> +    sparse \
>> +    spice-server-devel \
>> +    systemtap-sdt-devel \
>> +    tar \
>> +    usbredir-devel \
>> +    virglrenderer-devel \
>> +    vte3-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 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
>
> Calling "make docket-image-fedora" you can reduce this script to:

Remember for this to work we need to enforce the dependencies in the
tests/docker/Makefile.include and integrate into our make machinery.
Currently this dockerfile lives outside of the rest of our make
machinery.

We've talked about having Docker environments for building test pieces
before so I wonder if this is a good fit for expanding the make system
support for these sort of jobs?

>
> -- >8 --
> FROM qemu:fedora
> ENV PACKAGES \
>     $PACKAGES \
>     alsa-lib-devel \
>     curl \
>     cyrus-sasl-devel \
>     libepoxy-devel \
>     libgbm-devel \
>     libiscsi-devel \
>     libnfs-devel \
>     libseccomp-devel \
>     libudev-devel \
>     pulseaudio-libs-devel \
>     rdma-core-devel \
>     wget \
>     xfsprogs-devel
>
> RUN dnf install -y $PACKAGES
> RUN rpm -q $PACKAGES | sort > /packages.txt
> 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
> ---
>
> sharing a big docker layer.


--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/2] scripts/coverity-scan: Add Docker support

[Philippe Mathieu-Daudé]

On 14/11/18 12:25, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> On 13/11/18 19:46, 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>
>>> ---
>>>    scripts/coverity-scan/coverity-scan.docker | 120 +++++++++++++++++++++
>>>    scripts/coverity-scan/run-coverity-scan    |  58 ++++++++++
>>>    2 files changed, 178 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..81f69459954
>>> --- /dev/null
>>> +++ b/scripts/coverity-scan/coverity-scan.docker
>>> @@ -0,0 +1,120 @@
>>> +# 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 28 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.
>>> +# The work of actually doing the build is handled by the
>>> +# run-coverity-scan script.
>>> +
>>> +
>>> +FROM fedora:28
>>> +ENV PACKAGES \
>>> +    alsa-lib-devel \
>>> +    bc \
>>> +    bison \
>>> +    bluez-libs-devel \
>>> +    brlapi-devel \
>>> +    bzip2 \
>>> +    bzip2-devel \
>>> +    ccache \
>>> +    clang \
>>> +    curl \
>>> +    cyrus-sasl-devel \
>>> +    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 \
>>> +    libcap-devel \
>>> +    libcap-ng-devel \
>>> +    libcurl-devel \
>>> +    libepoxy-devel \
>>> +    libfdt-devel \
>>> +    libgbm-devel \
>>> +    libiscsi-devel \
>>> +    libjpeg-devel \
>>> +    libnfs-devel \
>>> +    libpng-devel \
>>> +    librbd-devel \
>>> +    libseccomp-devel \
>>> +    libssh2-devel \
>>> +    libubsan \
>>> +    libudev-devel \
>>> +    libusbx-devel \
>>> +    libxml2-devel \
>>> +    llvm \
>>> +    lzo-devel \
>>> +    make \
>>> +    mingw32-bzip2 \
>>> +    mingw32-curl \
>>> +    mingw32-glib2 \
>>> +    mingw32-gmp \
>>> +    mingw32-gnutls \
>>> +    mingw32-gtk3 \
>>> +    mingw32-libjpeg-turbo \
>>> +    mingw32-libpng \
>>> +    mingw32-libssh2 \
>>> +    mingw32-libtasn1 \
>>> +    mingw32-nettle \
>>> +    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-libssh2 \
>>> +    mingw64-libtasn1 \
>>> +    mingw64-nettle \
>>> +    mingw64-pixman \
>>> +    mingw64-pkg-config \
>>> +    mingw64-SDL2 \
>>> +    ncurses-devel \
>>> +    nettle-devel \
>>> +    nss-devel \
>>> +    numactl-devel \
>>> +    perl \
>>> +    pixman-devel \
>>> +    pulseaudio-libs-devel \
>>> +    python3 \
>>> +    PyYAML \
>>> +    rdma-core-devel \
>>> +    SDL2-devel \
>>> +    snappy-devel \
>>> +    sparse \
>>> +    spice-server-devel \
>>> +    systemtap-sdt-devel \
>>> +    tar \
>>> +    usbredir-devel \
>>> +    virglrenderer-devel \
>>> +    vte3-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 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
>>
>> Calling "make docket-image-fedora" you can reduce this script to:
> 
> Remember for this to work we need to enforce the dependencies in the
> tests/docker/Makefile.include and integrate into our make machinery.
> Currently this dockerfile lives outside of the rest of our make
> machinery.

Yes, but since this image is ran via a script which calls "docker build 
..." it could previously call "make docket-image-fedora".

Currenty the qemu:fedora layer takes a bit more than 2GB, space worth on 
laptop SSD ;)

> 
> We've talked about having Docker environments for building test pieces
> before so I wonder if this is a good fit for expanding the make system
> support for these sort of jobs?

I am not sure which of the various Docker talk you are thinking of...

For this particular case this is probably not worth integrating it into 
the make system.

However it makes sense to me to have the qemu:fedora and this image 
pushed. Probably worth another thread although.

> 
>>
>> -- >8 --
>> FROM qemu:fedora
>> ENV PACKAGES \
>>      $PACKAGES \
>>      alsa-lib-devel \
>>      curl \
>>      cyrus-sasl-devel \
>>      libepoxy-devel \
>>      libgbm-devel \
>>      libiscsi-devel \
>>      libnfs-devel \
>>      libseccomp-devel \
>>      libudev-devel \
>>      pulseaudio-libs-devel \
>>      rdma-core-devel \
>>      wget \
>>      xfsprogs-devel
>>
>> RUN dnf install -y $PACKAGES
>> RUN rpm -q $PACKAGES | sort > /packages.txt
>> 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
>> ---
>>
>> sharing a big docker layer.
> 
> 
> --
> Alex Bennée
> 

Re: [Qemu-devel] [PATCH 2/2] scripts/coverity-scan: Add Docker support

[Paolo Bonzini]

On 13/11/2018 20:37, Philippe Mathieu-Daudé wrote:
> Calling "make docket-image-fedora" you can reduce this script to:
> 
> -- >8 --
> FROM qemu:fedora
> ENV PACKAGES \
>     $PACKAGES \
>     alsa-lib-devel \
>     curl \
>     cyrus-sasl-devel \
>     libepoxy-devel \
>     libgbm-devel \
>     libiscsi-devel \
>     libnfs-devel \
>     libseccomp-devel \
>     libudev-devel \
>     pulseaudio-libs-devel \
>     rdma-core-devel \
>     wget \
>     xfsprogs-devel

... these can actually be moved to
tests/docker/dockerfiles/fedora.docker, improving the coverage...
> 
> RUN dnf install -y $PACKAGES
> RUN rpm -q $PACKAGES | sort > /packages.txt

... and removing the need for these two.

> 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

If the tokenfile is not needed when updating the tools, you could also
move the Dockerfile to tests/docker/dockerfiles/fedora-coverity.docker
and just do "make docker-image-fedora-coverity"

> +    # 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.

With make docker-image-*, you can just pass V=1.

Paolo

Re: [Qemu-devel] [PATCH 2/2] scripts/coverity-scan: Add Docker support

[Peter Maydell]

On 14 November 2018 at 12:02, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> 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
>
> If the tokenfile is not needed when updating the tools, you could also
> move the Dockerfile to tests/docker/dockerfiles/fedora-coverity.docker
> and just do "make docker-image-fedora-coverity"

It is needed when updating the tools, which is why I had
to mess around with the secrets to pass it in. (Specifically,
you have to pass the token to the Coverity web site to be
able to download the tools zipfile.)

thanks
-- PMM