qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Ilya Leoshkevich <iii@linux.ibm.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Thomas Huth" <thuth@redhat.com>,
	"Wainer dos Santos Moschetta" <wainersm@redhat.com>,
	"Beraldo Leal" <bleal@redhat.com>,
	"Cornelia Huck" <cohuck@redhat.com>,
	qemu-devel@nongnu.org, qemu-s390x@nongnu.org,
	"Christian Borntraeger" <borntraeger@de.ibm.com>,
	"Ulrich Weigand" <ulrich.weigand@de.ibm.com>
Subject: Re: [RFC] gitlab: introduce s390x wasmtime job
Date: Tue, 5 Jul 2022 13:58:38 +0100	[thread overview]
Message-ID: <YsQ1fuMRPpA+9AzX@redhat.com> (raw)
In-Reply-To: <20220704224844.2903473-1-iii@linux.ibm.com>

On Tue, Jul 05, 2022 at 12:48:44AM +0200, Ilya Leoshkevich wrote:
> wasmtime is a WebAssembly runtime, which includes a large testsuite.
> This testsuite uses qemu-user (aarch64 and s390x are supported) in
> order to exercise foreign architectures.

So you're saying that WebAssembly itself is aware of qemu-user
in its test suite, as opposed to us simply choosing to run its
test suite under qemu-user ?

Any idea why its limited to just two arches ?  Can it be made
to cover all QEMU arches, or are these the only ones that
wasmtime knows how to generate code for ?

>                                          Over time it found several
> regressions in qemu itself, and it would be beneficial to catch the
> similar ones earlier.

If we put this job in QEMU CI someone will have to be able to
interpret the results when it fails.

How practical is it going to be for QEMU maintainers to understand
a failure in wasmtime test suite, and correlate that back to a
problem in QEMU ? The risk with introducing any significant 3rd
party project to a CI system, is the lack of knowledge around
that external project creating a signifcant burden for the CI
system maintainers.

> To this end, this patch introduces a job that runs stable wasmtime
> testsuite against qemu-s390x. The job is split into the following
> components:
> 
> - A script for running the tests. Usable on developers' machines:
> 
>     qemu$ mkdir build
>     qemu$ cd build
>     qemu/build$ ../tests/wasmtime/test s390x
> 
> - A script for building the tests (build-toolchain.sh).
> 
> - A dockerfile describing an image with the prebuilt testsuite
>   (debian-s390x-wasmtime-cross.docker).
> 
> - gitlab job definition for building the image.
> 
> - gitlab job definition for using the image to run the tests.
> 
> It's possible to use this with aarch64 as well, but it segfaults at
> the moment, therefore this patch does not provide job definitions for
> it. This needs to be investigated separately.
> 
> The example of a resulting pipeline can be seen at [1].
> 
> The test job runs for about 30 minutes mostly due to unnecessary
> rebuilds. They will be gone once [2] is integrated and makes it to a
> stable release.
> 
> This patch depends on madvise(MADV_DONTNEED) passthrough support [3].
> 
> [1] https://gitlab.com/iii-i/qemu/-/pipelines/579677396
> [2] https://github.com/bytecodealliance/wasmtime/pull/4377
> [3] https://lists.gnu.org/archive/html/qemu-devel/2022-07/msg00112.html
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  .gitlab-ci.d/container-cross.yml              | 10 +++
>  .gitlab-ci.d/container-template.yml           |  2 +-
>  .gitlab-ci.d/qemu-project.yml                 |  1 +
>  .gitlab-ci.d/wasmtime-template.yml            |  6 ++
>  .gitlab-ci.d/wasmtime.yml                     |  9 ++
>  tests/docker/Makefile.include                 |  6 ++
>  .../build-toolchain.sh                        | 83 +++++++++++++++++++
>  .../debian-s390x-wasmtime-cross.docker        | 16 ++++
>  tests/wasmtime/test                           | 39 +++++++++
>  9 files changed, 171 insertions(+), 1 deletion(-)
>  create mode 100644 .gitlab-ci.d/wasmtime-template.yml
>  create mode 100644 .gitlab-ci.d/wasmtime.yml
>  create mode 100755 tests/docker/dockerfiles/debian-s390x-wasmtime-cross.d/build-toolchain.sh
>  create mode 100644 tests/docker/dockerfiles/debian-s390x-wasmtime-cross.docker
>  create mode 100755 tests/wasmtime/test
> 
> diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml
> index b7963498a3..b3c4b76a16 100644
> --- a/.gitlab-ci.d/container-cross.yml
> +++ b/.gitlab-ci.d/container-cross.yml
> @@ -138,6 +138,16 @@ s390x-debian-cross-container:
>    variables:
>      NAME: debian-s390x-cross
>  
> +s390x-debian-wasmtime-cross-container:
> +  extends: .container_job_template
> +  stage: containers
> +  needs: ['s390x-debian-cross-container']

This job is in 'containers' stage, but it depends on a container
built in the same stage. Seems gitlab manages to schedule that
by effectively delaying the the build. Suggests that actually we
can entirely get rid of 'containers-layer2' and just let gitlab
figure out the container dependencies.

> +  variables:
> +    NAME: debian-s390x-wasmtime-cross
> +    DOCKER_SCRIPT_ARGS: >
> +      --extra-files
> +      tests/docker/dockerfiles/debian-s390x-wasmtime-cross.d/build-toolchain.sh


> diff --git a/tests/docker/dockerfiles/debian-s390x-wasmtime-cross.d/build-toolchain.sh b/tests/docker/dockerfiles/debian-s390x-wasmtime-cross.d/build-toolchain.sh
> new file mode 100755
> index 0000000000..a28d61a353
> --- /dev/null
> +++ b/tests/docker/dockerfiles/debian-s390x-wasmtime-cross.d/build-toolchain.sh
> @@ -0,0 +1,83 @@
> +#!/bin/sh
> +
> +# Build the stable wasmtime testsuite and run it with qemu-user from $PATH.
> +# ".rustup", ".cargo" and "wasmtime" subdirectories will be created or updated
> +# in the current directory.
> +#
> +# Based on https://github.com/bytecodealliance/wasmtime/blob/v0.37.0/.github/workflows/main.yml#L208.
> +#
> +# Usage:
> +#
> +#     ./test TARGET_ARCH [CARGO_ARGS ...]
> +#
> +# where TARGET_ARCH is the architecture to test (aarch64 or s390x) and
> +# CARGO_ARGS are the extra arguments passed to cargo test.
> +
> +set -e -u -x
> +
> +# Dependency versions.
> +export RUSTUP_TOOLCHAIN=1.62.0
> +
> +# Bump when https://github.com/bytecodealliance/wasmtime/pull/4377 is
> +# integrated. Until this moment there will be some unnecessary rebuilds.
> +wasmtime_version=0.37.0
> +
> +# Script arguments.
> +arch=$1
> +shift
> +arch_upper=$(echo "$arch" | tr '[:lower:]' '[:upper:]')
> +
> +# Install/update Rust.
> +export RUSTUP_HOME="$PWD/.rustup"
> +export CARGO_HOME="$PWD/.cargo"
> +curl \
> +    --proto '=https' \
> +    --tlsv1.2 \
> +    -sSf \
> +    https://sh.rustup.rs \
> +    | sh -s -- -y \
> +        --default-toolchain="$RUSTUP_TOOLCHAIN" \
> +        --target=wasm32-wasi \
> +        --target=wasm32-unknown-unknown \
> +        --target="$arch"-unknown-linux-gnu

Why can't we just install the distros' rust packages ?


> +cat >"$CARGO_HOME/config" <<HERE
> +[build]
> +# Save space by not generating data to speed-up delta builds.
> +incremental = false
> +
> +[profile.test]
> +# Save space by not generating debug information.
> +debug = 0
> +
> +[net]
> +# Speed up crates.io index update.
> +git-fetch-with-cli = true
> +HERE
> +. "$PWD/.cargo/env"
> +
> +# Checkout/update wasmtime.
> +if [ -d wasmtime ]; then
> +    cd wasmtime
> +    git fetch --force --tags
> +    git checkout v"$wasmtime_version"
> +    git submodule update --init --recursive
> +else
> +    git clone \
> +        --depth=1 \
> +        --recurse-submodules \
> +        --shallow-submodules \
> +        -b v"$wasmtime_version" \
> +        https://github.com/bytecodealliance/wasmtime.git
> +    cd wasmtime
> +fi
> +
> +# Run wasmtime tests.
> +export CARGO_BUILD_TARGET="$arch-unknown-linux-gnu"
> +runner_var=CARGO_TARGET_${arch_upper}_UNKNOWN_LINUX_GNU_RUNNER
> +linker_var=CARGO_TARGET_${arch_upper}_UNKNOWN_LINUX_GNU_LINKER
> +eval "export $runner_var=\"qemu-$arch -L /usr/$arch-linux-gnu\""
> +eval "export $linker_var=$arch-linux-gnu-gcc"
> +export CARGO_PROFILE_DEV_OPT_LEVEL=2
> +export WASMTIME_TEST_NO_HOG_MEMORY=1
> +export RUST_BACKTRACE=1
> +ci/run-tests.sh --locked "$@"

This build-toolchain.sh script is invoked during the dockerfile
build stage, but it appears you're running the test suite here.
Shouldn't this be left until the CI build job instead ?

> diff --git a/tests/docker/dockerfiles/debian-s390x-wasmtime-cross.docker b/tests/docker/dockerfiles/debian-s390x-wasmtime-cross.docker
> new file mode 100644
> index 0000000000..d08a66dcc2
> --- /dev/null
> +++ b/tests/docker/dockerfiles/debian-s390x-wasmtime-cross.docker
> @@ -0,0 +1,16 @@
> +# Image containing pre-built wasmtime tests for s390x.
> +
> +FROM registry.gitlab.com/qemu-project/qemu/qemu/debian-s390x-cross:latest
> +
> +RUN export DEBIAN_FRONTEND=noninteractive && \
> +    eatmydata apt-get update && \
> +    eatmydata apt-get dist-upgrade -y && \
> +    eatmydata apt-get install --no-install-recommends -y \
> +            curl \
> +            libglib2.0-dev && \
> +    eatmydata apt-get autoremove -y && \
> +    eatmydata apt-get autoclean -y
> +
> +RUN mkdir /build
> +ADD build-toolchain.sh /build
> +RUN cd /build && ./build-toolchain.sh s390x --no-run


Is this '--no-run' arg used by  ci/run-tests.sh in some way ?


> diff --git a/tests/wasmtime/test b/tests/wasmtime/test
> new file mode 100755
> index 0000000000..10e2c3f886
> --- /dev/null
> +++ b/tests/wasmtime/test
> @@ -0,0 +1,39 @@
> +#!/bin/sh
> +
> +# Build qemu-user in the current directory, build the stable wasmtime
> +# testsuite, and test them together. ".rustup", ".cargo" and "wasmtime"
> +# subdirectories, as well as qemu build files, will be created or updated in
> +# the current directory.
> +#
> +# Based on https://github.com/bytecodealliance/wasmtime/blob/v0.37.0/.github/workflows/main.yml#L208.
> +#
> +# Usage:
> +#
> +#     ./test TARGET_ARCH [CARGO_ARGS ...]
> +#
> +# where TARGET_ARCH is the architecture to test (aarch64 or s390x) and
> +# CARGO_ARGS are the extra arguments passed to cargo test.
> +
> +set -e -u -x
> +
> +# Script arguments.
> +arch=$1
> +shift
> +
> +# Build QEMU.
> +srcdir=$(cd "$(dirname "$0")" && pwd)/../..
> +docker_files_dir="$srcdir"/tests/docker/dockerfiles
> +"$srcdir"/configure \
> +    --target-list="$arch"-linux-user \
> +    --disable-tools \
> +    --disable-slirp \
> +    --disable-fdt \
> +    --disable-capstone \
> +    --disable-docs
> +make --output-sync -j"$(nproc)"
> +export PATH="$PWD:$PATH"
> +
> +# Run wasmtime tests.
> +exec \
> +    "$docker_files_dir"/debian-s390x-wasmtime-cross.d/build-toolchain.sh \
> +    "$arch" "$@"
> -- 
> 2.35.3
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2022-07-05 13:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-04 22:48 [RFC] gitlab: introduce s390x wasmtime job Ilya Leoshkevich
2022-07-05 12:58 ` Daniel P. Berrangé [this message]
2022-07-05 13:57   ` Peter Maydell
2022-07-05 14:37     ` Ilya Leoshkevich
2022-07-05 14:40       ` Peter Maydell
2022-07-05 20:41         ` Ilya Leoshkevich
2022-12-16 15:10           ` Alex Bennée
2022-12-19 21:42             ` Ilya Leoshkevich
2022-12-19 22:18               ` Alex Bennée
2022-07-05 14:13   ` Ilya Leoshkevich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YsQ1fuMRPpA+9AzX@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.bennee@linaro.org \
    --cc=bleal@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=cohuck@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=iii@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=thuth@redhat.com \
    --cc=ulrich.weigand@de.ibm.com \
    --cc=wainersm@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).