Re: [PATCH 3/3] automation: add a QEMU based x86_64 Dom0/DomU test

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 22 Oct 2021, Anthony PERARD wrote:
> On Thu, Oct 21, 2021 at 04:08:39PM -0700, Stefano Stabellini wrote:
> > diff --git a/automation/scripts/qemu-alpine-x86_64.sh 
> > b/automation/scripts/qemu-alpine-x86_64.sh
> > new file mode 100644
> > index 0000000000..41b05210d6
> > --- /dev/null
> > +++ b/automation/scripts/qemu-alpine-x86_64.sh
> > @@ -0,0 +1,92 @@
> > +#!/bin/bash
> > +
> > +set -ex
> > +
> > +apt-get -qy update
> > +apt-get -qy install --no-install-recommends qemu-system-x86 \
> > +                                            cpio \
> > +                                            curl \
> > +                                            busybox-static
> 
> Please, don't install packages during the CI job. If you need new
> packages, update the container.

The container used to run this script is the one used for the Xen build:
automation/build/debian/unstable.dockerfile, AKA
registry.gitlab.com/xen-project/xen/debian:unstable. We don't have a
specific container for the sole purpose of running tests.

Thus, I could add qemu-system-x86 to
automation/build/debian/unstable.dockerfile, but then we increase the
size of the debian unstable build container needlessly for all related
build jobs.

Or we could add one more special container just for running tests, but
then it is one more container to maintain and keep up-to-date.

This is why I chose as a compromise to keep the underlying container as
is, and just apt-get the extra 3-4 packages here. It is the same thing
we do on ARM: automation/scripts/qemu-alpine-arm64.sh. Also keep in mind
that this job is run in the "test" step where we have far fewer jobs at
the moment and the runners are not busy. (It would be different in the
"build" step where we have many jobs.)

I am not entirely sure what is the best solution overall, but for this
series at this stage I would prefer to keep the same strategy used for
the ARM tests (i.e. reuse the debian unstable build container and
apt-get the few missing packages.) If we do change the way we do it, I
would rather change both x86 and ARM at the same time.


> That said, "curl" doesn't seems to be needed.

Yeah I'll take away curl.


> > +# DomU Busybox
> > +cd binaries
> > +mkdir -p initrd
> > +mkdir -p initrd/bin
> > +mkdir -p initrd/sbin
> > +mkdir -p initrd/etc
> > +mkdir -p initrd/dev
> > +mkdir -p initrd/proc
> > +mkdir -p initrd/sys
> > +mkdir -p initrd/lib
> > +mkdir -p initrd/var
> > +mkdir -p initrd/mnt
> > +cp /bin/busybox initrd/bin/busybox
> > +initrd/bin/busybox --install initrd/bin
> > +echo "#!/bin/sh
> > +
> > +mount -t proc proc /proc
> > +mount -t sysfs sysfs /sys
> > +mount -t devtmpfs devtmpfs /dev
> > +/bin/sh" > initrd/init
> > +chmod +x initrd/init
> > +cd initrd
> > +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
> 
> This isn't confusing at all, depending on the tool used to make an
> archive, the resulting initrd has a different purpose :-).
>     initrd.tar.gz -> dom0
>     initrd.cpio.gz -> domU
> 
> > +cd ..
> 
> Maybe add a comment here saying that we are now preparing dom0 root
> filesystem? (as there is one for domu)

Sure I can add a comment


> > +mkdir -p rootfs
> > +cd rootfs
> > +tar xvzf ../initrd.tar.gz
> > +mkdir proc
> > +mkdir run
> > +mkdir srv
> > +mkdir sys
> > +rm var/run
> > +cp -ar ../dist/install/* .
> > +mv ../initrd.cpio.gz ./root
> > +cp ../bzImage ./root
> > +echo "name=\"test\"
> > +memory=512
> > +vcpus=1
> > +kernel=\"/root/bzImage\"
> > +ramdisk=\"/root/initrd.cpio.gz\"
> > +extra=\"console=hvc0 root=/dev/ram0 rdinit=/bin/sh\"
> > +" > root/test.cfg
> > +echo "#!/bin/bash
> > +
> 
> Maybe add `set -x` ?

OK


> > +export LD_LIBRARY_PATH=/usr/local/lib
> > +bash /etc/init.d/xencommons start
> > +
> > +xl list
> > +
> > +xl create -c /root/test.cfg
> > +
> > +" > etc/local.d/xen.start
> > +chmod +x etc/local.d/xen.start
> > +echo "rc_verbose=yes" >> etc/rc.conf
> > +find . |cpio -H newc -o|gzip > ../xen-rootfs.cpio.gz
> > +cd ../..
> > +
> > +cat >> binaries/pxelinux.0 <<- EOF
> 
> So, I've look at <<- meaning as I never used that before and it seems to
> remove all leading tab chr from the input, yet they are no tab in the
> input. So maybe use just <<EOF instead, without the dash -.

OK


> > +#!ipxe
> > +
> > +kernel xen console=com1
> > +module bzImage console=hvc0
> > +module xen-rootfs.cpio.gz
> > +boot
> > +EOF