| Message ID | 20241021143539.3898995-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add {adl,zen3p}-pvshim-* tests | expand |
On 21/10/2024 3:35 pm, Andrew Cooper wrote: > GitlabCI has no testing of Xen's PVH entrypoint. Fix this. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > CC: Daniel P. Smith <dpsmith@apertussolutions.com> > CC: Anthony PERARD <anthony.perard@vates.tech> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Michal Orzel <michal.orzel@amd.com> > CC: Doug Goldstein <cardoe@cardoe.com> > > OSSTest (which is disappearing imminently) found a pvshim bug in the > hyperlaunch series, and I found a second shortly after while trying to take > more of the series. > > https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1505518838 > --- > automation/gitlab-ci/test.yaml | 16 ++++++++++++++++ > automation/scripts/qubes-x86-64.sh | 10 ++++++++-- > 2 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index b27c2be17487..e76a37bef32d 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 4b6311efffa8..ace494b938d8 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_cfg= > > 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="pv" > + domU_extra_cfg=' > +pvshim = 1 > +' > fi > ;; > 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 ~Andrew
On 21/10/2024 3:41 pm, Andrew Cooper wrote: > On 21/10/2024 3:35 pm, Andrew Cooper wrote: >> GitlabCI has no testing of Xen's PVH entrypoint. Fix this. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >> CC: Daniel P. Smith <dpsmith@apertussolutions.com> >> CC: Anthony PERARD <anthony.perard@vates.tech> >> CC: Stefano Stabellini <sstabellini@kernel.org> >> CC: Michal Orzel <michal.orzel@amd.com> >> CC: Doug Goldstein <cardoe@cardoe.com> >> >> OSSTest (which is disappearing imminently) found a pvshim bug in the >> hyperlaunch series, and I found a second shortly after while trying to take >> more of the series. >> >> https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1505518838 >> --- >> automation/gitlab-ci/test.yaml | 16 ++++++++++++++++ >> automation/scripts/qubes-x86-64.sh | 10 ++++++++-- >> 2 files changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml >> index b27c2be17487..e76a37bef32d 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 4b6311efffa8..ace494b938d8 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_cfg= >> >> 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="pv" >> + domU_extra_cfg=' >> +pvshim = 1 >> +' >> fi >> ;; >> > 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 ~Andrew
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 > ;; >
On 21/10/2024 11:28 pm, Stefano Stabellini wrote: > On Mon, 21 Oct 2024, Andrew Cooper wrote: >> 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 >> @@ -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. That was more fallout of refactoring. My first attempt (which worked) involved writing a whole domU_config= block, and then I decided that was a bad thing. Selecting PVShim without also forcing type to PVH is a little fragile if anyone changes the domU_type default. I think it's cleaner to keep both together, even if it happens to be re-selecting the default. >> + domU_extra_config=' >> +pvshim = 1 >> +' > Is there a reason this cannot be: > > domU_extra_config='pvshim = 1' > > ? > > These are just minor cosmetics. Again, refactoring from before pulling out domU_type. I'm not fussed - I can shrink this to one line. ~Andrew
On Tue, Oct 22, 2024 at 02:25:54PM +0100, Andrew Cooper wrote: > On 21/10/2024 11:28 pm, Stefano Stabellini wrote: > > On Mon, 21 Oct 2024, Andrew Cooper wrote: > >> 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 > >> @@ -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. > > That was more fallout of refactoring. > > My first attempt (which worked) involved writing a whole domU_config= > block, and then I decided that was a bad thing. > > Selecting PVShim without also forcing type to PVH is a little fragile if > anyone changes the domU_type default. I think it's cleaner to keep both > together, even if it happens to be re-selecting the default. IMO it's okay to leave re-setting the default here explicitly. > >> + domU_extra_config=' > >> +pvshim = 1 > >> +' > > Is there a reason this cannot be: > > > > domU_extra_config='pvshim = 1' > > > > ? > > > > These are just minor cosmetics. > > Again, refactoring from before pulling out domU_type. > > I'm not fussed - I can shrink this to one line. Either is fine for me. The short one is more readable now, while the long one is easier to extend in the future. Anyway, for the series (c80e9241209cc9ed7f66c3f45275f837ddafc21c and previous two): Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index b27c2be17487..e76a37bef32d 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 4b6311efffa8..ace494b938d8 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_cfg= 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="pv" + domU_extra_cfg=' +pvshim = 1 +' fi ;;
GitlabCI has no testing of Xen's PVH entrypoint. Fix this. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> CC: Daniel P. Smith <dpsmith@apertussolutions.com> CC: Anthony PERARD <anthony.perard@vates.tech> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Michal Orzel <michal.orzel@amd.com> CC: Doug Goldstein <cardoe@cardoe.com> OSSTest (which is disappearing imminently) found a pvshim bug in the hyperlaunch series, and I found a second shortly after while trying to take more of the series. https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1505518838 --- automation/gitlab-ci/test.yaml | 16 ++++++++++++++++ automation/scripts/qubes-x86-64.sh | 10 ++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-)