| Message ID | 20220922134058.1410-2-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | GitLab CI cleanup & improvements for Arm | expand |
On Thu, 22 Sep 2022, Michal Orzel wrote: > Currently, all the arm64 defconfig build jobs, regardless of the > container used, end up building Xen with the extra config options > specified in the main build script (e.g. CONFIG_EXPERT, > CONFIG_STATIC_MEMORY). Because these options are only needed for > specific test jobs, the current behavior of the CI is incorrect > as we add the extra options to all the defconfig builds. This means > that on arm64 there is not a single job performing proper defconfig build. > > To fix this issue, add custom build jobs each time there is a need for > building Xen with additional config options. Introduce EXTRA_XEN_CONFIG > variable to be used by these jobs to store the required options. This > variable will be then read by the main build script to modify the .config > file. This will also help users to understand what is needed to run specific > test. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > --- > This patch could actually be consider to be taken for 4.17 release. > The reason why is because currently our CI for arm64 does not even > peform clean defconfig build which is quite crucial target to be tested. > Performing builds always with EXPERT and UNSUPPORTED is not something so > beneficial for release tests. This is up to the release manager. + Henry I agree this should go in 4.17, so that gitlab-ci can test non-DEBUG builds on ARM. > --- > automation/gitlab-ci/build.yaml | 15 +++++++++++++++ > automation/gitlab-ci/test.yaml | 4 ++-- > automation/scripts/build | 8 ++------ > 3 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > index 720ce6e07ba0..a39ed72aac6d 100644 > --- a/automation/gitlab-ci/build.yaml > +++ b/automation/gitlab-ci/build.yaml > @@ -566,6 +566,21 @@ alpine-3.12-gcc-debug-arm64: > variables: > CONTAINER: alpine:3.12-arm64v8 > > +alpine-3.12-gcc-arm64-staticmem: > + extends: .gcc-arm64-build > + variables: > + CONTAINER: alpine:3.12-arm64v8 > + EXTRA_XEN_CONFIG: | Why the "|" ? I was trying to look for its documentation in the gitlab yaml docs but couldn't find it. > + CONFIG_EXPERT=y > + CONFIG_UNSUPPORTED=y > + CONFIG_STATIC_MEMORY=y > + > +alpine-3.12-gcc-arm64-boot-cpupools: > + extends: .gcc-arm64-build > + variables: > + CONTAINER: alpine:3.12-arm64v8 > + EXTRA_XEN_CONFIG: | > + CONFIG_BOOT_TIME_CPUPOOLS=y > > ## Test artifacts common > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index d899b3bdbf7a..4f96e6e322de 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -88,7 +88,7 @@ qemu-smoke-arm64-gcc-staticmem: > script: > - ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log > needs: > - - alpine-3.12-gcc-arm64 > + - alpine-3.12-gcc-arm64-staticmem > - alpine-3.12-arm64-rootfs-export > - kernel-5.19-arm64-export > - qemu-system-aarch64-6.0.0-arm64-export > @@ -107,7 +107,7 @@ qemu-smoke-arm64-gcc-boot-cpupools: > script: > - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log > needs: > - - alpine-3.12-gcc-arm64 > + - alpine-3.12-gcc-arm64-boot-cpupools > - alpine-3.12-arm64-rootfs-export > - kernel-5.19-arm64-export > - qemu-system-aarch64-6.0.0-arm64-export > diff --git a/automation/scripts/build b/automation/scripts/build > index 2f15ab3198e6..7d441cedb4ae 100755 > --- a/automation/scripts/build > +++ b/automation/scripts/build > @@ -15,12 +15,8 @@ if [[ "${RANDCONFIG}" == "y" ]]; then > make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig > hypervisor_only="y" > else > - if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then > - echo " > -CONFIG_EXPERT=y > -CONFIG_UNSUPPORTED=y > -CONFIG_STATIC_MEMORY=y > -CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config > + if [ -n "${EXTRA_XEN_CONFIG}" ]; then NIT: for uniformity with rest of the file use if [[ -n "${EXTRA_XEN_CONFIG}" ]]; then > + echo "${EXTRA_XEN_CONFIG}" > xen/.config > make -j$(nproc) -C xen olddefconfig > else > make -j$(nproc) -C xen defconfig > -- > 2.25.1 >
Hi Stefano, > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Subject: Re: [PATCH 1/9] automation: Use custom build jobs when extra > config options are needed > > On Thu, 22 Sep 2022, Michal Orzel wrote: > > Currently, all the arm64 defconfig build jobs, regardless of the > > container used, end up building Xen with the extra config options > > specified in the main build script (e.g. CONFIG_EXPERT, > > CONFIG_STATIC_MEMORY). Because these options are only needed for > > specific test jobs, the current behavior of the CI is incorrect > > as we add the extra options to all the defconfig builds. This means > > that on arm64 there is not a single job performing proper defconfig build. > > > > To fix this issue, add custom build jobs each time there is a need for > > building Xen with additional config options. Introduce EXTRA_XEN_CONFIG > > variable to be used by these jobs to store the required options. This > > variable will be then read by the main build script to modify the .config > > file. This will also help users to understand what is needed to run specific > > test. > > > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > > --- > > This patch could actually be consider to be taken for 4.17 release. > > The reason why is because currently our CI for arm64 does not even > > peform clean defconfig build which is quite crucial target to be tested. > > Performing builds always with EXPERT and UNSUPPORTED is not something > so > > beneficial for release tests. This is up to the release manager. > > + Henry > > I agree this should go in 4.17, so that gitlab-ci can test non-DEBUG > builds on ARM. Yes sure, I agree once this series is properly reviewed, this can be merged to 4.17. Kind regards, Henry
Hi Stefano, On 22/09/2022 23:40, Stefano Stabellini wrote: > > > On Thu, 22 Sep 2022, Michal Orzel wrote: >> Currently, all the arm64 defconfig build jobs, regardless of the >> container used, end up building Xen with the extra config options >> specified in the main build script (e.g. CONFIG_EXPERT, >> CONFIG_STATIC_MEMORY). Because these options are only needed for >> specific test jobs, the current behavior of the CI is incorrect >> as we add the extra options to all the defconfig builds. This means >> that on arm64 there is not a single job performing proper defconfig build. >> >> To fix this issue, add custom build jobs each time there is a need for >> building Xen with additional config options. Introduce EXTRA_XEN_CONFIG >> variable to be used by these jobs to store the required options. This >> variable will be then read by the main build script to modify the .config >> file. This will also help users to understand what is needed to run specific >> test. >> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> --- >> This patch could actually be consider to be taken for 4.17 release. >> The reason why is because currently our CI for arm64 does not even >> peform clean defconfig build which is quite crucial target to be tested. >> Performing builds always with EXPERT and UNSUPPORTED is not something so >> beneficial for release tests. This is up to the release manager. > > + Henry > > I agree this should go in 4.17, so that gitlab-ci can test non-DEBUG > builds on ARM. > Do you mean the whole series should go in? I'm ok with that, even though I only marked this patch as the one that should go in as it can be seen as a fix. But I can also see the benefits of merging the whole series. > >> --- >> automation/gitlab-ci/build.yaml | 15 +++++++++++++++ >> automation/gitlab-ci/test.yaml | 4 ++-- >> automation/scripts/build | 8 ++------ >> 3 files changed, 19 insertions(+), 8 deletions(-) >> >> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml >> index 720ce6e07ba0..a39ed72aac6d 100644 >> --- a/automation/gitlab-ci/build.yaml >> +++ b/automation/gitlab-ci/build.yaml >> @@ -566,6 +566,21 @@ alpine-3.12-gcc-debug-arm64: >> variables: >> CONTAINER: alpine:3.12-arm64v8 >> >> +alpine-3.12-gcc-arm64-staticmem: >> + extends: .gcc-arm64-build >> + variables: >> + CONTAINER: alpine:3.12-arm64v8 >> + EXTRA_XEN_CONFIG: | > > Why the "|" ? > > I was trying to look for its documentation in the gitlab yaml docs but > couldn't find it. > By default gitlab variables are one liners so that they can store one key:value pair. If you want to define a variable storing multiple values (in this case we want to store multi-line string because of .config format) you need to use |. You can check [1]. > >> + CONFIG_EXPERT=y >> + CONFIG_UNSUPPORTED=y >> + CONFIG_STATIC_MEMORY=y >> + >> +alpine-3.12-gcc-arm64-boot-cpupools: >> + extends: .gcc-arm64-build >> + variables: >> + CONTAINER: alpine:3.12-arm64v8 >> + EXTRA_XEN_CONFIG: | >> + CONFIG_BOOT_TIME_CPUPOOLS=y >> >> ## Test artifacts common >> >> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml >> index d899b3bdbf7a..4f96e6e322de 100644 >> --- a/automation/gitlab-ci/test.yaml >> +++ b/automation/gitlab-ci/test.yaml >> @@ -88,7 +88,7 @@ qemu-smoke-arm64-gcc-staticmem: >> script: >> - ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log >> needs: >> - - alpine-3.12-gcc-arm64 >> + - alpine-3.12-gcc-arm64-staticmem >> - alpine-3.12-arm64-rootfs-export >> - kernel-5.19-arm64-export >> - qemu-system-aarch64-6.0.0-arm64-export >> @@ -107,7 +107,7 @@ qemu-smoke-arm64-gcc-boot-cpupools: >> script: >> - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log >> needs: >> - - alpine-3.12-gcc-arm64 >> + - alpine-3.12-gcc-arm64-boot-cpupools >> - alpine-3.12-arm64-rootfs-export >> - kernel-5.19-arm64-export >> - qemu-system-aarch64-6.0.0-arm64-export >> diff --git a/automation/scripts/build b/automation/scripts/build >> index 2f15ab3198e6..7d441cedb4ae 100755 >> --- a/automation/scripts/build >> +++ b/automation/scripts/build >> @@ -15,12 +15,8 @@ if [[ "${RANDCONFIG}" == "y" ]]; then >> make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig >> hypervisor_only="y" >> else >> - if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then >> - echo " >> -CONFIG_EXPERT=y >> -CONFIG_UNSUPPORTED=y >> -CONFIG_STATIC_MEMORY=y >> -CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config >> + if [ -n "${EXTRA_XEN_CONFIG}" ]; then > > NIT: for uniformity with rest of the file use > > if [[ -n "${EXTRA_XEN_CONFIG}" ]]; then > Ok, will do in v2. > >> + echo "${EXTRA_XEN_CONFIG}" > xen/.config >> make -j$(nproc) -C xen olddefconfig >> else >> make -j$(nproc) -C xen defconfig >> -- >> 2.25.1 >> ~Michal [1] https://docs.gitlab.com/ee/ci/variables/#store-multiple-values-in-one-variable
On Fri, 23 Sep 2022, Michal Orzel wrote: > Hi Stefano, > > On 22/09/2022 23:40, Stefano Stabellini wrote: > > > > > > On Thu, 22 Sep 2022, Michal Orzel wrote: > >> Currently, all the arm64 defconfig build jobs, regardless of the > >> container used, end up building Xen with the extra config options > >> specified in the main build script (e.g. CONFIG_EXPERT, > >> CONFIG_STATIC_MEMORY). Because these options are only needed for > >> specific test jobs, the current behavior of the CI is incorrect > >> as we add the extra options to all the defconfig builds. This means > >> that on arm64 there is not a single job performing proper defconfig build. > >> > >> To fix this issue, add custom build jobs each time there is a need for > >> building Xen with additional config options. Introduce EXTRA_XEN_CONFIG > >> variable to be used by these jobs to store the required options. This > >> variable will be then read by the main build script to modify the .config > >> file. This will also help users to understand what is needed to run specific > >> test. > >> > >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> > >> --- > >> This patch could actually be consider to be taken for 4.17 release. > >> The reason why is because currently our CI for arm64 does not even > >> peform clean defconfig build which is quite crucial target to be tested. > >> Performing builds always with EXPERT and UNSUPPORTED is not something so > >> beneficial for release tests. This is up to the release manager. > > > > + Henry > > > > I agree this should go in 4.17, so that gitlab-ci can test non-DEBUG > > builds on ARM. > > > Do you mean the whole series should go in? > I'm ok with that, even though I only marked this patch as the one that should go in > as it can be seen as a fix. But I can also see the benefits of merging the whole series. I think only this patch should go in. I like this series but it is best to stay on the safe side and push to staging the rest of the series later after the release. > >> --- > >> automation/gitlab-ci/build.yaml | 15 +++++++++++++++ > >> automation/gitlab-ci/test.yaml | 4 ++-- > >> automation/scripts/build | 8 ++------ > >> 3 files changed, 19 insertions(+), 8 deletions(-) > >> > >> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > >> index 720ce6e07ba0..a39ed72aac6d 100644 > >> --- a/automation/gitlab-ci/build.yaml > >> +++ b/automation/gitlab-ci/build.yaml > >> @@ -566,6 +566,21 @@ alpine-3.12-gcc-debug-arm64: > >> variables: > >> CONTAINER: alpine:3.12-arm64v8 > >> > >> +alpine-3.12-gcc-arm64-staticmem: > >> + extends: .gcc-arm64-build > >> + variables: > >> + CONTAINER: alpine:3.12-arm64v8 > >> + EXTRA_XEN_CONFIG: | > > > > Why the "|" ? > > > > I was trying to look for its documentation in the gitlab yaml docs but > > couldn't find it. > > > By default gitlab variables are one liners so that they can store one key:value pair. > If you want to define a variable storing multiple values (in this case we want to > store multi-line string because of .config format) you need to use |. You can check [1]. OK > > > >> + CONFIG_EXPERT=y > >> + CONFIG_UNSUPPORTED=y > >> + CONFIG_STATIC_MEMORY=y > >> + > >> +alpine-3.12-gcc-arm64-boot-cpupools: > >> + extends: .gcc-arm64-build > >> + variables: > >> + CONTAINER: alpine:3.12-arm64v8 > >> + EXTRA_XEN_CONFIG: | > >> + CONFIG_BOOT_TIME_CPUPOOLS=y > >> > >> ## Test artifacts common > >> > >> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > >> index d899b3bdbf7a..4f96e6e322de 100644 > >> --- a/automation/gitlab-ci/test.yaml > >> +++ b/automation/gitlab-ci/test.yaml > >> @@ -88,7 +88,7 @@ qemu-smoke-arm64-gcc-staticmem: > >> script: > >> - ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log > >> needs: > >> - - alpine-3.12-gcc-arm64 > >> + - alpine-3.12-gcc-arm64-staticmem > >> - alpine-3.12-arm64-rootfs-export > >> - kernel-5.19-arm64-export > >> - qemu-system-aarch64-6.0.0-arm64-export > >> @@ -107,7 +107,7 @@ qemu-smoke-arm64-gcc-boot-cpupools: > >> script: > >> - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log > >> needs: > >> - - alpine-3.12-gcc-arm64 > >> + - alpine-3.12-gcc-arm64-boot-cpupools > >> - alpine-3.12-arm64-rootfs-export > >> - kernel-5.19-arm64-export > >> - qemu-system-aarch64-6.0.0-arm64-export > >> diff --git a/automation/scripts/build b/automation/scripts/build > >> index 2f15ab3198e6..7d441cedb4ae 100755 > >> --- a/automation/scripts/build > >> +++ b/automation/scripts/build > >> @@ -15,12 +15,8 @@ if [[ "${RANDCONFIG}" == "y" ]]; then > >> make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig > >> hypervisor_only="y" > >> else > >> - if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then > >> - echo " > >> -CONFIG_EXPERT=y > >> -CONFIG_UNSUPPORTED=y > >> -CONFIG_STATIC_MEMORY=y > >> -CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config > >> + if [ -n "${EXTRA_XEN_CONFIG}" ]; then > > > > NIT: for uniformity with rest of the file use > > > > if [[ -n "${EXTRA_XEN_CONFIG}" ]]; then > > > Ok, will do in v2. > > > > >> + echo "${EXTRA_XEN_CONFIG}" > xen/.config > >> make -j$(nproc) -C xen olddefconfig > >> else > >> make -j$(nproc) -C xen defconfig > >> -- > >> 2.25.1 > >> > > ~Michal > > [1] https://docs.gitlab.com/ee/ci/variables/#store-multiple-values-in-one-variable >
diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index 720ce6e07ba0..a39ed72aac6d 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -566,6 +566,21 @@ alpine-3.12-gcc-debug-arm64: variables: CONTAINER: alpine:3.12-arm64v8 +alpine-3.12-gcc-arm64-staticmem: + extends: .gcc-arm64-build + variables: + CONTAINER: alpine:3.12-arm64v8 + EXTRA_XEN_CONFIG: | + CONFIG_EXPERT=y + CONFIG_UNSUPPORTED=y + CONFIG_STATIC_MEMORY=y + +alpine-3.12-gcc-arm64-boot-cpupools: + extends: .gcc-arm64-build + variables: + CONTAINER: alpine:3.12-arm64v8 + EXTRA_XEN_CONFIG: | + CONFIG_BOOT_TIME_CPUPOOLS=y ## Test artifacts common diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index d899b3bdbf7a..4f96e6e322de 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -88,7 +88,7 @@ qemu-smoke-arm64-gcc-staticmem: script: - ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log needs: - - alpine-3.12-gcc-arm64 + - alpine-3.12-gcc-arm64-staticmem - alpine-3.12-arm64-rootfs-export - kernel-5.19-arm64-export - qemu-system-aarch64-6.0.0-arm64-export @@ -107,7 +107,7 @@ qemu-smoke-arm64-gcc-boot-cpupools: script: - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log needs: - - alpine-3.12-gcc-arm64 + - alpine-3.12-gcc-arm64-boot-cpupools - alpine-3.12-arm64-rootfs-export - kernel-5.19-arm64-export - qemu-system-aarch64-6.0.0-arm64-export diff --git a/automation/scripts/build b/automation/scripts/build index 2f15ab3198e6..7d441cedb4ae 100755 --- a/automation/scripts/build +++ b/automation/scripts/build @@ -15,12 +15,8 @@ if [[ "${RANDCONFIG}" == "y" ]]; then make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig hypervisor_only="y" else - if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then - echo " -CONFIG_EXPERT=y -CONFIG_UNSUPPORTED=y -CONFIG_STATIC_MEMORY=y -CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config + if [ -n "${EXTRA_XEN_CONFIG}" ]; then + echo "${EXTRA_XEN_CONFIG}" > xen/.config make -j$(nproc) -C xen olddefconfig else make -j$(nproc) -C xen defconfig
Currently, all the arm64 defconfig build jobs, regardless of the container used, end up building Xen with the extra config options specified in the main build script (e.g. CONFIG_EXPERT, CONFIG_STATIC_MEMORY). Because these options are only needed for specific test jobs, the current behavior of the CI is incorrect as we add the extra options to all the defconfig builds. This means that on arm64 there is not a single job performing proper defconfig build. To fix this issue, add custom build jobs each time there is a need for building Xen with additional config options. Introduce EXTRA_XEN_CONFIG variable to be used by these jobs to store the required options. This variable will be then read by the main build script to modify the .config file. This will also help users to understand what is needed to run specific test. Signed-off-by: Michal Orzel <michal.orzel@amd.com> --- This patch could actually be consider to be taken for 4.17 release. The reason why is because currently our CI for arm64 does not even peform clean defconfig build which is quite crucial target to be tested. Performing builds always with EXPERT and UNSUPPORTED is not something so beneficial for release tests. This is up to the release manager. --- automation/gitlab-ci/build.yaml | 15 +++++++++++++++ automation/gitlab-ci/test.yaml | 4 ++-- automation/scripts/build | 8 ++------ 3 files changed, 19 insertions(+), 8 deletions(-)