[PATCH OSSTEST] Add basic PVH flights.

[PATCH OSSTEST] Add basic PVH flights.

[Ian Campbell]

These are the usual PV debian flights with pvh=1 added to the
configuration file.

A job is created for each of Intel and AMD, although obviously AMD is
expected to fail at the moment.

In my testing I got:
    (XEN) Attempt to create a PVH guest on a system without necessary hardware support
because my test box happens to be AMD.

I have confirmed that the pvh=1 option is correctly present in the
guest cfg for the new pvh job, and that no pvh= is present at all in
the existing test-amd64-amd64-xl job (which is expected and desired if
no pvh runvar is present).

Beyond that I've not tested this at all I fully expect even Intel to
fail in the first instance, due to issues such as lack of necessary
kernel options etc. I suggest to take this now and iterate on any
further changes.

For a xen-unstable flight this results in these runvars:
$ ./mg-show-flight-runvars pvh| grep -- -pvh | sort
test-amd64-amd64-xl-pvh-amd               all_hostflags               arch-amd64,arch-xen-amd64,suite-wheezy,purpose-test,hvm-amd
test-amd64-amd64-xl-pvh-amd               arch                        amd64
test-amd64-amd64-xl-pvh-amd               buildjob                    build-amd64
test-amd64-amd64-xl-pvh-amd               debian_arch                 amd64
test-amd64-amd64-xl-pvh-amd               debian_kernkind             pvops
test-amd64-amd64-xl-pvh-amd               debian_pvh                  1
test-amd64-amd64-xl-pvh-amd               kernbuildjob                build-amd64-pvops
test-amd64-amd64-xl-pvh-amd               kernkind                    pvops
test-amd64-amd64-xl-pvh-amd               toolstack                   xl
test-amd64-amd64-xl-pvh-amd               xenbuildjob                 build-amd64
test-amd64-amd64-xl-pvh-intel             all_hostflags               arch-amd64,arch-xen-amd64,suite-wheezy,purpose-test,hvm-intel
test-amd64-amd64-xl-pvh-intel             arch                        amd64
test-amd64-amd64-xl-pvh-intel             buildjob                    build-amd64
test-amd64-amd64-xl-pvh-intel             debian_arch                 amd64
test-amd64-amd64-xl-pvh-intel             debian_kernkind             pvops
test-amd64-amd64-xl-pvh-intel             debian_pvh                  1
test-amd64-amd64-xl-pvh-intel             kernbuildjob                build-amd64-pvops
test-amd64-amd64-xl-pvh-intel             kernkind                    pvops
test-amd64-amd64-xl-pvh-intel             toolstack                   xl
test-amd64-amd64-xl-pvh-intel             xenbuildjob                 build-amd64

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 make-flight     | 23 +++++++++++++++++++++++
 ts-debian-fixup |  6 ++++++
 2 files changed, 29 insertions(+)

diff --git a/make-flight b/make-flight
index 9963a46..a91f256 100755
--- a/make-flight
+++ b/make-flight
@@ -309,6 +309,17 @@ test_matrix_do_one () {
   *)               test_xend=n ;;
   esac
 
+  # PVH tests for versions >= 4.5 only
+  case "$xenbranch" in
+  xen-3.*-testing) test_pvh=n ;;
+  xen-4.0-testing) test_pvh=n ;;
+  xen-4.1-testing) test_pvh=n ;;
+  xen-4.2-testing) test_pvh=n ;;
+  xen-4.3-testing) test_pvh=n ;;
+  xen-4.4-testing) test_pvh=n ;;
+  *)               test_pvh=y ;;
+  esac
+
   do_rumpkernel_tests
 
   # xend PV guest test on x86 only
@@ -364,6 +375,18 @@ test_matrix_do_one () {
 
   fi
 
+  if [ x$test_pvh = xy -a $xenarch = amd64 -a $dom0arch = amd64 ]; then
+
+    for cpuvendor in amd intel; do
+
+      job_create_test test-$xenarch$kern-$dom0arch-xl-pvh-$cpuvendor \
+                test-debian xl $xenarch $dom0arch \
+                debian_pvh=1 $debian_runvars \
+                all_hostflags=$most_hostflags,hvm-$cpuvendor
+
+    done
+
+  fi
   do_passthrough_tests
 }
 
diff --git a/ts-debian-fixup b/ts-debian-fixup
index f001418..00477c5 100755
--- a/ts-debian-fixup
+++ b/ts-debian-fixup
@@ -118,6 +118,12 @@ sub otherfixupcfg () {
     $cfg =~ s/^vcpus.*//mg;
     $cfg .= "\nvcpus = $vcpus\n";
 
+    my $pvh = guest_var($gho,'pvh',undef);
+    if ($pvh) {
+	$cfg =~ s/^pvh.*//mg;
+	$cfg .= "\npvh=$pvh\n";
+    }
+
     # PCI passthrough
     # Look for runvars   <gn>_pcipassthrough_<devtype>=<hostident>
     # and pass through all matching devices from the specified host.
-- 
2.1.1

Re: [PATCH OSSTEST] Add basic PVH flights.

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST] Add basic PVH flights."):
> These are the usual PV debian flights with pvh=1 added to the
> configuration file.
> 
> A job is created for each of Intel and AMD, although obviously AMD is
> expected to fail at the moment.
...
> Beyond that I've not tested this at all I fully expect even Intel to
> fail in the first instance, due to issues such as lack of necessary
> kernel options etc. I suggest to take this now and iterate on any
> further changes.

That seems reasonable.

> For a xen-unstable flight this results in these runvars:
> diff --git a/ts-debian-fixup b/ts-debian-fixup
> index f001418..00477c5 100755
> --- a/ts-debian-fixup
> +++ b/ts-debian-fixup
> @@ -118,6 +118,12 @@ sub otherfixupcfg () {
...
> +    my $pvh = guest_var($gho,'pvh',undef);
> +    if ($pvh) {
> +	$cfg =~ s/^pvh.*//mg;

This should probably be

  +	$cfg =~ s/^pvh\b.*//mg;

unless you deliberately intend to strip out any other phv-related
settings which xen-create-image might put there ?

Thanks,
Ian.

Re: [PATCH OSSTEST] Add basic PVH flights.

[Ian Campbell]

On Wed, 2014-12-10 at 13:56 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST] Add basic PVH flights."):
> > These are the usual PV debian flights with pvh=1 added to the
> > configuration file.
> > 
> > A job is created for each of Intel and AMD, although obviously AMD is
> > expected to fail at the moment.
> ...
> > Beyond that I've not tested this at all I fully expect even Intel to
> > fail in the first instance, due to issues such as lack of necessary
> > kernel options etc. I suggest to take this now and iterate on any
> > further changes.
> 
> That seems reasonable.
> 
> > For a xen-unstable flight this results in these runvars:
> > diff --git a/ts-debian-fixup b/ts-debian-fixup
> > index f001418..00477c5 100755
> > --- a/ts-debian-fixup
> > +++ b/ts-debian-fixup
> > @@ -118,6 +118,12 @@ sub otherfixupcfg () {
> ...
> > +    my $pvh = guest_var($gho,'pvh',undef);
> > +    if ($pvh) {
> > +	$cfg =~ s/^pvh.*//mg;
> 
> This should probably be
> 
>   +	$cfg =~ s/^pvh\b.*//mg;
> 
> unless you deliberately intend to strip out any other phv-related
> settings which xen-create-image might put there ?

Nope, your suggest is a good one.

Shall I resent or are you ok for me to do this change as I commit?

Re: [PATCH OSSTEST] Add basic PVH flights.

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST] Add basic PVH flights."):
> On Wed, 2014-12-10 at 13:56 +0000, Ian Jackson wrote:
> > This should probably be
> > 
> >   +	$cfg =~ s/^pvh\b.*//mg;
> > 
> > unless you deliberately intend to strip out any other phv-related
> > settings which xen-create-image might put there ?
> 
> Nope, your suggest is a good one.
> 
> Shall I resent or are you ok for me to do this change as I commit?

Please go ahead, but can you please first double check that it still
does actually still edit the config file as desired and cause the test
failure on your machine ?  It would be annoying if that line ceased to
take effect and the job spuriously passed.

That said,

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Ian.

Re: [PATCH OSSTEST] Add basic PVH flights.

[Ian Campbell]

On Wed, 2014-12-10 at 14:03 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH OSSTEST] Add basic PVH flights."):
> > On Wed, 2014-12-10 at 13:56 +0000, Ian Jackson wrote:
> > > This should probably be
> > > 
> > >   +	$cfg =~ s/^pvh\b.*//mg;
> > > 
> > > unless you deliberately intend to strip out any other phv-related
> > > settings which xen-create-image might put there ?
> > 
> > Nope, your suggest is a good one.
> > 
> > Shall I resent or are you ok for me to do this change as I commit?
> 
> Please go ahead, but can you please first double check that it still
> does actually still edit the config file as desired and cause the test
> failure on your machine ?  It would be annoying if that line ceased to
> take effect and the job spuriously passed.

I checked both the pvh and non-pvh job and they did/didn't contain a
pvh=1 as expected.

> That said,
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks. As discussed on IRC I've added it to my to-push queue which is
pending the current pretest stuff propagating. I'll flush that once I
see a pass of osstest's own gate.

The queue contains:
$ git log --oneline origin/pretest..pretest 
082e565 Add basic PVH flights.
1f110e8 Osstest/Debian: support adding a rootdelay property to bootargs
f254b4d Osstest/Debian: Add support for "ExtraInitramfsModules" host property
7d8be54 Osstest/Debian: Refactor code to set bootargs in u-boot script
1b01799 ts-debian-install: rename cfg_xend to cfg
1f48acb gitignore: ignore images directory
1b95fe3 README: list chiark-utils-bin as requirement
abc19f4 TestSupport: allow overriding of on_* in prepareguest_part_xencfg

It's also in the pretest branch of my osstest tree on xenbits.

(Wei, just FYI since some patches of yours are in there)

Ian.