RE: [PATCH v3 05/10] automation: Add Arm containers to containerize script

[Jiamei Xie <Jiamei.Xie@xxxxxxx>]

Hi Michal,

> -----Original Message-----
> From: Michal Orzel <michal.orzel@xxxxxxx>
> Sent: Thursday, October 20, 2022 2:59 PM
> To: Jiamei Xie <Jiamei.Xie@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Doug Goldstein <cardoe@xxxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> >> Michal Orzel
> >> Sent: Tuesday, September 27, 2022 5:47 PM
> >> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> >> Cc: Michal Orzel <michal.orzel@xxxxxxx>; Doug Goldstein
> >> <cardoe@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >> 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@xxxxxxx>
> >> Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >> ---
> 
> >
> > [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