Re: [PULL 06/14] ci: Add a migration compatibility test job

[Fabiano Rosas <farosas@suse.de>]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 29 Jan 2024 at 03:04, <peterx@redhat.com> wrote:
>>
>> From: Fabiano Rosas <farosas@suse.de>
>>
>> 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>
>> Link: https://lore.kernel.org/r/20240118164951.30350-3-farosas@suse.de
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>> ---
>>  .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
>
> There seems to be a problem with this new CI job. Running a CI
> run in my local repository it fails:
>
> https://gitlab.com/pm215/qemu/-/jobs/6075873685
>
> $ export QEMU_PREV_VERSION="$(sed 's/\([0-9.]*\)\.[0-9]*/v .0/' VERSION)"
> $ git checkout $QEMU_PREV_VERSION
> error: pathspec 'v8.2.0' did not match any file(s) known to git
> Running after_script
> Running after script...
> $ mv build build-previous
> mv: cannot stat 'build': No such file or directory
> WARNING: after_script failed, but job will continue unaffected: exit code 1
> Saving cache for failed job
>
>
> I don't think you can assume that private forks doing submaintainer CI
> runs necessarily have the full set of tags that the main repo does.

Yes, I thought this would be rare enough not to be an issue, but it
seems it's not. I don't know what could be done here, if there's no tag,
then there's no way to resolve the actual commit hash I think.

> I suspect the sed run will also do the wrong thing when run on the
> commit that updates the version, because then it will replace
> "9.0.0" with "9.0.0".

I just ignored this completly because my initial idea was to leave this
job disabled and only run it for migration patchsets and pull requests,
so it wouldn't make sense to run at that commit.

This job is also not entirely fail proof by design because we could
always be hitting bugs in the older QEMU version that were already fixed
in the new version.

I think the simplest fix here is to leave the test disabled, possibly
with an env variable to enable it.