| Message ID | 20220927094727.12762-6-michal.orzel@amd.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | GitLab CI cleanup & improvements for Arm | expand |
Hi Michal, > -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > Michal Orzel > Sent: Tuesday, September 27, 2022 5:47 PM > To: xen-devel@lists.xenproject.org > Cc: Michal Orzel <michal.orzel@amd.com>; Doug Goldstein > <cardoe@cardoe.com>; Stefano Stabellini <sstabellini@kernel.org> > Subject: [PATCH v3 05/10] automation: Add Arm containers to containerize > script > > Script automation/scripts/containerize makes it easy to build Xen within > predefined containers from gitlab container registry. This script is > currently missing the helpers to select Arm containers, so populate the > necessary entries. > > Signed-off-by: Michal Orzel <michal.orzel@amd.com> > Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes in v3: > - none > Changes in v2: > - modify commit msg to reflect that we are missing helpers but in reality > it could be possible to use Arm containers by specifying the full path > to gitlab container registry. However, such usage is annoying. > --- > automation/scripts/containerize | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/automation/scripts/containerize > b/automation/scripts/containerize > index 9d4beca4fa4b..0f4645c4cccb 100755 > --- a/automation/scripts/containerize > +++ b/automation/scripts/containerize > @@ -25,6 +25,7 @@ die() { > BASE="registry.gitlab.com/xen-project/xen" > case "_${CONTAINER}" in > _alpine) CONTAINER="${BASE}/alpine:3.12" ;; > + _alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; > _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;; > _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;; > _centos7) CONTAINER="${BASE}/centos:7" ;; > @@ -35,6 +36,8 @@ case "_${CONTAINER}" in > _stretch|_) CONTAINER="${BASE}/debian:stretch" ;; > _buster-gcc-ibt) CONTAINER="${BASE}/debian:buster-gcc-ibt" ;; > _unstable|_) CONTAINER="${BASE}/debian:unstable" ;; > + _unstable-arm32-gcc) CONTAINER="${BASE}/debian:unstable-arm32- > gcc" ;; > + _unstable-arm64v8) CONTAINER="${BASE}/debian:unstable-arm64v8" ;; > _trusty) CONTAINER="${BASE}/ubuntu:trusty" ;; > _xenial) CONTAINER="${BASE}/ubuntu:xenial" ;; > _opensuse-leap|_leap) CONTAINER="${BASE}/suse:opensuse-leap" ;; > -- > 2.25.1 [Jiamei Xie] I wonder if an default container for arm can be added. For example, if "CONTAINER=arm64 automation/scripts/containerize bash", set the default CONTAINER as "registry.gitlab.com/xen-project/xen/alpine:3.12-arm64v8" Best wishes Jiamei Xie
Hi Jiamei, On 20/10/2022 05:00, Jiamei Xie wrote: > > > Hi Michal, > >> -----Original Message----- >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of >> Michal Orzel >> Sent: Tuesday, September 27, 2022 5:47 PM >> To: xen-devel@lists.xenproject.org >> Cc: Michal Orzel <michal.orzel@amd.com>; Doug Goldstein >> <cardoe@cardoe.com>; Stefano Stabellini <sstabellini@kernel.org> >> Subject: [PATCH v3 05/10] automation: Add Arm containers to containerize >> script >> >> Script automation/scripts/containerize makes it easy to build Xen within >> predefined containers from gitlab container registry. This script is >> currently missing the helpers to select Arm containers, so populate the >> necessary entries. >> >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >> Acked-by: Stefano Stabellini <sstabellini@kernel.org> >> --- > > [Jiamei Xie] > I wonder if an default container for arm can be added. For example, if > "CONTAINER=arm64 automation/scripts/containerize bash", > set the default CONTAINER as "registry.gitlab.com/xen-project/xen/alpine:3.12-arm64v8" > It can be added doing the following: diff --git a/automation/scripts/containerize b/automation/scripts/containerize index 0f4645c4cccb..b395bd359ecf 100755 --- a/automation/scripts/containerize +++ b/automation/scripts/containerize @@ -25,7 +25,7 @@ die() { BASE="registry.gitlab.com/xen-project/xen" case "_${CONTAINER}" in _alpine) CONTAINER="${BASE}/alpine:3.12" ;; - _alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; + _alpine-arm64v8|_arm64) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;; _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;; _centos7) CONTAINER="${BASE}/centos:7" ;; The question is whether it would be beneficial. After all you would still need to type CONTAINER=arm64, whereas at the moment, you need to type CONTAINER=alpine-arm64v8. TBH I'm not sure it is improving anything (?). ~Michal
Hi Michal, > -----Original Message----- > From: Michal Orzel <michal.orzel@amd.com> > Sent: Thursday, October 20, 2022 2:59 PM > To: Jiamei Xie <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org > Cc: Doug Goldstein <cardoe@cardoe.com>; Stefano Stabellini > <sstabellini@kernel.org> > Subject: Re: [PATCH v3 05/10] automation: Add Arm containers to > containerize script > > Hi Jiamei, > > On 20/10/2022 05:00, Jiamei Xie wrote: > > > > > > Hi Michal, > > > >> -----Original Message----- > >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of > >> Michal Orzel > >> Sent: Tuesday, September 27, 2022 5:47 PM > >> To: xen-devel@lists.xenproject.org > >> Cc: Michal Orzel <michal.orzel@amd.com>; Doug Goldstein > >> <cardoe@cardoe.com>; Stefano Stabellini <sstabellini@kernel.org> > >> Subject: [PATCH v3 05/10] automation: Add Arm containers to > containerize > >> script > >> > >> Script automation/scripts/containerize makes it easy to build Xen within > >> predefined containers from gitlab container registry. This script is > >> currently missing the helpers to select Arm containers, so populate the > >> necessary entries. > >> > >> Signed-off-by: Michal Orzel <michal.orzel@amd.com> > >> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > >> --- > > > > > [Jiamei Xie] > > I wonder if an default container for arm can be added. For example, if > > "CONTAINER=arm64 automation/scripts/containerize bash", > > set the default CONTAINER as "registry.gitlab.com/xen- > project/xen/alpine:3.12-arm64v8" > > > > It can be added doing the following: > > diff --git a/automation/scripts/containerize > b/automation/scripts/containerize > index 0f4645c4cccb..b395bd359ecf 100755 > --- a/automation/scripts/containerize > +++ b/automation/scripts/containerize > @@ -25,7 +25,7 @@ die() { > BASE="registry.gitlab.com/xen-project/xen" > case "_${CONTAINER}" in > _alpine) CONTAINER="${BASE}/alpine:3.12" ;; > - _alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; > + _alpine-arm64v8|_arm64) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; > _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;; > _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;; > _centos7) CONTAINER="${BASE}/centos:7" ;; > > The question is whether it would be beneficial. After all you would still need > to > type CONTAINER=arm64, whereas at the moment, you need to type > CONTAINER=alpine-arm64v8. > TBH I'm not sure it is improving anything (?). > > ~Michal [Jiamei Xie] I am not sure about this either. I added something like below f to run it on arm64 machine. But it didn't take "running container for a different architecture" into consideration. --- a/automation/scripts/containerize +++ b/automation/scripts/containerize @@ -18,6 +18,12 @@ die() { exit 1 } +# There are two containers that can run on aarch64, unstable and alpine. +# Set the default container to alpine for aarch64 +if [[ $(uname -m) = "aarch64" && -z ${CONTAINER} ]]; then + CONTAINER="alpine" +fi + # # The caller is expected to override the CONTAINER environment # variable with the container they wish to launch. @@ -41,6 +47,11 @@ case "_${CONTAINER}" in _opensuse-tumbleweed|_tumbleweed) CONTAINER="${BASE}/suse:opensuse-tumbleweed" ;; esac +# Containers for aarch64 have a sufix "-arm64v8" +if [[ $(uname -m) = "aarch64" ]]; then + CONTAINER="${CONTAINER}-arm64v8" +fi + # Use this variable to control whether root should be used case "_${CONTAINER_UID0}" in _1) userarg= ;; Best wishes Jiamei Xie
Hi Jiamei, On 20/10/2022 09:13, Jiamei Xie wrote: > > > Hi Michal, > >> -----Original Message----- >> From: Michal Orzel <michal.orzel@amd.com> >> Sent: Thursday, October 20, 2022 2:59 PM >> To: Jiamei Xie <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org >> Cc: Doug Goldstein <cardoe@cardoe.com>; Stefano Stabellini >> <sstabellini@kernel.org> >> Subject: Re: [PATCH v3 05/10] automation: Add Arm containers to >> containerize script >> >> Hi Jiamei, >> >> On 20/10/2022 05:00, Jiamei Xie wrote: >>> >>> >>> Hi Michal, >>> >>>> -----Original Message----- >>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of >>>> Michal Orzel >>>> Sent: Tuesday, September 27, 2022 5:47 PM >>>> To: xen-devel@lists.xenproject.org >>>> Cc: Michal Orzel <michal.orzel@amd.com>; Doug Goldstein >>>> <cardoe@cardoe.com>; Stefano Stabellini <sstabellini@kernel.org> >>>> Subject: [PATCH v3 05/10] automation: Add Arm containers to >> containerize >>>> script >>>> >>>> Script automation/scripts/containerize makes it easy to build Xen within >>>> predefined containers from gitlab container registry. This script is >>>> currently missing the helpers to select Arm containers, so populate the >>>> necessary entries. >>>> >>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com> >>>> Acked-by: Stefano Stabellini <sstabellini@kernel.org> >>>> --- >> >>> >>> [Jiamei Xie] >>> I wonder if an default container for arm can be added. For example, if >>> "CONTAINER=arm64 automation/scripts/containerize bash", >>> set the default CONTAINER as "registry.gitlab.com/xen- >> project/xen/alpine:3.12-arm64v8" >>> >> >> It can be added doing the following: >> >> diff --git a/automation/scripts/containerize >> b/automation/scripts/containerize >> index 0f4645c4cccb..b395bd359ecf 100755 >> --- a/automation/scripts/containerize >> +++ b/automation/scripts/containerize >> @@ -25,7 +25,7 @@ die() { >> BASE="registry.gitlab.com/xen-project/xen" >> case "_${CONTAINER}" in >> _alpine) CONTAINER="${BASE}/alpine:3.12" ;; >> - _alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; >> + _alpine-arm64v8|_arm64) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; >> _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;; >> _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;; >> _centos7) CONTAINER="${BASE}/centos:7" ;; >> >> The question is whether it would be beneficial. After all you would still need >> to >> type CONTAINER=arm64, whereas at the moment, you need to type >> CONTAINER=alpine-arm64v8. >> TBH I'm not sure it is improving anything (?). >> >> ~Michal > [Jiamei Xie] > I am not sure about this either. I added something like below f to run it on arm64 machine. But it didn't take "running container for a different architecture" into consideration. > So your question is not about adding default container when selecting CONTAINER=arm64, but adding a default one when running on arm64 platform. Right now, the default one is debian:stretch (if you don't type CONTAINER= at all). Do I understand it right that you would like the same behavior when running on arm64 platform (currently, it would also select debian:stretch)? So that when executing: ./automation/scripts/containerize ... it would automatically select alpine-arm64v8? > --- a/automation/scripts/containerize > +++ b/automation/scripts/containerize > @@ -18,6 +18,12 @@ die() { > exit 1 > } > > +# There are two containers that can run on aarch64, unstable and alpine. > +# Set the default container to alpine for aarch64 > +if [[ $(uname -m) = "aarch64" && -z ${CONTAINER} ]]; then The output from `uname -m` for arm64 can be aarch64 and arm64. > + CONTAINER="alpine" > +fi > + > # > # The caller is expected to override the CONTAINER environment > # variable with the container they wish to launch. > @@ -41,6 +47,11 @@ case "_${CONTAINER}" in > _opensuse-tumbleweed|_tumbleweed) CONTAINER="${BASE}/suse:opensuse-tumbleweed" ;; > esac > > +# Containers for aarch64 have a sufix "-arm64v8" > +if [[ $(uname -m) = "aarch64" ]]; then > + CONTAINER="${CONTAINER}-arm64v8" > +fi This is not needed. CONTAINER can be selected on the first check and let case/esac block to determine the full path to container. > + > # Use this variable to control whether root should be used > case "_${CONTAINER_UID0}" in > _1) userarg= ;; > > > Best wishes > Jiamei Xie > > What you are asking for can be done in a simpler way. The following is enough: diff --git a/automation/scripts/containerize b/automation/scripts/containerize index 0f4645c4cccb..4e7e8bb48e3a 100755 --- a/automation/scripts/containerize +++ b/automation/scripts/containerize @@ -18,6 +18,11 @@ die() { exit 1 } +# Select default container when running on arm64 machine. +if [ -z "${CONTAINER}" ] && uname -m | grep -qE 'aarch64|arm64'; then + CONTAINER="alpine-arm64v8" +fi + # # The caller is expected to override the CONTAINER environment # variable with the container they wish to launch. ~Michal
Hi Michal, > -----Original Message----- > From: Michal Orzel <michal.orzel@amd.com> > Sent: Thursday, October 20, 2022 3:52 PM > To: Jiamei Xie <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org > Cc: Doug Goldstein <cardoe@cardoe.com>; Stefano Stabellini > <sstabellini@kernel.org> > Subject: Re: [PATCH v3 05/10] automation: Add Arm containers to > containerize script > > Hi Jiamei, > > On 20/10/2022 09:13, Jiamei Xie wrote: > > > > > > Hi Michal, > > > >> -----Original Message----- > >> From: Michal Orzel <michal.orzel@amd.com> > >> Sent: Thursday, October 20, 2022 2:59 PM > >> To: Jiamei Xie <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org > >> Cc: Doug Goldstein <cardoe@cardoe.com>; Stefano Stabellini > >> <sstabellini@kernel.org> > >> Subject: Re: [PATCH v3 05/10] automation: Add Arm containers to > >> containerize script > >> > >> Hi Jiamei, > >> > >> On 20/10/2022 05:00, Jiamei Xie wrote: > >>> > >>> > >>> Hi Michal, > >>> > >>>> -----Original Message----- > >>>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf > Of > >>>> Michal Orzel > >>>> Sent: Tuesday, September 27, 2022 5:47 PM > >>>> To: xen-devel@lists.xenproject.org > >>>> Cc: Michal Orzel <michal.orzel@amd.com>; Doug Goldstein > >>>> <cardoe@cardoe.com>; Stefano Stabellini <sstabellini@kernel.org> > >>>> Subject: [PATCH v3 05/10] automation: Add Arm containers to > >> containerize > >>>> script > >>>> > >>>> Script automation/scripts/containerize makes it easy to build Xen > within > >>>> predefined containers from gitlab container registry. This script is > >>>> currently missing the helpers to select Arm containers, so populate the > >>>> necessary entries. > >>>> > >>>> Signed-off-by: Michal Orzel <michal.orzel@amd.com> > >>>> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > >>>> --- > >> > >>> > >>> [Jiamei Xie] > >>> I wonder if an default container for arm can be added. For example, if > >>> "CONTAINER=arm64 automation/scripts/containerize bash", > >>> set the default CONTAINER as "registry.gitlab.com/xen- > >> project/xen/alpine:3.12-arm64v8" > >>> > >> > >> It can be added doing the following: > >> > >> diff --git a/automation/scripts/containerize > >> b/automation/scripts/containerize > >> index 0f4645c4cccb..b395bd359ecf 100755 > >> --- a/automation/scripts/containerize > >> +++ b/automation/scripts/containerize > >> @@ -25,7 +25,7 @@ die() { > >> BASE="registry.gitlab.com/xen-project/xen" > >> case "_${CONTAINER}" in > >> _alpine) CONTAINER="${BASE}/alpine:3.12" ;; > >> - _alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; > >> + _alpine-arm64v8|_arm64) CONTAINER="${BASE}/alpine:3.12- > arm64v8" ;; > >> _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;; > >> _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;; > >> _centos7) CONTAINER="${BASE}/centos:7" ;; > >> > >> The question is whether it would be beneficial. After all you would still > need > >> to > >> type CONTAINER=arm64, whereas at the moment, you need to type > >> CONTAINER=alpine-arm64v8. > >> TBH I'm not sure it is improving anything (?). > >> > >> ~Michal > > [Jiamei Xie] > > I am not sure about this either. I added something like below f to run it on > arm64 machine. But it didn't take "running container for a different > architecture" into consideration. > > > So your question is not about adding default container when selecting > CONTAINER=arm64, but adding > a default one when running on arm64 platform. Right now, the default one > is debian:stretch > (if you don't type CONTAINER= at all). Do I understand it right that you > would like the same > behavior when running on arm64 platform (currently, it would also select > debian:stretch)? > So that when executing: > ./automation/scripts/containerize ... > it would automatically select alpine-arm64v8? > Yes, this is what I mean. > > > --- a/automation/scripts/containerize > > +++ b/automation/scripts/containerize > > @@ -18,6 +18,12 @@ die() { > > exit 1 > > } > > > > +# There are two containers that can run on aarch64, unstable and alpine. > > +# Set the default container to alpine for aarch64 > > +if [[ $(uname -m) = "aarch64" && -z ${CONTAINER} ]]; then > The output from `uname -m` for arm64 can be aarch64 and arm64. > > > + CONTAINER="alpine" > > +fi > > + > > # > > # The caller is expected to override the CONTAINER environment > > # variable with the container they wish to launch. > > @@ -41,6 +47,11 @@ case "_${CONTAINER}" in > > _opensuse-tumbleweed|_tumbleweed) > CONTAINER="${BASE}/suse:opensuse-tumbleweed" ;; > > esac > > > > +# Containers for aarch64 have a sufix "-arm64v8" > > +if [[ $(uname -m) = "aarch64" ]]; then > > + CONTAINER="${CONTAINER}-arm64v8" > > +fi > This is not needed. CONTAINER can be selected on the first check and let > case/esac block > to determine the full path to container. > > > + > > # Use this variable to control whether root should be used > > case "_${CONTAINER_UID0}" in > > _1) userarg= ;; > > > > > > Best wishes > > Jiamei Xie > > > > > > What you are asking for can be done in a simpler way. The following is > enough: > > diff --git a/automation/scripts/containerize > b/automation/scripts/containerize > index 0f4645c4cccb..4e7e8bb48e3a 100755 > --- a/automation/scripts/containerize > +++ b/automation/scripts/containerize > @@ -18,6 +18,11 @@ die() { > exit 1 > } > > +# Select default container when running on arm64 machine. > +if [ -z "${CONTAINER}" ] && uname -m | grep -qE 'aarch64|arm64'; then > + CONTAINER="alpine-arm64v8" > +fi > + > # > # The caller is expected to override the CONTAINER environment > # variable with the container they wish to launch. > > ~Michal Yeah, I agree with this implementation. Best wishes Jiamei Xie
diff --git a/automation/scripts/containerize b/automation/scripts/containerize index 9d4beca4fa4b..0f4645c4cccb 100755 --- a/automation/scripts/containerize +++ b/automation/scripts/containerize @@ -25,6 +25,7 @@ die() { BASE="registry.gitlab.com/xen-project/xen" case "_${CONTAINER}" in _alpine) CONTAINER="${BASE}/alpine:3.12" ;; + _alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;; _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;; _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;; _centos7) CONTAINER="${BASE}/centos:7" ;; @@ -35,6 +36,8 @@ case "_${CONTAINER}" in _stretch|_) CONTAINER="${BASE}/debian:stretch" ;; _buster-gcc-ibt) CONTAINER="${BASE}/debian:buster-gcc-ibt" ;; _unstable|_) CONTAINER="${BASE}/debian:unstable" ;; + _unstable-arm32-gcc) CONTAINER="${BASE}/debian:unstable-arm32-gcc" ;; + _unstable-arm64v8) CONTAINER="${BASE}/debian:unstable-arm64v8" ;; _trusty) CONTAINER="${BASE}/ubuntu:trusty" ;; _xenial) CONTAINER="${BASE}/ubuntu:xenial" ;; _opensuse-leap|_leap) CONTAINER="${BASE}/suse:opensuse-leap" ;;