Re: [RFC PATCH 4/4] ci: Add the new migration device tests

[Peter Xu <peterx@redhat.com>]

On Thu, May 23, 2024 at 05:19:22PM -0300, Fabiano Rosas wrote:
> We have two new migration tests that check cross version
> compatibility. One uses the vmstate-static-checker.py script to
> compare the vmstate structures from two different QEMU versions. The
> other runs a simple migration with a few devices present in the VM, to
> catch obvious breakages.
> 
> Add both tests to the migration-compat-common job.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  .gitlab-ci.d/buildtest.yml | 43 +++++++++++++++++++++++++++++++-------
>  1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 91c57efded..bc7ac35983 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -202,18 +202,47 @@ build-previous-qemu:
>    needs:
>      - job: build-previous-qemu
>      - job: build-system-opensuse
> -  # The old QEMU could have bugs unrelated to migration that are
> -  # already fixed in the current development branch, so this test
> -  # might fail.
> +  # This test is allowed to fail because:
> +  #
> +  # - The old QEMU could have bugs unrelated to migration that are
> +  #   already fixed in the current development branch.

Did you ever hit a real failure with this?  I'm wondering whether we can
remove this allow_failure thing.

> +  #
> +  # - The vmstate-static-checker script trips on renames and other
> +  #   backward-compatible changes to the vmstate structs.

I think I keep my preference per last time we talked on this. :)

I still think it's too early to involve a test that can report false
negative.  I'd still keep running this before soft-freeze like I used to
do, throw issues to others and urge them to fix before release.  Per my
previous experience that doesn't consume me a lot of time, and it's not
common to see issues either.

So I want people to really pay attention when someone sees a migration CI
test failed, rather than we help people form the habit in "oh migration CI
failed again?  I think that's fine, it allows failing anyway".

So far I still don't see as much benefit to adding this if we need to pay
for the other false negative issue.  I'll fully support it if e.g. we can
fix the tool to avoid reporting false negatives, but that may take effort
that I didn't check.

>    allow_failure: true
>    variables:
>      IMAGE: opensuse-leap
>      MAKE_CHECK_ARGS: check-build
>    script:
> -    # Use the migration-tests from the older QEMU tree. This avoids
> -    # testing an old QEMU against new features/tests that it is not
> -    # compatible with.
> -    - cd build-previous
> +    - cd build
> +    # device state static test: Tests the vmstate structures for
> +    # compatibility across QEMU versions. Uses the latest version of
> +    # the tests.
> +    # old to new
> +    - PYTHON=pyvenv/bin/python3
> +          QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-${TARGET}
> +          QTEST_QEMU_BINARY=./qemu-system-${TARGET}
> +          ./tests/qtest/migration-test -p /${TARGET}/migration/vmstate-checker-script
> +    # new to old skipped because vmstate version bumps are always
> +    # backward incompatible.
> +
> +    # device state runtime test: Performs a cross-version migration
> +    # with a select list of devices (see DEFAULT_DEVICES in
> +    # migration-test.c). Using the multifd tcp test here, but any will
> +    # do.
> +    # old to new
> +    - QTEST_DEVICE_OPTS=all QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-${TARGET}
> +          QTEST_QEMU_BINARY=./qemu-system-${TARGET} ./tests/qtest/migration-test
> +          -p /${TARGET}/migration/multifd/tcp/channels/plain/none
> +    # new to old
> +    - QTEST_DEVICE_OPTS=all QTEST_QEMU_BINARY_DST=../build-previous/qemu-system-${TARGET}
> +          QTEST_QEMU_BINARY=./qemu-system-${TARGET} ./tests/qtest/migration-test
> +          -p /${TARGET}/migration/multifd/tcp/channels/plain/none
> +
> +    # migration core tests: Use the migration-tests from the older
> +    # QEMU tree. This avoids testing an old QEMU against new
> +    # features/tests that it is not compatible with.
> +    - cd ../build-previous
>      # old to new
>      - QTEST_QEMU_BINARY_SRC=./qemu-system-${TARGET}
>            QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} ./tests/qtest/migration-test
> -- 
> 2.35.3
> 

-- 
Peter Xu