[PATCH 0/3] CI: Improve domU handling

[PATCH 0/3] CI: Improve domU handling

[Andrew Cooper]

A follow-up to an RFC patch on the initrd handling improvements, this time
non-RFC.

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/1831808006

Andrew Cooper (3):
  CI/qubes: Deduplicate the handling of ${dom0_check}
  CI: Use bash arrays to simplfy dom0 rootfs construction
  CI: Adjust how domU is packaged in dom0

 automation/scripts/qubes-x86-64.sh            | 39 ++++++++++++-------
 .../scripts/xilinx-smoke-dom0-x86_64.sh       | 32 +++++++++------
 2 files changed, 46 insertions(+), 25 deletions(-)


base-commit: 7ab4b392b78b5ac1c7a1fb1d085637526e67521a
-- 
2.39.5

[PATCH 1/3] CI/qubes: Deduplicate the handling of ${dom0_check}

[Andrew Cooper]

Make it clear that ${dom0_check} is unconditional.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/scripts/qubes-x86-64.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index bfdd2ceb99ba..10af274a0ba7 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -221,12 +221,11 @@ if [ -n "$domU_check" ]; then
 tail -F /var/log/xen/console/guest-domU.log 2>/dev/null | sed -e \"s/^/(domU) /\" &
 tail -F /var/log/xen/qemu-dm-domU.log 2>/dev/null | sed -e \"s/^/(qemu-dm) /\" &
 xl -vvv create /etc/xen/domU.cfg
-${dom0_check}
 " >> etc/local.d/xen.start
-else
-    echo "${dom0_check}" >> etc/local.d/xen.start
 fi
 
+echo "${dom0_check}" >> etc/local.d/xen.start
+
 chmod +x etc/local.d/xen.start
 mkdir -p etc/xen
 echo "$domU_config" > etc/xen/domU.cfg
-- 
2.39.5

[PATCH 2/3] CI: Use bash arrays to simplfy dom0 rootfs construction

[Andrew Cooper]

For Qubes, this requires switching from sh to bash.

This reduces the number of times the target filename needs to be written to 1.

Expand the comment to explain the concatination constraints.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

I would like to find a slightly nicer way of conditional parts, but nothing
comes to mind.
---
 automation/scripts/qubes-x86-64.sh             | 14 +++++++++-----
 automation/scripts/xilinx-smoke-dom0-x86_64.sh | 16 +++++++++-------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 10af274a0ba7..1dd3f48b3d29 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 set -ex -o pipefail
 
@@ -187,10 +187,14 @@ Kernel \r on an \m (\l)
     rm -rf rootfs
 fi
 
-# Dom0 rootfs
-cp binaries/ucode.cpio binaries/dom0-rootfs.cpio.gz
-cat binaries/rootfs.cpio.gz >> binaries/dom0-rootfs.cpio.gz
-cat binaries/xen-tools.cpio.gz >> binaries/dom0-rootfs.cpio.gz
+# Dom0 rootfs.  The order or concatination is important; ucode wants to come
+# first, and all uncompressed must be ahead of compressed.
+parts=(
+    binaries/ucode.cpio
+    binaries/rootfs.cpio.gz
+    binaries/xen-tools.cpio.gz
+)
+cat "${parts[@]}" > binaries/dom0-rootfs.cpio.gz
 
 # test-local configuration
 mkdir -p rootfs
diff --git a/automation/scripts/xilinx-smoke-dom0-x86_64.sh b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
index 8f02fa73bd06..0fbabb41054a 100755
--- a/automation/scripts/xilinx-smoke-dom0-x86_64.sh
+++ b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
@@ -103,13 +103,15 @@ find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
 cd ..
 rm -rf rootfs
 
-# Dom0 rootfs
-cp binaries/ucode.cpio binaries/dom0-rootfs.cpio.gz
-cat binaries/rootfs.cpio.gz >> binaries/dom0-rootfs.cpio.gz
-cat binaries/xen-tools.cpio.gz >> binaries/dom0-rootfs.cpio.gz
-if [[ "${TEST}" == argo ]]; then
-    cat binaries/argo.cpio.gz >> binaries/dom0-rootfs.cpio.gz
-fi
+# Dom0 rootfs.  The order or concatination is important; ucode wants to come
+# first, and all uncompressed must be ahead of compressed.
+parts=(
+    binaries/ucode.cpio
+    binaries/rootfs.cpio.gz
+    binaries/xen-tools.cpio.gz
+)
+[[ "${TEST}" == argo ]] && parts+=(binaries/argo.cpio.gz)
+cat "${parts[@]}" > binaries/dom0-rootfs.cpio.gz
 
 # test-local configuration
 mkdir -p rootfs
-- 
2.39.5

[PATCH 3/3] CI: Adjust how domU is packaged in dom0

[Andrew Cooper]

Package domU in /root for dom0 and insert into the uncompressed part of dom0's
rootfs, rather than recompressing it as part of the overlay.

For Qubes, this avoids putting the domU kernel in dom0's rootfs for tests
which aren't going to boot a guest.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/scripts/qubes-x86-64.sh            | 20 +++++++++++++------
 .../scripts/xilinx-smoke-dom0-x86_64.sh       | 16 +++++++++++----
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 1dd3f48b3d29..17a37134f46a 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -154,8 +154,8 @@ esac
 domU_config="
 type = '${domU_type}'
 name = 'domU'
-kernel = '/boot/vmlinuz'
-ramdisk = '/boot/initrd-domU'
+kernel = '/root/vmlinuz-domU'
+ramdisk = '/root/initrd-domU'
 cmdline = 'root=/dev/ram0 console=hvc0'
 memory = 512
 vif = [ ${domU_vif} ]
@@ -185,12 +185,24 @@ Kernel \r on an \m (\l)
     find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
     cd ..
     rm -rf rootfs
+
+    # Package domU kernel+rootfs in /root for dom0 (uncompressed)
+    mkdir -p rootfs/root
+    cd rootfs
+    cp ../binaries/bzImage root/vmlinuz-domU
+    cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
+    find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
+    cd ..
+    rm -rf rootfs
 fi
 
 # Dom0 rootfs.  The order or concatination is important; ucode wants to come
 # first, and all uncompressed must be ahead of compressed.
 parts=(
     binaries/ucode.cpio
+)
+[ -n "$domU_check" ] && parts+=(binaries/domU-in-dom0.cpio)
+parts+=(
     binaries/rootfs.cpio.gz
     binaries/xen-tools.cpio.gz
 )
@@ -238,10 +250,6 @@ mkdir -p etc/default
 echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
 echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
 mkdir -p var/log/xen/console
-cp ../binaries/bzImage boot/vmlinuz
-if [ -n "$domU_check" ]; then
-    cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
-fi
 find . | cpio -H newc -o | gzip >> ../binaries/dom0-rootfs.cpio.gz
 cd ..
 
diff --git a/automation/scripts/xilinx-smoke-dom0-x86_64.sh b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
index 0fbabb41054a..29817ff81d0a 100755
--- a/automation/scripts/xilinx-smoke-dom0-x86_64.sh
+++ b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
@@ -22,8 +22,8 @@ DOMU_CMD=""
 DOMU_CFG='
 type = "pvh"
 name = "domU"
-kernel = "/boot/vmlinuz"
-ramdisk = "/boot/initrd-domU"
+kernel = "/root/vmlinuz-domU"
+ramdisk = "/root/initrd-domU"
 extra = "root=/dev/ram0 console=hvc0"
 memory = 512
 '
@@ -103,10 +103,20 @@ find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
 cd ..
 rm -rf rootfs
 
+# Package domU kernel+rootfs in /root for dom0 (uncompressed)
+mkdir -p rootfs/root
+cd rootfs
+cp ../binaries/bzImage root/vmlinuz-domU
+cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
+find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
+cd ..
+rm -rf rootfs
+
 # Dom0 rootfs.  The order or concatination is important; ucode wants to come
 # first, and all uncompressed must be ahead of compressed.
 parts=(
     binaries/ucode.cpio
+    binaries/domU-in-dom0.cpio
     binaries/rootfs.cpio.gz
     binaries/xen-tools.cpio.gz
 )
@@ -127,8 +137,6 @@ echo "${DOMU_CFG}${DOMU_CFG_EXTRA}" > etc/xen/domU.cfg
 echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
 echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
 mkdir -p var/log/xen/console
-cp ../binaries/bzImage boot/vmlinuz
-cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
 find . | cpio -H newc -o | gzip >> ../binaries/dom0-rootfs.cpio.gz
 cd ..
 
-- 
2.39.5

Re: [PATCH 1/3] CI/qubes: Deduplicate the handling of ${dom0_check}

[Anthony PERARD]

On Thu, May 22, 2025 at 06:36:38PM +0100, Andrew Cooper wrote:
> Make it clear that ${dom0_check} is unconditional.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD

Re: [PATCH 3/3] CI: Adjust how domU is packaged in dom0

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 4504 bytes --]

On Thu, May 22, 2025 at 06:36:40PM +0100, Andrew Cooper wrote:
> Package domU in /root for dom0 and insert into the uncompressed part of dom0's
> rootfs, rather than recompressing it as part of the overlay.

It doesn't really need moving to /root to achieve this, no? The
domU-in-dom0.cpio can very well contain boot/* files.

> For Qubes, this avoids putting the domU kernel in dom0's rootfs for tests
> which aren't going to boot a guest.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Anthony PERARD <anthony.perard@vates.tech>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Michal Orzel <michal.orzel@amd.com>
> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  automation/scripts/qubes-x86-64.sh            | 20 +++++++++++++------
>  .../scripts/xilinx-smoke-dom0-x86_64.sh       | 16 +++++++++++----
>  2 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 1dd3f48b3d29..17a37134f46a 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -154,8 +154,8 @@ esac
>  domU_config="
>  type = '${domU_type}'
>  name = 'domU'
> -kernel = '/boot/vmlinuz'
> -ramdisk = '/boot/initrd-domU'
> +kernel = '/root/vmlinuz-domU'
> +ramdisk = '/root/initrd-domU'
>  cmdline = 'root=/dev/ram0 console=hvc0'
>  memory = 512
>  vif = [ ${domU_vif} ]
> @@ -185,12 +185,24 @@ Kernel \r on an \m (\l)
>      find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
>      cd ..
>      rm -rf rootfs
> +
> +    # Package domU kernel+rootfs in /root for dom0 (uncompressed)
> +    mkdir -p rootfs/root
> +    cd rootfs
> +    cp ../binaries/bzImage root/vmlinuz-domU
> +    cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
> +    find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
> +    cd ..
> +    rm -rf rootfs
>  fi
>  
>  # Dom0 rootfs.  The order or concatination is important; ucode wants to come
>  # first, and all uncompressed must be ahead of compressed.
>  parts=(
>      binaries/ucode.cpio
> +)
> +[ -n "$domU_check" ] && parts+=(binaries/domU-in-dom0.cpio)
> +parts+=(
>      binaries/rootfs.cpio.gz
>      binaries/xen-tools.cpio.gz
>  )
> @@ -238,10 +250,6 @@ mkdir -p etc/default
>  echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
>  echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
>  mkdir -p var/log/xen/console
> -cp ../binaries/bzImage boot/vmlinuz
> -if [ -n "$domU_check" ]; then
> -    cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
> -fi
>  find . | cpio -H newc -o | gzip >> ../binaries/dom0-rootfs.cpio.gz
>  cd ..
>  
> diff --git a/automation/scripts/xilinx-smoke-dom0-x86_64.sh b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
> index 0fbabb41054a..29817ff81d0a 100755
> --- a/automation/scripts/xilinx-smoke-dom0-x86_64.sh
> +++ b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
> @@ -22,8 +22,8 @@ DOMU_CMD=""
>  DOMU_CFG='
>  type = "pvh"
>  name = "domU"
> -kernel = "/boot/vmlinuz"
> -ramdisk = "/boot/initrd-domU"
> +kernel = "/root/vmlinuz-domU"
> +ramdisk = "/root/initrd-domU"
>  extra = "root=/dev/ram0 console=hvc0"
>  memory = 512
>  '
> @@ -103,10 +103,20 @@ find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
>  cd ..
>  rm -rf rootfs
>  
> +# Package domU kernel+rootfs in /root for dom0 (uncompressed)
> +mkdir -p rootfs/root
> +cd rootfs
> +cp ../binaries/bzImage root/vmlinuz-domU
> +cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
> +find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
> +cd ..
> +rm -rf rootfs
> +
>  # Dom0 rootfs.  The order or concatination is important; ucode wants to come
>  # first, and all uncompressed must be ahead of compressed.
>  parts=(
>      binaries/ucode.cpio
> +    binaries/domU-in-dom0.cpio
>      binaries/rootfs.cpio.gz
>      binaries/xen-tools.cpio.gz
>  )
> @@ -127,8 +137,6 @@ echo "${DOMU_CFG}${DOMU_CFG_EXTRA}" > etc/xen/domU.cfg
>  echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
>  echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
>  mkdir -p var/log/xen/console
> -cp ../binaries/bzImage boot/vmlinuz
> -cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
>  find . | cpio -H newc -o | gzip >> ../binaries/dom0-rootfs.cpio.gz
>  cd ..
>  
> -- 
> 2.39.5
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] CI: Use bash arrays to simplfy dom0 rootfs construction

[Anthony PERARD]

On Thu, May 22, 2025 at 06:36:39PM +0100, Andrew Cooper wrote:
> For Qubes, this requires switching from sh to bash.
> 
> This reduces the number of times the target filename needs to be written to 1.
> 
> Expand the comment to explain the concatination constraints.

Isn't the correct spelling "concatenation"? Same for the two comments.

> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> I would like to find a slightly nicer way of conditional parts, but nothing
> comes to mind.

Well, one way I can think of is having a new variable which can carry
the rootfs part associated with a particular test, then that variable
can be updated at the time we configure for that test. Something like:

# init
declare -a append_rootfs_part
# or append_rootfs_part=() is probably fine too.

case $test in
  argo)
    append_rootfs_part+=(argo.cpio.gz)
    # ... other test configuration
    ;;
esac

# Dom0 rootfs
parts=(
    rootfs.cpio.gz
    xen-tools.cpio.gz
    "${append_rootfs_part[@]}"
)

And that should works fine, even if there isn't any extra rootfs part.

> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 10af274a0ba7..1dd3f48b3d29 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -187,10 +187,14 @@ Kernel \r on an \m (\l)
>      rm -rf rootfs
>  fi
>  
> -# Dom0 rootfs
> -cp binaries/ucode.cpio binaries/dom0-rootfs.cpio.gz
> -cat binaries/rootfs.cpio.gz >> binaries/dom0-rootfs.cpio.gz
> -cat binaries/xen-tools.cpio.gz >> binaries/dom0-rootfs.cpio.gz
> +# Dom0 rootfs.  The order or concatination is important; ucode wants to come

                             ^ of concatenation

Same typo in the other comment.

Beside the typo, patch looks fine:
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD

Re: [PATCH 2/3] CI: Use bash arrays to simplfy dom0 rootfs construction

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2490 bytes --]

On Tue, May 27, 2025 at 04:01:34PM +0200, Anthony PERARD wrote:
> On Thu, May 22, 2025 at 06:36:39PM +0100, Andrew Cooper wrote:
> > For Qubes, this requires switching from sh to bash.
> > 
> > This reduces the number of times the target filename needs to be written to 1.
> > 
> > Expand the comment to explain the concatination constraints.
> 
> Isn't the correct spelling "concatenation"? Same for the two comments.
> 
> > 
> > No functional change.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > I would like to find a slightly nicer way of conditional parts, but nothing
> > comes to mind.
> 
> Well, one way I can think of is having a new variable which can carry
> the rootfs part associated with a particular test, then that variable
> can be updated at the time we configure for that test. Something like:
> 
> # init
> declare -a append_rootfs_part
> # or append_rootfs_part=() is probably fine too.
> 
> case $test in
>   argo)
>     append_rootfs_part+=(argo.cpio.gz)
>     # ... other test configuration
>     ;;
> esac
> 
> # Dom0 rootfs
> parts=(
>     rootfs.cpio.gz
>     xen-tools.cpio.gz
>     "${append_rootfs_part[@]}"
> )
> 
> And that should works fine, even if there isn't any extra rootfs part.

That would work for compressed parts, but not for uncompressed - which
need to come before all compressed. But maybe there could be two arrays
- one for uncompressed and another for compressed? Then, each could be
extended anywhere, without messing the order.

> 
> > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> > index 10af274a0ba7..1dd3f48b3d29 100755
> > --- a/automation/scripts/qubes-x86-64.sh
> > +++ b/automation/scripts/qubes-x86-64.sh
> > @@ -187,10 +187,14 @@ Kernel \r on an \m (\l)
> >      rm -rf rootfs
> >  fi
> >  
> > -# Dom0 rootfs
> > -cp binaries/ucode.cpio binaries/dom0-rootfs.cpio.gz
> > -cat binaries/rootfs.cpio.gz >> binaries/dom0-rootfs.cpio.gz
> > -cat binaries/xen-tools.cpio.gz >> binaries/dom0-rootfs.cpio.gz
> > +# Dom0 rootfs.  The order or concatination is important; ucode wants to come
> 
>                              ^ of concatenation
> 
> Same typo in the other comment.
> 
> Beside the typo, patch looks fine:
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
> 
> Thanks,
> 
> -- 
> Anthony PERARD

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/3] CI: Use bash arrays to simplfy dom0 rootfs construction

[Andrew Cooper]

On 27/05/2025 4:19 pm, Marek Marczykowski-Górecki wrote:
> On Tue, May 27, 2025 at 04:01:34PM +0200, Anthony PERARD wrote:
>> On Thu, May 22, 2025 at 06:36:39PM +0100, Andrew Cooper wrote:
>>> For Qubes, this requires switching from sh to bash.
>>>
>>> This reduces the number of times the target filename needs to be written to 1.
>>>
>>> Expand the comment to explain the concatination constraints.
>> Isn't the correct spelling "concatenation"? Same for the two comments.
>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> I would like to find a slightly nicer way of conditional parts, but nothing
>>> comes to mind.
>> Well, one way I can think of is having a new variable which can carry
>> the rootfs part associated with a particular test, then that variable
>> can be updated at the time we configure for that test. Something like:
>>
>> # init
>> declare -a append_rootfs_part
>> # or append_rootfs_part=() is probably fine too.
>>
>> case $test in
>>   argo)
>>     append_rootfs_part+=(argo.cpio.gz)
>>     # ... other test configuration
>>     ;;
>> esac
>>
>> # Dom0 rootfs
>> parts=(
>>     rootfs.cpio.gz
>>     xen-tools.cpio.gz
>>     "${append_rootfs_part[@]}"
>> )
>>
>> And that should works fine, even if there isn't any extra rootfs part.
> That would work for compressed parts, but not for uncompressed - which
> need to come before all compressed. But maybe there could be two arrays
> - one for uncompressed and another for compressed? Then, each could be
> extended anywhere, without messing the order.

Hmm, two might work, but they surely need to not be quoted when forming
parts=(), or having multiple entries will go wrong on the eventual cat
command line.

~Andrew

Re: [PATCH 3/3] CI: Adjust how domU is packaged in dom0

[Andrew Cooper]

On 27/05/2025 2:55 pm, Marek Marczykowski-Górecki wrote:
> On Thu, May 22, 2025 at 06:36:40PM +0100, Andrew Cooper wrote:
>> Package domU in /root for dom0 and insert into the uncompressed part of dom0's
>> rootfs, rather than recompressing it as part of the overlay.
> It doesn't really need moving to /root to achieve this, no? The
> domU-in-dom0.cpio can very well contain boot/* files.

Yes, but is /boot really an appropriate place to be putting test artefacts?

~Andrew

Re: [PATCH 1/3] CI/qubes: Deduplicate the handling of ${dom0_check}

[Stefano Stabellini]

On Tue, 27 May 2025, Anthony PERARD wrote:
> On Thu, May 22, 2025 at 06:36:38PM +0100, Andrew Cooper wrote:
> > Make it clear that ${dom0_check} is unconditional.
> > 
> > No functional change.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [PATCH 2/3] CI: Use bash arrays to simplfy dom0 rootfs construction

[Stefano Stabellini]

On Tue, 27 May 2025, Anthony PERARD wrote:
> On Thu, May 22, 2025 at 06:36:39PM +0100, Andrew Cooper wrote:
> > For Qubes, this requires switching from sh to bash.
> > 
> > This reduces the number of times the target filename needs to be written to 1.
> > 
> > Expand the comment to explain the concatination constraints.
> 
> Isn't the correct spelling "concatenation"? Same for the two comments.
> 
> > 
> > No functional change.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> > I would like to find a slightly nicer way of conditional parts, but nothing
> > comes to mind.
> 
> Well, one way I can think of is having a new variable which can carry
> the rootfs part associated with a particular test, then that variable
> can be updated at the time we configure for that test. Something like:
> 
> # init
> declare -a append_rootfs_part
> # or append_rootfs_part=() is probably fine too.
> 
> case $test in
>   argo)
>     append_rootfs_part+=(argo.cpio.gz)
>     # ... other test configuration
>     ;;
> esac
> 
> # Dom0 rootfs
> parts=(
>     rootfs.cpio.gz
>     xen-tools.cpio.gz
>     "${append_rootfs_part[@]}"
> )
> 
> And that should works fine, even if there isn't any extra rootfs part.
> 
> > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> > index 10af274a0ba7..1dd3f48b3d29 100755
> > --- a/automation/scripts/qubes-x86-64.sh
> > +++ b/automation/scripts/qubes-x86-64.sh
> > @@ -187,10 +187,14 @@ Kernel \r on an \m (\l)
> >      rm -rf rootfs
> >  fi
> >  
> > -# Dom0 rootfs
> > -cp binaries/ucode.cpio binaries/dom0-rootfs.cpio.gz
> > -cat binaries/rootfs.cpio.gz >> binaries/dom0-rootfs.cpio.gz
> > -cat binaries/xen-tools.cpio.gz >> binaries/dom0-rootfs.cpio.gz
> > +# Dom0 rootfs.  The order or concatination is important; ucode wants to come
> 
>                              ^ of concatenation
> 
> Same typo in the other comment.
> 
> Beside the typo, patch looks fine:
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [PATCH 3/3] CI: Adjust how domU is packaged in dom0

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 4608 bytes --]

On Thu, 22 May 2025, Andrew Cooper wrote:
> Package domU in /root for dom0 and insert into the uncompressed part of dom0's
> rootfs, rather than recompressing it as part of the overlay.
> 
> For Qubes, this avoids putting the domU kernel in dom0's rootfs for tests
> which aren't going to boot a guest.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Anthony PERARD <anthony.perard@vates.tech>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Michal Orzel <michal.orzel@amd.com>
> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  automation/scripts/qubes-x86-64.sh            | 20 +++++++++++++------
>  .../scripts/xilinx-smoke-dom0-x86_64.sh       | 16 +++++++++++----
>  2 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 1dd3f48b3d29..17a37134f46a 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -154,8 +154,8 @@ esac
>  domU_config="
>  type = '${domU_type}'
>  name = 'domU'
> -kernel = '/boot/vmlinuz'
> -ramdisk = '/boot/initrd-domU'
> +kernel = '/root/vmlinuz-domU'
> +ramdisk = '/root/initrd-domU'
>  cmdline = 'root=/dev/ram0 console=hvc0'
>  memory = 512
>  vif = [ ${domU_vif} ]
> @@ -185,12 +185,24 @@ Kernel \r on an \m (\l)
>      find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
>      cd ..
>      rm -rf rootfs
> +
> +    # Package domU kernel+rootfs in /root for dom0 (uncompressed)
> +    mkdir -p rootfs/root
> +    cd rootfs
> +    cp ../binaries/bzImage root/vmlinuz-domU
> +    cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
> +    find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
> +    cd ..
> +    rm -rf rootfs
>  fi
>  
>  # Dom0 rootfs.  The order or concatination is important; ucode wants to come
>  # first, and all uncompressed must be ahead of compressed.
>  parts=(
>      binaries/ucode.cpio
> +)
> +[ -n "$domU_check" ] && parts+=(binaries/domU-in-dom0.cpio)

This is a NIT but I have been trying to avoid this format in favor of

if [ -n "$domU_check" ]
then
    parts+=(binaries/domU-in-dom0.cpio)
fi

for readibility.


I can see the patch is correct. It adds a bit of complexity in exchange
for a small improvement. I am not sure if the trade off is worth it, but
I'll ack it anyway.

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

I can change the style of the check on commit


> +parts+=(
>      binaries/rootfs.cpio.gz
>      binaries/xen-tools.cpio.gz
>  )
> @@ -238,10 +250,6 @@ mkdir -p etc/default
>  echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
>  echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
>  mkdir -p var/log/xen/console
> -cp ../binaries/bzImage boot/vmlinuz
> -if [ -n "$domU_check" ]; then
> -    cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
> -fi
>  find . | cpio -H newc -o | gzip >> ../binaries/dom0-rootfs.cpio.gz
>  cd ..
>  
> diff --git a/automation/scripts/xilinx-smoke-dom0-x86_64.sh b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
> index 0fbabb41054a..29817ff81d0a 100755
> --- a/automation/scripts/xilinx-smoke-dom0-x86_64.sh
> +++ b/automation/scripts/xilinx-smoke-dom0-x86_64.sh
> @@ -22,8 +22,8 @@ DOMU_CMD=""
>  DOMU_CFG='
>  type = "pvh"
>  name = "domU"
> -kernel = "/boot/vmlinuz"
> -ramdisk = "/boot/initrd-domU"
> +kernel = "/root/vmlinuz-domU"
> +ramdisk = "/root/initrd-domU"
>  extra = "root=/dev/ram0 console=hvc0"
>  memory = 512
>  '
> @@ -103,10 +103,20 @@ find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
>  cd ..
>  rm -rf rootfs
>  
> +# Package domU kernel+rootfs in /root for dom0 (uncompressed)
> +mkdir -p rootfs/root
> +cd rootfs
> +cp ../binaries/bzImage root/vmlinuz-domU
> +cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
> +find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
> +cd ..
> +rm -rf rootfs
> +
>  # Dom0 rootfs.  The order or concatination is important; ucode wants to come
>  # first, and all uncompressed must be ahead of compressed.
>  parts=(
>      binaries/ucode.cpio
> +    binaries/domU-in-dom0.cpio
>      binaries/rootfs.cpio.gz
>      binaries/xen-tools.cpio.gz
>  )
> @@ -127,8 +137,6 @@ echo "${DOMU_CFG}${DOMU_CFG_EXTRA}" > etc/xen/domU.cfg
>  echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
>  echo "QEMU_XEN=/bin/false" >> etc/default/xencommons
>  mkdir -p var/log/xen/console
> -cp ../binaries/bzImage boot/vmlinuz
> -cp ../binaries/domU-rootfs.cpio.gz boot/initrd-domU
>  find . | cpio -H newc -o | gzip >> ../binaries/dom0-rootfs.cpio.gz
>  cd ..
>  
> -- 
> 2.39.5
> 

Re: [PATCH 3/3] CI: Adjust how domU is packaged in dom0

[Andrew Cooper]

On 28/05/2025 1:30 am, Stefano Stabellini wrote:
> On Thu, 22 May 2025, Andrew Cooper wrote:
>> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
>> index 1dd3f48b3d29..17a37134f46a 100755
>> --- a/automation/scripts/qubes-x86-64.sh
>> +++ b/automation/scripts/qubes-x86-64.sh
>> @@ -185,12 +185,24 @@ Kernel \r on an \m (\l)
>>      find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
>>      cd ..
>>      rm -rf rootfs
>> +
>> +    # Package domU kernel+rootfs in /root for dom0 (uncompressed)
>> +    mkdir -p rootfs/root
>> +    cd rootfs
>> +    cp ../binaries/bzImage root/vmlinuz-domU
>> +    cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
>> +    find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
>> +    cd ..
>> +    rm -rf rootfs
>>  fi
>>  
>>  # Dom0 rootfs.  The order or concatination is important; ucode wants to come
>>  # first, and all uncompressed must be ahead of compressed.
>>  parts=(
>>      binaries/ucode.cpio
>> +)
>> +[ -n "$domU_check" ] && parts+=(binaries/domU-in-dom0.cpio)
> This is a NIT but I have been trying to avoid this format in favor of
>
> if [ -n "$domU_check" ]
> then
>     parts+=(binaries/domU-in-dom0.cpio)
> fi
>
> for readibility.

This is a weird one, because the (relevant) readability is the
components of parts, and it's easier to scan without the extra blank
lines.  Nevertheless, ...

>
>
> I can see the patch is correct. It adds a bit of complexity in exchange
> for a small improvement. I am not sure if the trade off is worth it, but
> I'll ack it anyway.

... see the thread on the previous patch.  This was the RFC "I'd like to
find a nicer way of doing it", and Anthony has made a suggestion which I
need to experiment with.

> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Thanks, but do you have any input on the /boot vs /root question on the
other part of the thread?

~Andrew

Re: [PATCH 3/3] CI: Adjust how domU is packaged in dom0

[Stefano Stabellini]

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

On Wed, 28 May 2025, Andrew Cooper wrote:
> On 28/05/2025 1:30 am, Stefano Stabellini wrote:
> > On Thu, 22 May 2025, Andrew Cooper wrote:
> >> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> >> index 1dd3f48b3d29..17a37134f46a 100755
> >> --- a/automation/scripts/qubes-x86-64.sh
> >> +++ b/automation/scripts/qubes-x86-64.sh
> >> @@ -185,12 +185,24 @@ Kernel \r on an \m (\l)
> >>      find . | cpio -H newc -o | gzip >> ../binaries/domU-rootfs.cpio.gz
> >>      cd ..
> >>      rm -rf rootfs
> >> +
> >> +    # Package domU kernel+rootfs in /root for dom0 (uncompressed)
> >> +    mkdir -p rootfs/root
> >> +    cd rootfs
> >> +    cp ../binaries/bzImage root/vmlinuz-domU
> >> +    cp ../binaries/domU-rootfs.cpio.gz root/initrd-domU
> >> +    find . | cpio -H newc -o > ../binaries/domU-in-dom0.cpio
> >> +    cd ..
> >> +    rm -rf rootfs
> >>  fi
> >>  
> >>  # Dom0 rootfs.  The order or concatination is important; ucode wants to come
> >>  # first, and all uncompressed must be ahead of compressed.
> >>  parts=(
> >>      binaries/ucode.cpio
> >> +)
> >> +[ -n "$domU_check" ] && parts+=(binaries/domU-in-dom0.cpio)
> > This is a NIT but I have been trying to avoid this format in favor of
> >
> > if [ -n "$domU_check" ]
> > then
> >     parts+=(binaries/domU-in-dom0.cpio)
> > fi
> >
> > for readibility.
> 
> This is a weird one, because the (relevant) readability is the
> components of parts, and it's easier to scan without the extra blank
> lines.  Nevertheless, ...
> 
> >
> >
> > I can see the patch is correct. It adds a bit of complexity in exchange
> > for a small improvement. I am not sure if the trade off is worth it, but
> > I'll ack it anyway.
> 
> ... see the thread on the previous patch.  This was the RFC "I'd like to
> find a nicer way of doing it", and Anthony has made a suggestion which I
> need to experiment with.

OK


> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> Thanks, but do you have any input on the /boot vs /root question on the
> other part of the thread?

I saw it. I don't have a strong opinion either way. Technically /boot is
correct but then things gets mixed up with Dom0 kernel and ramdisk.
/root allows it to be more clearly separated so I am OK with it.

Re: [PATCH 2/3] CI: Use bash arrays to simplfy dom0 rootfs construction

[Anthony PERARD]

On Tue, May 27, 2025 at 04:24:57PM +0100, Andrew Cooper wrote:
> On 27/05/2025 4:19 pm, Marek Marczykowski-Górecki wrote:
> > On Tue, May 27, 2025 at 04:01:34PM +0200, Anthony PERARD wrote:
> >> On Thu, May 22, 2025 at 06:36:39PM +0100, Andrew Cooper wrote:
> >>> For Qubes, this requires switching from sh to bash.
> >>>
> >>> This reduces the number of times the target filename needs to be written to 1.
> >>>
> >>> Expand the comment to explain the concatination constraints.
> >> Isn't the correct spelling "concatenation"? Same for the two comments.
> >>
> >>> No functional change.
> >>>
> >>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >>> ---
> >>> I would like to find a slightly nicer way of conditional parts, but nothing
> >>> comes to mind.
> >> Well, one way I can think of is having a new variable which can carry
> >> the rootfs part associated with a particular test, then that variable
> >> can be updated at the time we configure for that test. Something like:
> >>
> >> # init
> >> declare -a append_rootfs_part
> >> # or append_rootfs_part=() is probably fine too.
> >>
> >> case $test in
> >>   argo)
> >>     append_rootfs_part+=(argo.cpio.gz)
> >>     # ... other test configuration
> >>     ;;
> >> esac
> >>
> >> # Dom0 rootfs
> >> parts=(
> >>     rootfs.cpio.gz
> >>     xen-tools.cpio.gz
> >>     "${append_rootfs_part[@]}"
> >> )
> >>
> >> And that should works fine, even if there isn't any extra rootfs part.
> > That would work for compressed parts, but not for uncompressed - which
> > need to come before all compressed. But maybe there could be two arrays
> > - one for uncompressed and another for compressed? Then, each could be
> > extended anywhere, without messing the order.

You could use "${append_rootfs_part:#*.gz}" and
"${(M)append_rootfs_part:#*.gz}" to grab the uncompressed part then the
compressed part... on zsh :-). But something similar could be codded in
bash. But I guess two variables will be more acceptable.

> Hmm, two might work, but they surely need to not be quoted when forming
> parts=(), or having multiple entries will go wrong on the eventual cat
> command line.

The double quote are needed! Well not really because it's very unlikely
that there's going to be blanked characters in paths to parts.

Maybe this will help understand how "${var[@]}" is expended into
multiple arguments:

# Testing with just for loop, also show the difference between
#  "${v[@]}" and "${v[*]}":

$ parts=(one two)
$ for i in "${parts[@]}"; do echo "- $i"; done
- one
- two
$ for i in "${parts[*]}"; do echo "- $i"; done
- one two
$ extra=("first extra" "second extra")
$ for i in "${extra[@]}"; do echo "- $i"; done
- first extra
- second extra
$ parts=(one "${extra[@]}" two)
$ for i in "${parts[@]}"; do echo "- $i"; done
- one
- first extra
- second extra
- two

# And now with function

$ print_array(){ for i in "$@"; do echo "- $i"; done; }
$ print_array "${parts[@]}"
- one
- first extra
- second extra
- two
$ print_array ${parts[@]}
- one
- first
- extra
- second
- extra
- two

Cheers,

-- 
Anthony PERARD

Re: [PATCH 2/3] CI: Use bash arrays to simplfy dom0 rootfs construction

[Andrew Cooper]

On 28/05/2025 10:45 am, Anthony PERARD wrote:
> On Tue, May 27, 2025 at 04:24:57PM +0100, Andrew Cooper wrote:
>> On 27/05/2025 4:19 pm, Marek Marczykowski-Górecki wrote:
>>> On Tue, May 27, 2025 at 04:01:34PM +0200, Anthony PERARD wrote:
>>>> On Thu, May 22, 2025 at 06:36:39PM +0100, Andrew Cooper wrote:
>>>>> For Qubes, this requires switching from sh to bash.
>>>>>
>>>>> This reduces the number of times the target filename needs to be written to 1.
>>>>>
>>>>> Expand the comment to explain the concatination constraints.
>>>> Isn't the correct spelling "concatenation"? Same for the two comments.
>>>>
>>>>> No functional change.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>>> ---
>>>>> I would like to find a slightly nicer way of conditional parts, but nothing
>>>>> comes to mind.
>>>> Well, one way I can think of is having a new variable which can carry
>>>> the rootfs part associated with a particular test, then that variable
>>>> can be updated at the time we configure for that test. Something like:
>>>>
>>>> # init
>>>> declare -a append_rootfs_part
>>>> # or append_rootfs_part=() is probably fine too.
>>>>
>>>> case $test in
>>>>   argo)
>>>>     append_rootfs_part+=(argo.cpio.gz)
>>>>     # ... other test configuration
>>>>     ;;
>>>> esac
>>>>
>>>> # Dom0 rootfs
>>>> parts=(
>>>>     rootfs.cpio.gz
>>>>     xen-tools.cpio.gz
>>>>     "${append_rootfs_part[@]}"
>>>> )
>>>>
>>>> And that should works fine, even if there isn't any extra rootfs part.
>>> That would work for compressed parts, but not for uncompressed - which
>>> need to come before all compressed. But maybe there could be two arrays
>>> - one for uncompressed and another for compressed? Then, each could be
>>> extended anywhere, without messing the order.
> You could use "${append_rootfs_part:#*.gz}" and
> "${(M)append_rootfs_part:#*.gz}" to grab the uncompressed part then the
> compressed part... on zsh :-). But something similar could be codded in
> bash. But I guess two variables will be more acceptable.

I believe there's a restriction that only one type of compression can be
used, but I don't particularly fancy tying it to gz specifically.

Something else to look at in some copious free time is .xz or so.  For
test-artefacts its surely a size win, but whether it's better overall
depends on whether using xz in this script doesn't undo the
optimisations we've been trying.  Once this series is in, we're down to
a handful of tiny text files, so I expect it to be in the noise.

>> Hmm, two might work, but they surely need to not be quoted when forming
>> parts=(), or having multiple entries will go wrong on the eventual cat
>> command line.
> The double quote are needed!

Yes, sorry.  That was a stupid suggestion of mine.  I really ought to
know how "$@" works by now...

~Andrew