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 <[email protected]>
>> ---
>> 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
