On Mon, 21 Oct 2024, Andrew Cooper wrote:
> > Bah - serves me right for some last minute refactoring. The domain type
> > should be pvh for pvshim=1 to work.
> >
> > New pipeline:
> > https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1505540810
>
> And from the same bit of refactoring, a mismatch between
> domU_extra_{cfg,config}. Consolidated on the latter.
>
> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/8143613752
I reviewed the commit c80e9241209cc9ed7f66c3f45275f837ddafc21c from your
branch instead. See below.
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index b27c2be174..e76a37bef3 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -240,6 +240,14 @@ adl-pci-hvm-x86-64-gcc-debug:
> - *x86-64-test-needs
> - alpine-3.18-gcc-debug
>
> +adl-pvshim-x86-64-gcc-debug:
> + extends: .adl-x86-64
> + script:
> + - ./automation/scripts/qubes-x86-64.sh pvshim 2>&1 | tee ${LOGFILE}
> + needs:
> + - *x86-64-test-needs
> + - alpine-3.18-gcc-debug
> +
> zen3p-smoke-x86-64-gcc-debug:
> extends: .zen3p-x86-64
> script:
> @@ -272,6 +280,14 @@ zen3p-pci-hvm-x86-64-gcc-debug:
> - *x86-64-test-needs
> - alpine-3.18-gcc-debug
>
> +zen3p-pvshim-x86-64-gcc-debug:
> + extends: .zen3p-x86-64
> + script:
> + - ./automation/scripts/qubes-x86-64.sh pvshim 2>&1 | tee ${LOGFILE}
> + needs:
> + - *x86-64-test-needs
> + - alpine-3.18-gcc-debug
> +
> qemu-smoke-dom0-arm64-gcc:
> extends: .qemu-arm64
> script:
> diff --git a/automation/scripts/qubes-x86-64.sh
> b/automation/scripts/qubes-x86-64.sh
> index 76fbafac84..95090eb12c 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -8,6 +8,7 @@ set -ex
> # - dom0pvh-hvm PVH dom0, HVM domU
> # - pci-hvm PV dom0, HVM domU + PCI Passthrough
> # - pci-pv PV dom0, PV domU + PCI Passthrough
> +# - pvshim PV dom0, PVSHIM domU
> # - s3 PV dom0, S3 suspend/resume
> test_variant=$1
>
> @@ -20,8 +21,8 @@ domU_vif="'bridge=xenbr0',"
> domU_extra_config=
>
> case "${test_variant}" in
> - ### test: smoke test & smoke test PVH & smoke test HVM
> - ""|"dom0pvh"|"dom0pvh-hvm")
> + ### test: smoke test & smoke test PVH & smoke test HVM & smoke test
> PVSHIM
> + ""|"dom0pvh"|"dom0pvh-hvm"|"pvshim")
> passed="ping test passed"
> domU_check="
> ifconfig eth0 192.168.0.2
> @@ -44,6 +45,11 @@ echo \"${passed}\"
>
> if [ "${test_variant}" = "dom0pvh-hvm" ]; then
> domU_type="hvm"
> + elif [ "${test_variant}" = "pvshim" ]; then
> + domU_type="pvh"
This is not necessary since PVH is already the default. In theory, it is
harmless, but it caused me to do a double-take because I initially
thought I was missing something, given that PVH is expected to be the
default at this point.
> + domU_extra_config='
> +pvshim = 1
> +'
Is there a reason this cannot be:
domU_extra_config='pvshim = 1'
?
These are just minor cosmetics.
> fi
> ;;
>
