[PATCH v4 0/3] migration & CI: Add a CI job for migration compat testing

[PATCH v4 0/3] migration & CI: Add a CI job for migration compat testing

[Fabiano Rosas]

Here's the second half of adding a migration compatibility test to CI.

We've already added support for running the full set of migration
tests with two QEMU binaries since commit 5050ad2a380
("tests/qtest/migration: Support more than one QEMU binary"), now
what's left is adding it to the CI.

changes since v3:

- Removed all the 'since' logic and started using the n-1 version of
  the tests.

- New patch to fix the aarch64 cpu. We shouldn't have been using
  'max'.

- New patch to disable aarch64 tests while the fix^ doesn't reach a
  released version.

v3:
https://lore.kernel.org/r/20240105180449.11562-1-farosas@suse.de
v2:
https://lore.kernel.org/r/20240104171857.20108-1-farosas@suse.de
v1:
https://lore.kernel.org/r/20231207155809.25673-1-farosas@suse.de

Fabiano Rosas (3):
  tests/qtest/migration: Don't use -cpu max for aarch64
  ci: Add a migration compatibility test job
  ci: Disable migration compatibility tests for aarch64

 .gitlab-ci.d/buildtest.yml   | 64 ++++++++++++++++++++++++++++++++++++
 tests/qtest/migration-test.c |  2 +-
 2 files changed, 65 insertions(+), 1 deletion(-)

-- 
2.35.3

[PATCH v4 1/3] tests/qtest/migration: Don't use -cpu max for aarch64

[Fabiano Rosas]

The 'max' cpu is not expected to be stable in terms of features across
QEMU versions, so it should not be expected to migrate.

While the tests currently all pass with -cpu max, that is only because
we're not testing across QEMU versions, which is the more common
use-case for migration.

We've recently introduced compatibility tests that use two different
QEMU versions and the tests are now failing for aarch64. The next
patch adds those tests to CI, so we cannot use the 'max' cpu
anymore. Replace it with the 'neoverse-n1', which has a fixed set of
features.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d3066e119f..b2390656f0 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -820,7 +820,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
         memory_size = "150M";
         machine_alias = "virt";
         machine_opts = "gic-version=max";
-        arch_opts = g_strdup_printf("-cpu max -kernel %s", bootpath);
+        arch_opts = g_strdup_printf("-cpu neoverse-n1 -kernel %s", bootpath);
         start_address = ARM_TEST_MEM_START;
         end_address = ARM_TEST_MEM_END;
     } else {
-- 
2.35.3

[PATCH v4 2/3] ci: Add a migration compatibility test job

[Fabiano Rosas]

The migration tests have support for being passed two QEMU binaries to
test migration compatibility.

Add a CI job that builds the lastest release of QEMU and another job
that uses that version plus an already present build of the current
version and run the migration tests with the two, both as source and
destination. I.e.:

 old QEMU (n-1) -> current QEMU (development tree)
 current QEMU (development tree) -> old QEMU (n-1)

The purpose of this CI job is to ensure the code we're about to merge
will not cause a migration compatibility problem when migrating the
next release (which will contain that code) to/from the previous
release.

The version of migration-test used will be the one matching the older
QEMU. That way we can avoid special-casing new tests that wouldn't be
compatible with the older QEMU.

Note: for user forks, the version tags need to be pushed to gitlab
otherwise it won't be able to checkout a different version.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 .gitlab-ci.d/buildtest.yml | 60 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index e1c7801598..f0b0edc634 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -167,6 +167,66 @@ build-system-centos:
       x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
     MAKE_CHECK_ARGS: check-build
 
+# Previous QEMU release. Used for cross-version migration tests.
+build-previous-qemu:
+  extends: .native_build_job_template
+  artifacts:
+    when: on_success
+    expire_in: 2 days
+    paths:
+      - build-previous
+    exclude:
+      - build-previous/**/*.p
+      - build-previous/**/*.a.p
+      - build-previous/**/*.fa.p
+      - build-previous/**/*.c.o
+      - build-previous/**/*.c.o.d
+      - build-previous/**/*.fa
+  needs:
+    job: amd64-opensuse-leap-container
+  variables:
+    IMAGE: opensuse-leap
+    TARGETS: x86_64-softmmu aarch64-softmmu
+  before_script:
+    - export QEMU_PREV_VERSION="$(sed 's/\([0-9.]*\)\.[0-9]*/v\1.0/' VERSION)"
+    - git checkout $QEMU_PREV_VERSION
+  after_script:
+    - mv build build-previous
+
+.migration-compat-common:
+  extends: .common_test_job_template
+  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.
+  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
+    # old to new
+    - QTEST_QEMU_BINARY_SRC=./qemu-system-${TARGET}
+          QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} ./tests/qtest/migration-test
+    # new to old
+    - QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
+          QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} ./tests/qtest/migration-test
+
+migration-compat-aarch64:
+  extends: .migration-compat-common
+  variables:
+    TARGET: aarch64
+
+migration-compat-x86_64:
+  extends: .migration-compat-common
+  variables:
+    TARGET: x86_64
+
 check-system-centos:
   extends: .native_test_job_template
   needs:
-- 
2.35.3

[PATCH v4 3/3] ci: Disable migration compatibility tests for aarch64

[Fabiano Rosas]

Until 9.0 is out, we need to keep the aarch64 job disabled because the
tests always use the n-1 version of migration-test. That happens to be
broken for aarch64 in 8.2. Once 9.0 is out, it will become the n-1
version and it will bring the fixed tests.

We can revert this patch when 9.0 releases.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 .gitlab-ci.d/buildtest.yml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index f0b0edc634..b344a4685f 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -217,10 +217,14 @@ build-previous-qemu:
     - QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
           QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} ./tests/qtest/migration-test
 
+# This job is disabled until we release 9.0. The existing
+# migration-test in 8.2 is broken on aarch64. The fix was already
+# commited, but it will only take effect once 9.0 is out.
 migration-compat-aarch64:
   extends: .migration-compat-common
   variables:
     TARGET: aarch64
+    QEMU_JOB_OPTIONAL: 1
 
 migration-compat-x86_64:
   extends: .migration-compat-common
-- 
2.35.3

Re: [PATCH v4 3/3] ci: Disable migration compatibility tests for aarch64

[Peter Xu]

On Thu, Jan 18, 2024 at 01:49:51PM -0300, Fabiano Rosas wrote:
> Until 9.0 is out, we need to keep the aarch64 job disabled because the
> tests always use the n-1 version of migration-test. That happens to be
> broken for aarch64 in 8.2. Once 9.0 is out, it will become the n-1
> version and it will bring the fixed tests.
> 
> We can revert this patch when 9.0 releases.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  .gitlab-ci.d/buildtest.yml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index f0b0edc634..b344a4685f 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -217,10 +217,14 @@ build-previous-qemu:
>      - QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
>            QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} ./tests/qtest/migration-test
>  
> +# This job is disabled until we release 9.0. The existing
> +# migration-test in 8.2 is broken on aarch64. The fix was already
> +# commited, but it will only take effect once 9.0 is out.
>  migration-compat-aarch64:
>    extends: .migration-compat-common
>    variables:
>      TARGET: aarch64
> +    QEMU_JOB_OPTIONAL: 1

Optionally you can move the whole migration-compat-aarch64 from previous
patch to here, then even if someone kicks off CI for previous commit it
won't fail.

Would QEMU_JOB_SKIPPED suits more here?

        https://www.qemu.org/docs/master/devel/ci.html

        QEMU_JOB_OPTIONAL

        The job is expected to be successful in general, but is not run by
        default due to need to conserve limited CI resources. It is
        available to be started manually by the contributor in the CI
        pipelines UI.

        QEMU_JOB_SKIPPED

        The job is not reliably successsful in general, so is not currently
        suitable to be run by default. Ideally this should be a temporary
        marker until the problems can be addressed, or the job permanently
        removed.

I suppose they all fall into "manual trigger" as a result, but just in case
it'll behave differently in the future.

>  
>  migration-compat-x86_64:
>    extends: .migration-compat-common
> -- 
> 2.35.3
> 

-- 
Peter Xu

Re: [PATCH v4 0/3] migration & CI: Add a CI job for migration compat testing

[Peter Xu]

Fabiano,

On Thu, Jan 18, 2024 at 01:49:48PM -0300, Fabiano Rosas wrote:
> Here's the second half of adding a migration compatibility test to CI.
> 
> We've already added support for running the full set of migration
> tests with two QEMU binaries since commit 5050ad2a380
> ("tests/qtest/migration: Support more than one QEMU binary"), now
> what's left is adding it to the CI.
> 
> changes since v3:
> 
> - Removed all the 'since' logic and started using the n-1 version of
>   the tests.
> 
> - New patch to fix the aarch64 cpu. We shouldn't have been using
>   'max'.
> 
> - New patch to disable aarch64 tests while the fix^ doesn't reach a
>   released version.
> 
> v3:
> https://lore.kernel.org/r/20240105180449.11562-1-farosas@suse.de
> v2:
> https://lore.kernel.org/r/20240104171857.20108-1-farosas@suse.de
> v1:
> https://lore.kernel.org/r/20231207155809.25673-1-farosas@suse.de

I gave it a try but build-previous-qemu somehow failed..

  https://gitlab.com/peterx/qemu/-/jobs/5965634871

You normally attach a "CI run" which I wanted to reference, but it's gone
in this v4 unfortunately.  The error:

  error: pathspec 'v8.2.0' did not match any file(s) known to git

is pretty weird, and I had a feeling it was something wrong before that.
Maybe it's because I tried to run this specific test multiple times (and
all failed), somehow the wrong thing was cached, so the real error (of the
1st run when there is no cache) could have got lost.  Not sure.

Please have a look when you have time, thanks.

> 
> Fabiano Rosas (3):
>   tests/qtest/migration: Don't use -cpu max for aarch64
>   ci: Add a migration compatibility test job
>   ci: Disable migration compatibility tests for aarch64
> 
>  .gitlab-ci.d/buildtest.yml   | 64 ++++++++++++++++++++++++++++++++++++
>  tests/qtest/migration-test.c |  2 +-
>  2 files changed, 65 insertions(+), 1 deletion(-)
> 
> -- 
> 2.35.3
> 

-- 
Peter Xu

Re: [PATCH v4 0/3] migration & CI: Add a CI job for migration compat testing

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> Fabiano,
>
> On Thu, Jan 18, 2024 at 01:49:48PM -0300, Fabiano Rosas wrote:
>> Here's the second half of adding a migration compatibility test to CI.
>> 
>> We've already added support for running the full set of migration
>> tests with two QEMU binaries since commit 5050ad2a380
>> ("tests/qtest/migration: Support more than one QEMU binary"), now
>> what's left is adding it to the CI.
>> 
>> changes since v3:
>> 
>> - Removed all the 'since' logic and started using the n-1 version of
>>   the tests.
>> 
>> - New patch to fix the aarch64 cpu. We shouldn't have been using
>>   'max'.
>> 
>> - New patch to disable aarch64 tests while the fix^ doesn't reach a
>>   released version.
>> 
>> v3:
>> https://lore.kernel.org/r/20240105180449.11562-1-farosas@suse.de
>> v2:
>> https://lore.kernel.org/r/20240104171857.20108-1-farosas@suse.de
>> v1:
>> https://lore.kernel.org/r/20231207155809.25673-1-farosas@suse.de
>
> I gave it a try but build-previous-qemu somehow failed..
>
>   https://gitlab.com/peterx/qemu/-/jobs/5965634871
>
> You normally attach a "CI run" which I wanted to reference, but it's gone
> in this v4 unfortunately.  The error:
>
>   error: pathspec 'v8.2.0' did not match any file(s) known to git

I mentioned this at the bottom of the commit message for patch 2/3. You
need to push your tags. Otherwise your fork on gitlab won't have
knowledge of v8.2.0.

Re: [PATCH v4 3/3] ci: Disable migration compatibility tests for aarch64

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Thu, Jan 18, 2024 at 01:49:51PM -0300, Fabiano Rosas wrote:
>> Until 9.0 is out, we need to keep the aarch64 job disabled because the
>> tests always use the n-1 version of migration-test. That happens to be
>> broken for aarch64 in 8.2. Once 9.0 is out, it will become the n-1
>> version and it will bring the fixed tests.
>> 
>> We can revert this patch when 9.0 releases.
>> 
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  .gitlab-ci.d/buildtest.yml | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
>> index f0b0edc634..b344a4685f 100644
>> --- a/.gitlab-ci.d/buildtest.yml
>> +++ b/.gitlab-ci.d/buildtest.yml
>> @@ -217,10 +217,14 @@ build-previous-qemu:
>>      - QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
>>            QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} ./tests/qtest/migration-test
>>  
>> +# This job is disabled until we release 9.0. The existing
>> +# migration-test in 8.2 is broken on aarch64. The fix was already
>> +# commited, but it will only take effect once 9.0 is out.
>>  migration-compat-aarch64:
>>    extends: .migration-compat-common
>>    variables:
>>      TARGET: aarch64
>> +    QEMU_JOB_OPTIONAL: 1
>
> Optionally you can move the whole migration-compat-aarch64 from previous
> patch to here, then even if someone kicks off CI for previous commit it
> won't fail.

I want to avoid having to think about this when 9.0 is out. This way
it'll be just a revert of this patch.

>
> Would QEMU_JOB_SKIPPED suits more here?
>
>         https://www.qemu.org/docs/master/devel/ci.html
>
>         QEMU_JOB_OPTIONAL
>
>         The job is expected to be successful in general, but is not run by
>         default due to need to conserve limited CI resources. It is
>         available to be started manually by the contributor in the CI
>         pipelines UI.
>
>         QEMU_JOB_SKIPPED
>
>         The job is not reliably successsful in general, so is not currently
>         suitable to be run by default. Ideally this should be a temporary
>         marker until the problems can be addressed, or the job permanently
>         removed.
>
> I suppose they all fall into "manual trigger" as a result, but just in case
> it'll behave differently in the future.
>
Ok, we can use SKIPPED.

>>  
>>  migration-compat-x86_64:
>>    extends: .migration-compat-common
>> -- 
>> 2.35.3
>> 

Re: [PATCH v4 3/3] ci: Disable migration compatibility tests for aarch64

[Peter Xu]

On Fri, Jan 19, 2024 at 10:04:56AM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Thu, Jan 18, 2024 at 01:49:51PM -0300, Fabiano Rosas wrote:
> >> Until 9.0 is out, we need to keep the aarch64 job disabled because the
> >> tests always use the n-1 version of migration-test. That happens to be
> >> broken for aarch64 in 8.2. Once 9.0 is out, it will become the n-1
> >> version and it will bring the fixed tests.
> >> 
> >> We can revert this patch when 9.0 releases.
> >> 
> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >> ---
> >>  .gitlab-ci.d/buildtest.yml | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> >> index f0b0edc634..b344a4685f 100644
> >> --- a/.gitlab-ci.d/buildtest.yml
> >> +++ b/.gitlab-ci.d/buildtest.yml
> >> @@ -217,10 +217,14 @@ build-previous-qemu:
> >>      - QTEST_QEMU_BINARY_DST=./qemu-system-${TARGET}
> >>            QTEST_QEMU_BINARY=../build/qemu-system-${TARGET} ./tests/qtest/migration-test
> >>  
> >> +# This job is disabled until we release 9.0. The existing
> >> +# migration-test in 8.2 is broken on aarch64. The fix was already
> >> +# commited, but it will only take effect once 9.0 is out.
> >>  migration-compat-aarch64:
> >>    extends: .migration-compat-common
> >>    variables:
> >>      TARGET: aarch64
> >> +    QEMU_JOB_OPTIONAL: 1
> >
> > Optionally you can move the whole migration-compat-aarch64 from previous
> > patch to here, then even if someone kicks off CI for previous commit it
> > won't fail.
> 
> I want to avoid having to think about this when 9.0 is out. This way
> it'll be just a revert of this patch.

That's ok.

> 
> >
> > Would QEMU_JOB_SKIPPED suits more here?
> >
> >         https://www.qemu.org/docs/master/devel/ci.html
> >
> >         QEMU_JOB_OPTIONAL
> >
> >         The job is expected to be successful in general, but is not run by
> >         default due to need to conserve limited CI resources. It is
> >         available to be started manually by the contributor in the CI
> >         pipelines UI.
> >
> >         QEMU_JOB_SKIPPED
> >
> >         The job is not reliably successsful in general, so is not currently
> >         suitable to be run by default. Ideally this should be a temporary
> >         marker until the problems can be addressed, or the job permanently
> >         removed.
> >
> > I suppose they all fall into "manual trigger" as a result, but just in case
> > it'll behave differently in the future.
> >
> Ok, we can use SKIPPED.

No need to repost then; if this is the only thing to change I can do it
when queuing.  Will wait a bit more for comment on patch 2.

-- 
Peter Xu
 

Re: [PATCH v4 0/3] migration & CI: Add a CI job for migration compat testing

[Peter Xu]

On Fri, Jan 19, 2024 at 10:01:31AM -0300, Fabiano Rosas wrote:
> I mentioned this at the bottom of the commit message for patch 2/3. You
> need to push your tags. Otherwise your fork on gitlab won't have
> knowledge of v8.2.0.

Oops, my fault.  Yeah it works now; I queued this into staging.

Thanks,

-- 
Peter Xu