[OSSTEST Nested PATCH v7 0/6] Introduction of netsted HVM test job

[OSSTEST Nested PATCH v7 0/6] Introduction of netsted HVM test job

[longtao.pang]

This patch set adds nested HVM test case for osstest.
In this test case, a Xen hypervisor (L1) runs on top of another Xen hypervisor (L0). 
Upon L1 hypervisor, we will then create a nested guest (L2), and test if the Linux guest can then be installed and run well.
About nested Xen virtualization, refer to http://wiki.xenproject.org/wiki/Nested_Virtualization_in_Xen.

Test steps
	0. To run osstest in standalone mode, write a config file in
           '~/.xen-osstest/config', and then create a standalone.config file to
           define 'TREE_LINUX', 'REVISION_LINUX' and 'NESTED_OS_IMAGE' which
           will be used for nested test. The directry path of 'Debian Images'
           could be difined in '~/.xen-osstest/config'. 
	1. run 'build-amd64' job and then 'build-amd64-pvops', to prepare xen
           installation tarball and hvm guest kernel.
	2. run 'test-amd64-amd6-nested' job, it does following:
		a. invoke test step of 'ts-debain-hvm-install' to install 
		   a normal HVM guest
		b. invoke test step of 'ts-nested-setup' to make some
                   appropriate runvars which selecthost() would recognise and
                   prepare the configurations for installing L2 guest VM. 
		c. invoke test step of 'ts-xen-install' to install xen on 
		   the normal guest, alter it into a L1 hypervisor
		d. invoke test step of 'ts-debain-hvm-install' again, but 
	           take the L1 hypervisor as host, install the L2 guest on it
		e. invoke test step of 'ts-guest-stop', stop L2 guest.
                f. invoke test step of 'ts-guest-destroy' to destroy L1 guest.

This patch set reuses 'ts-debian-hvm-install' for both L1 installation and L2 installation, define 'nested' and 'nested2' as L1 and L2's hostname, define 'nested_l1 as L1's host ident.
It also reuses 'ts-xen-install' with L1's hostname 'nested' input param to differentiate from L0 Xen installation.
This patch series has been tested on test machines of amd64 arch, debian-7.6.0-amd64 as guests OS, with hvm domain0 of Linux kernel 3.18.5, in standalone mode.
Also, we use linux-stable tree as domain0 kernel source.
----------------------------------------------------------------
longtao.pang (6):
      parsing grub which has 'submenu' primitive
      Edit some testsupport APIs for nested test
      Changes on test step of debain hvm guest install
      Add new script to custmize nested test configuration
      Add test job for nest test case
      Compose the main recipe of nested test job

 Osstest/Debian.pm      |   21 ++++++++++++++-----
 Osstest/TestSupport.pm |    8 ++++++-
 make-flight            |   21 +++++++++++++++++++
 sg-run-job             |   11 ++++++++++
 ts-debian-hvm-install  |    8 +++++--
 ts-nested-setup        |   54 ++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 115 insertions(+), 8 deletions(-)
 create mode 100755 ts-nested-setup

[OSSTEST Nested PATCH v7 1/6] parsing grub which has 'submenu' primitive

[longtao.pang]

>From a hvm kernel build from Linux stable Kernel tree,
the auto generated grub2 menu will have 'submenu' primitive, upon the
'menuentry' items. Xen boot entries will be grouped into a submenu. This
patch adds capability to support such grub formats.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v7:
Remove the reformatting change for Debian.pm and keep the original format.
---
 Osstest/Debian.pm |   21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 6784024..35163a0 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -398,10 +398,18 @@ sub setupboot_grub2 ($$$$) {
     
         my $count= 0;
         my $entry;
+        my $submenu;
         while (<$f>) {
             next if m/^\s*\#/ || !m/\S/;
             if (m/^\s*\}\s*$/) {
-                die unless $entry;
+                die unless $entry || $submenu;
+                if(!defined $entry && defined $submenu){
+                    logm("Met end of a submenu starting from ".
+                        "$submenu->{StartLine}. ".
+                        "Our want kern is $want_kernver");
+                    $submenu=undef;
+                    next;
+                }
                 my (@missing) =
                     grep { !defined $entry->{$_} } 
 		        (defined $xenhopt
@@ -432,21 +440,24 @@ sub setupboot_grub2 ($$$$) {
                 $entry= { Title => $1, StartLine => $., Number => $count };
                 $count++;
             }
-            if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
+            if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
+                $submenu={ StartLine =>$.};
+            }
+            if (m/^\s*multiboot\s*(?:\/boot)*\/(xen\S+)/) {
                 die unless $entry;
                 $entry->{Hv}= $1;
             }
-            if (m/^\s*multiboot\s*\/(vmlinu[xz]-(\S+))/) {
+            if (m/^\s*multiboot\s*(?:\/boot)*\/(vmlinu[xz]-(\S+))/) {
                 die unless $entry;
                 $entry->{KernOnly}= $1;
                 $entry->{KernVer}= $2;
             }
-            if (m/^\s*module\s*\/(vmlinu[xz]-(\S+))/) {
+            if (m/^\s*module\s*(?:\/boot)*\/(vmlinu[xz]-(\S+))/) {
                 die unless $entry;
                 $entry->{KernDom0}= $1;
                 $entry->{KernVer}= $2;
             }
-            if (m/^\s*module\s*\/(initrd\S+)/) {
+            if (m/^\s*module\s*(?:\/boot)*\/(initrd\S+)/) {
                 $entry->{Initrd}= $1;
             }
 	    if (m/^\s*module\s*\/(xenpolicy\S+)/) {
-- 
1.7.10.4

[OSSTEST Nested PATCH v7 2/6] Edit some testsupport APIs for nested test

[longtao.pang]

1. Designate vif model to 'e1000' by make-flight.
2. In L2 installation context, its host (L1) IP address is not queried
from DNS, but after running "ts-nested-setup + host + nested", L1 IP is
stored in runvar.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v7:
1.set vif model as e1000 when make-flight
2.remove the changes about multi-reboot-time, since this is not code
issue but the network DNS setting not correct in my previous osstest
env.
---
 Osstest/TestSupport.pm |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index a467713..10c229d 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -819,6 +819,10 @@ sub selecthost ($) {
 	logm("Host $name is in HostGroup $ho->{Properties}{HostGroup}");
     }
 
+    if ( $r{"${name}_ip"} ) {
+        $setprop->("IpAddr", $r{"${name}_ip"});
+    }
+
     # Next, we use the config file's general properites as defaults
     foreach my $k (keys %c) {
 	next unless $k =~ m/^HostProp_([A-Z].*)$/;
@@ -1548,11 +1552,13 @@ sub prepareguest_part_xencfg ($$$$$) {
     my $oncrash= $xopts->{OnCrash} || 'preserve';
     my $vcpus= guest_var($gho, 'vcpus', $xopts->{DefVcpus} || 2);
     my $xoptcfg= $xopts->{ExtraConfig};
+    my $vif= guest_var($gho, 'vifmodel','');
+    my $vifmodel= $vif ? "model=$vif" : '';;
     $xoptcfg='' unless defined $xoptcfg;
     my $xencfg= <<END;
 name        = '$gho->{Name}'
 memory = ${ram_mb}
-vif         = [ 'type=ioemu,mac=$gho->{Ether}' ]
+vif         = [ 'type=ioemu,${vifmodel},mac=$gho->{Ether}' ]
 #
 on_poweroff = '$onpoweroff'
 on_reboot   = '$onreboot'
-- 
1.7.10.4

[OSSTEST Nested PATCH v7 3/6] Changes on test step of debain hvm guest install

[longtao.pang]

1. Increase disk size to accomodate to nested test requirment.
2. Since 'Debain-xxx-.iso' image will be stored there, therefore needs
more disk capacity, increase root partition size in preseed generation.
3. In L1 installation context, assign more memory to it; Since it
acts as a nested hypervisor anyway.
4. Comment out CDROM entry in sources.list to make HTTP URL entry
available for L1 hvm guest.
5. Enable nestedhvm feature in ExtraConfig.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v7:
1. Comment out CDROM entry in sources.list to make HTTP URL entry
available for L1 hvm guest.
2. Enable nestedhvm feature in ExtraConfig.
---
 ts-debian-hvm-install |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index cfd5144..8e676fa 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -36,7 +36,7 @@ our $ho= selecthost($whhost);
 
 # guest memory size will be set based on host free memory, see below
 our $ram_mb;
-our $disk_mb= 10000;
+our $disk_mb= 15000;
 
 our $guesthost= "$gn.guest.osstest";
 our $gho;
@@ -60,7 +60,7 @@ d-i partman-auto/expert_recipe string \\
                         use_filesystem{ } filesystem{ vfat } \\
                         mountpoint{ /boot/efi } \\
                 . \\
-                5000 50 5000 ext4 \\
+                10000 50 10000 ext4 \\
                         method{ format } format{ } \\
                         use_filesystem{ } filesystem{ ext4 } \\
                         mountpoint{ / } \\
@@ -75,6 +75,7 @@ d-i preseed/late_command string \\
         in-target mkdir -p /boot/efi/EFI/boot; \\
         in-target cp /boot/efi/EFI/debian/grubx64.efi /boot/efi/EFI/boot/bootx64.efi ;\\
         in-target mkdir -p /root/.ssh; \\
+        in-target sed -i '/^deb *cdrom/s/^/#/' /etc/apt/sources.list; \\
         in-target sh -c "echo -e '$authkeys'> /root/.ssh/authorized_keys";
 END
     return $preseed_file;
@@ -152,6 +153,7 @@ sub prep () {
     more_prepareguest_hvm($ho,$gho, $ram_mb, $disk_mb,
                           OnReboot => 'preserve',
                           Bios => $r{bios},
+                          ExtraConfig => 'nestedhvm=1',
                           PostImageHook => sub {
         my $cmds = iso_copy_content_from_image($gho, $newiso);
         $cmds .= prepare_initrd($initrddir,$newiso,$preseed_file_path);
@@ -173,6 +175,8 @@ my $ram_minslop = 100;
 my $ram_lots = 5000;
 if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
     $ram_mb = $ram_lots;
+} elsif ($gn eq 'nested') {
+    $ram_mb = 3072;
 } else {
     $ram_mb = 768;
 }
-- 
1.7.10.4

[OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested test configuration

[longtao.pang]

1. In this script, make some appropriate runvars which selecthost would
recognise.
2. Prepare the configurations for installing L2 guest VM.
3. Create a lv disk in L0 and hot-attach it to L1, need to restart L1 to
make the block disk to be recognized by L1 system, then using this disk
to create a VG that used for installing L2.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v7:
Add new ts script for nested configuration according maintainer's reply.
---
 ts-nested-setup |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100755 ts-nested-setup

diff --git a/ts-nested-setup b/ts-nested-setup
new file mode 100755
index 0000000..153c4a3
--- /dev/null
+++ b/ts-nested-setup
@@ -0,0 +1,54 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2015 Intel Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+use strict qw(vars);
+use DBI;
+use Osstest;
+use Osstest::Debian;
+use Osstest::TestSupport;
+
+tsreadconfig();
+our ($ident_l0,$nested_host) = @ARGV;
+our ($l0,$l1) = ts_get_host_guest(@ARGV);
+
+guest_check_ip($l1);
+target_cmd_root($l1, "mkdir -p /home/osstest/.ssh && cp /root/.ssh/authorized_keys /home/osstest/.ssh/");
+my $url = $c{WebspaceUrl}.$c{WebspaceCommon}.$l0->{Name}."_".'overlay.tar';
+target_cmd_root($l1, <<END);
+    wget -O overlay.tar $url
+    tar -xf overlay.tar -C /
+    rm overlay.tar -f
+    update-rc.d osstest-confirm-booted start 99 2 .
+END
+
+store_runvar('nested_l1',$nested_host);
+store_runvar("${nested_host}_ip",$l1->{Ip});
+store_runvar('multi_reboot_time',5);
+
+my $l2_disk_mb = 20000;
+my $l2_lv_name = 'nestedl2';
+my $vgname = $l0->{Name};
+$vgname .= ".$c{TestHostDomain}" if ($l0->{Suite} =~ m/lenny/);
+target_cmd_root($l0, <<END);
+    lvremove -f /dev/${vgname}/${l2_lv_name} ||:
+    lvcreate -L ${l2_disk_mb}M -n $l2_lv_name $vgname
+    dd if=/dev/zero of=/dev/${vgname}/${l2_lv_name} count=10
+    xl block-attach $l1->{Name} /dev/${vgname}/${l2_lv_name},raw,sdb,rw
+END
+target_install_packages_norec($l1, qw(lvm2 rsync genisoimage));
+target_reboot($l1);
+target_cmd_root($l1, "pvcreate /dev/xvdb && vgcreate ${l2_lv_name}_vg /dev/xvdb");
-- 
1.7.10.4

[OSSTEST Nested PATCH v7 5/6] Add test job for nest test case

[longtao.pang]

1. This patch adds creation of the nested test job, when job creation
procedure is invoked.
2. 'NESTED_OS_IMAGE' is the name of 'Debian ISO Images', which defined
in standalone.config.
3. Set nested L1's vif model as e1000 by make-flight.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v7:
Set L1's vif model as e1000 in runvar by make-flight.
---
 make-flight |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/make-flight b/make-flight
index 8ac3a87..b8f266f 100755
--- a/make-flight
+++ b/make-flight
@@ -204,6 +204,26 @@ do_hvm_win7_x64_tests () {
             all_hostflags=$most_hostflags,hvm
 }
 
+do_hvm_debian_nested_tests () {
+  if [ $xenarch != amd64 ]; then
+    return
+  fi
+  if [ $dom0arch != amd64 ]; then
+    return
+  fi
+
+  job_create_test test-$xenarch$kern-$dom0arch-nested test-nested xl \
+			$xenarch $dom0arch \
+            nested_image=$NESTED_OS_IMAGE \
+            nested2_image=$NESTED_OS_IMAGE \
+            bios=seabios \
+            kernbuildjob=build-amd64-pvops \
+            kernkind=pvops \
+            nested_vifmodel='e1000' \
+            device_model_version=qemu-xen \
+            all_hostflags=$most_hostflags,hvm
+}
+
 do_hvm_debian_test_one () {
   testname=$1
   bios=$2
@@ -430,6 +450,7 @@ test_matrix_do_one () {
     done
 
   fi
+  do_hvm_debian_nested_tests
   do_passthrough_tests
 }
 
-- 
1.7.10.4

[OSSTEST Nested PATCH v7 6/6] Compose the main recipe of nested test job

[longtao.pang]

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v7:
Add 'ts-guest-stop' and 'ts-guest-destroy' scripts to shutdown L2 and L1
after finishing test.
---
 sg-run-job |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sg-run-job b/sg-run-job
index a1ff24b..be55da8 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -297,6 +297,17 @@ proc run-job/test-pair {} {
 #    run-ts . remus-failover ts-remus-check         src_host dst_host + debian
 }
 
+proc need-hosts/test-nested {} {return host}
+proc run-job/test-nested {} {
+    run-ts . = ts-debian-hvm-install + host + nested
+    run-ts . = ts-nested-setup + host + nested
+    run-ts . = ts-xen-install nested_l1
+    run-ts . = ts-host-reboot nested_l1
+    run-ts . = ts-debian-hvm-install nested_l1 nested2
+    run-ts . = ts-guest-stop nested_l1 nested2
+    run-ts . = ts-guest-destroy host nested
+}
+
 proc test-guest-migr {g} {
     if {[catch { run-ts . = ts-migrate-support-check + host $g }]} return
 
-- 
1.7.10.4

Re: [OSSTEST Nested PATCH v7 1/6] parsing grub which has 'submenu' primitive

[Ian Campbell]

On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> From a hvm kernel build from Linux stable Kernel tree,
> the auto generated grub2 menu will have 'submenu' primitive, upon the
> 'menuentry' items. Xen boot entries will be grouped into a submenu. This
> patch adds capability to support such grub formats.
> 
> Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> ---
> Changes in v7:
> Remove the reformatting change for Debian.pm and keep the original format.

Thank you.

> ---
>  Osstest/Debian.pm |   21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 6784024..35163a0 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -398,10 +398,18 @@ sub setupboot_grub2 ($$$$) {
>      
>          my $count= 0;
>          my $entry;
> +        my $submenu;
>          while (<$f>) {
>              next if m/^\s*\#/ || !m/\S/;
>              if (m/^\s*\}\s*$/) {
> -                die unless $entry;
> +                die unless $entry || $submenu;
> +                if(!defined $entry && defined $submenu){
> +                    logm("Met end of a submenu starting from ".
> +                        "$submenu->{StartLine}. ".
> +                        "Our want kern is $want_kernver");
> +                    $submenu=undef;
> +                    next;
> +                }
>                  my (@missing) =
>                      grep { !defined $entry->{$_} } 
>  		        (defined $xenhopt
> @@ -432,21 +440,24 @@ sub setupboot_grub2 ($$$$) {
>                  $entry= { Title => $1, StartLine => $., Number => $count };
>                  $count++;
>              }
> -            if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
> +            if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> +                $submenu={ StartLine =>$.};
> +            }

This looks reasonable enough to support a single nesting, I suppose we
can leave more deeply nested submenus for another time.

So in that regard this patch looks ok to me.

> +            if (m/^\s*multiboot\s*(?:\/boot)*\/(xen\S+)/) {
>                  die unless $entry;
>                  $entry->{Hv}= $1;
>              }
> -            if (m/^\s*multiboot\s*\/(vmlinu[xz]-(\S+))/) {
> +            if (m/^\s*multiboot\s*(?:\/boot)*\/(vmlinu[xz]-(\S+))/) {

What are these changes all about? I think they must be unrelated to the
use of submenu (perhaps relate to having a separate /boot or not?). If
so then please do in a separate patch.

If this is somehow to do with submenu then please explain how/why in the
commit log.

BTW, your regex as it stand will accept /boot/boot/boot/boot/vmlinuz. I
think you maybe meant to add "(?:\/boot)?" to match zero or one
occurrences?

Ian.

Re: [OSSTEST Nested PATCH v7 2/6] Edit some testsupport APIs for nested test

[Ian Campbell]

On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> 1. Designate vif model to 'e1000' by make-flight.

Strictly you could s/to 'e1000'// here since the make-flight changes are
elsewhere and that would better describe the generic change.

> 2. In L2 installation context, its host (L1) IP address is not queried
> from DNS, but after running "ts-nested-setup + host + nested", L1 IP is
> stored in runvar.
> 
> Signed-off-by: longtao.pang <longtaox.pang@intel.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST Nested PATCH v7 3/6] Changes on test step of debain hvm guest install

[Ian Campbell]

On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> 1. Increase disk size to accomodate to nested test requirment.

"accommodate" and "requirement"

Ian, are you OK with this generic change or would you prefer a runvar? I
suspect the answer is going to be runvar.

> 2. Since 'Debain-xxx-.iso' image will be stored there, therefore needs
> more disk capacity, increase root partition size in preseed generation.

"Debian".

I'm not sure I follow what this one is saying though, does "there" refer
to the rootfs of the VM which is being built here?

> @@ -75,6 +75,7 @@ d-i preseed/late_command string \\
>          in-target mkdir -p /boot/efi/EFI/boot; \\
>          in-target cp /boot/efi/EFI/debian/grubx64.efi /boot/efi/EFI/boot/bootx64.efi ;\\
>          in-target mkdir -p /root/.ssh; \\
> +        in-target sed -i '/^deb *cdrom/s/^/#/' /etc/apt/sources.list; \\

I think a more conventional way to do this would be to use:
        s/^deb *cdrom/#&/g

>          in-target sh -c "echo -e '$authkeys'> /root/.ssh/authorized_keys";
>  END
>      return $preseed_file;
> @@ -152,6 +153,7 @@ sub prep () {
>      more_prepareguest_hvm($ho,$gho, $ram_mb, $disk_mb,
>                            OnReboot => 'preserve',
>                            Bios => $r{bios},
> +                          ExtraConfig => 'nestedhvm=1',

Please do only for the nested job i.e. using a runvar.

>                            PostImageHook => sub {
>          my $cmds = iso_copy_content_from_image($gho, $newiso);
>          $cmds .= prepare_initrd($initrddir,$newiso,$preseed_file_path);
> @@ -173,6 +175,8 @@ my $ram_minslop = 100;
>  my $ram_lots = 5000;
>  if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
>      $ram_mb = $ram_lots;
> +} elsif ($gn eq 'nested') {
> +    $ram_mb = 3072;

I think this ought to be driven from a runvar as well, e.g. <gn>_minram.

>  } else {
>      $ram_mb = 768;
>  }

Re: [OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested test configuration

[Ian Campbell]

On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> 1. In this script, make some appropriate runvars which selecthost would
> recognise.
> 2. Prepare the configurations for installing L2 guest VM.
> 3. Create a lv disk in L0 and hot-attach it to L1, need to restart L1 to
> make the block disk to be recognized by L1 system, then using this disk
> to create a VG that used for installing L2.
> 
> Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> ---
> Changes in v7:
> Add new ts script for nested configuration according maintainer's reply.
> ---
>  ts-nested-setup |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100755 ts-nested-setup
> 
> diff --git a/ts-nested-setup b/ts-nested-setup
> new file mode 100755
> index 0000000..153c4a3
> --- /dev/null
> +++ b/ts-nested-setup
> @@ -0,0 +1,54 @@
> +#!/usr/bin/perl -w
> +# This is part of "osstest", an automated testing framework for Xen.
> +# Copyright (C) 2015 Intel Inc.
> +#
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU Affero General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU Affero General Public License for more details.
> +#
> +# You should have received a copy of the GNU Affero General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +use strict qw(vars);
> +use DBI;
> +use Osstest;
> +use Osstest::Debian;
> +use Osstest::TestSupport;
> +
> +tsreadconfig();
> +our ($ident_l0,$nested_host) = @ARGV;

You don't used $ident and I think all your uses of $nested_host can be
replaced by $l1->{Guest} using the result of the ts_get_host_guest.

> +our ($l0,$l1) = ts_get_host_guest(@ARGV);
> +
> +guest_check_ip($l1);
> +target_cmd_root($l1, "mkdir -p /home/osstest/.ssh && cp /root/.ssh/authorized_keys /home/osstest/.ssh/");
> +my $url = $c{WebspaceUrl}.$c{WebspaceCommon}.$l0->{Name}."_".'overlay.tar';
> +target_cmd_root($l1, <<END);
> +    wget -O overlay.tar $url
> +    tar -xf overlay.tar -C /
> +    rm overlay.tar -f
> +    update-rc.d osstest-confirm-booted start 99 2 .
> +END

I think the need for this goes away with
http://article.gmane.org/gmane.comp.emulators.xen.devel/224420
from my distro test flight series.

As it happens I was rebasing that series this morning but due to other
issues I've not managed to run it yet. Once I've managed to at least
smoke test I'll CC you on the repost.

> +store_runvar('nested_l1',$nested_host);
> +store_runvar("${nested_host}_ip",$l1->{Ip});
> +store_runvar('multi_reboot_time',5);

I think you said in a previous patch that this last one isn't needed any
more?

> +my $l2_disk_mb = 20000;
> +my $l2_lv_name = 'nestedl2';
> +my $vgname = $l0->{Name};
> +$vgname .= ".$c{TestHostDomain}" if ($l0->{Suite} =~ m/lenny/);
> +target_cmd_root($l0, <<END);
> +    lvremove -f /dev/${vgname}/${l2_lv_name} ||:
> +    lvcreate -L ${l2_disk_mb}M -n $l2_lv_name $vgname
> +    dd if=/dev/zero of=/dev/${vgname}/${l2_lv_name} count=10
> +    xl block-attach $l1->{Name} /dev/${vgname}/${l2_lv_name},raw,sdb,rw

This attach won't be persistent over domain destroy, is that expected?

> +END
> +target_install_packages_norec($l1, qw(lvm2 rsync genisoimage));
> +target_reboot($l1);
> +target_cmd_root($l1, "pvcreate /dev/xvdb && vgcreate ${l2_lv_name}_vg /dev/xvdb");

Ian.

Re: [OSSTEST Nested PATCH v7 2/6] Edit some testsupport APIs for nested test

[Ian Campbell]

On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> +    my $vif= guest_var($gho, 'vifmodel','');
> +    my $vifmodel= $vif ? "model=$vif" : '';;

Two ";;" at the end.

Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case

[Ian Campbell]

On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> 1. This patch adds creation of the nested test job, when job creation
> procedure is invoked.
> 2. 'NESTED_OS_IMAGE' is the name of 'Debian ISO Images', which defined
> in standalone.config.

It will need to be defined in production-config too, and it will need to
be made available on the infra, which probably involves you telling us
which ISO is needed.

Or even better, use the same value as the existing Debian test, i.e.
debian-7.2.0-amd64-CD-1.iso which is hardcoded in make-flight but would
be better off refactored into production-config

> 3. Set nested L1's vif model as e1000 by make-flight.
> 
> Signed-off-by: longtao.pang <longtaox.pang@intel.com>

I think this needs to go after the next patch, else the recipe doesn't
exist yet.

> ---
> Changes in v7:
> Set L1's vif model as e1000 in runvar by make-flight.
> ---
>  make-flight |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/make-flight b/make-flight
> index 8ac3a87..b8f266f 100755
> --- a/make-flight
> +++ b/make-flight
> @@ -204,6 +204,26 @@ do_hvm_win7_x64_tests () {
>              all_hostflags=$most_hostflags,hvm
>  }
>  
> +do_hvm_debian_nested_tests () {
> +  if [ $xenarch != amd64 ]; then
> +    return
> +  fi
> +  if [ $dom0arch != amd64 ]; then
> +    return
> +  fi

You can do these on a line each, or even combine into one test. i.e.

    if [ $xenarch != amd64 -o $dom0arch != amd64 ]; then return; fi

> +
> +  job_create_test test-$xenarch$kern-$dom0arch-nested test-nested xl \
> +			$xenarch $dom0arch \
> +            nested_image=$NESTED_OS_IMAGE \
> +            nested2_image=$NESTED_OS_IMAGE \

I think for clarity you should use something like nestedl1 and nestedl2
for the runvar names.

> +            bios=seabios \
> +            kernbuildjob=build-amd64-pvops \
> +            kernkind=pvops \
> +            nested_vifmodel='e1000' \
> +            device_model_version=qemu-xen \
> +            all_hostflags=$most_hostflags,hvm
> +}
> +
>  do_hvm_debian_test_one () {
>    testname=$1
>    bios=$2
> @@ -430,6 +450,7 @@ test_matrix_do_one () {
>      done
>  
>    fi
> +  do_hvm_debian_nested_tests
>    do_passthrough_tests
>  }
>  

Re: [OSSTEST Nested PATCH v7 1/6] parsing grub which has 'submenu' primitive

[Hu, Robert]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, March 31, 2015 9:44 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v7 1/6] parsing grub which has 'submenu'
> primitive
> 
> On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > From a hvm kernel build from Linux stable Kernel tree,
> > the auto generated grub2 menu will have 'submenu' primitive, upon the
> > 'menuentry' items. Xen boot entries will be grouped into a submenu. This
> > patch adds capability to support such grub formats.
> >
> > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> > ---
> > Changes in v7:
> > Remove the reformatting change for Debian.pm and keep the original format.
> 
> Thank you.
> 
> > ---
> >  Osstest/Debian.pm |   21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> > index 6784024..35163a0 100644
> > --- a/Osstest/Debian.pm
> > +++ b/Osstest/Debian.pm
> > @@ -398,10 +398,18 @@ sub setupboot_grub2 ($$$$) {
> >
> >          my $count= 0;
> >          my $entry;
> > +        my $submenu;
> >          while (<$f>) {
> >              next if m/^\s*\#/ || !m/\S/;
> >              if (m/^\s*\}\s*$/) {
> > -                die unless $entry;
> > +                die unless $entry || $submenu;
> > +                if(!defined $entry && defined $submenu){
> > +                    logm("Met end of a submenu starting from ".
> > +                        "$submenu->{StartLine}. ".
> > +                        "Our want kern is $want_kernver");
> > +                    $submenu=undef;
> > +                    next;
> > +                }
> >                  my (@missing) =
> >                      grep { !defined $entry->{$_} }
> >  		        (defined $xenhopt
> > @@ -432,21 +440,24 @@ sub setupboot_grub2 ($$$$) {
> >                  $entry= { Title => $1, StartLine => $., Number =>
> $count };
> >                  $count++;
> >              }
> > -            if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
> > +            if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> > +                $submenu={ StartLine =>$.};
> > +            }
> 
> This looks reasonable enough to support a single nesting, I suppose we
> can leave more deeply nested submenus for another time.
> 
> So in that regard this patch looks ok to me.
> 
> > +            if (m/^\s*multiboot\s*(?:\/boot)*\/(xen\S+)/) {
> >                  die unless $entry;
> >                  $entry->{Hv}= $1;
> >              }
> > -            if (m/^\s*multiboot\s*\/(vmlinu[xz]-(\S+))/) {
> > +            if (m/^\s*multiboot\s*(?:\/boot)*\/(vmlinu[xz]-(\S+))/) {
> 
> What are these changes all about? I think they must be unrelated to the
> use of submenu (perhaps relate to having a separate /boot or not?). If
> so then please do in a separate patch.
> 
You're right. This has nothing to do with submenu.
Going to separate it out in another patch.
> If this is somehow to do with submenu then please explain how/why in the
> commit log.
> 
> BTW, your regex as it stand will accept /boot/boot/boot/boot/vmlinuz. I
> think you maybe meant to add "(?:\/boot)?" to match zero or one
> occurrences?
Yes, this is a potential bug. Thanks for point out!
> 
> Ian.

Re: [OSSTEST Nested PATCH v7 2/6] Edit some testsupport APIs for nested test

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, March 31, 2015 9:50 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v7 2/6] Edit some testsupport APIs for
> nested test
> 
> On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > 1. Designate vif model to 'e1000' by make-flight.
> 
> Strictly you could s/to 'e1000'// here since the make-flight changes are
> elsewhere and that would better describe the generic change.
> 
Do you mean that I should change the description from "Designate vif model to 'e1000' by make-flight" to "Designate vif model by make-flight"?
> > 2. In L2 installation context, its host (L1) IP address is not queried
> > from DNS, but after running "ts-nested-setup + host + nested", L1 IP
> > is stored in runvar.
> >
> > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 

Re: [OSSTEST Nested PATCH v7 3/6] Changes on test step of debain hvm guest install

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, March 31, 2015 9:56 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v7 3/6] Changes on test step of debain
> hvm guest install
> 
> On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > 1. Increase disk size to accomodate to nested test requirment.
> 
> "accommodate" and "requirement"
> 
> Ian, are you OK with this generic change or would you prefer a runvar? I
> suspect the answer is going to be runvar.
> 
So, should I waiting for Ian Jackson's reply and then take next action?
> > 2. Since 'Debain-xxx-.iso' image will be stored there, therefore needs
> > more disk capacity, increase root partition size in preseed generation.
> 
> "Debian".
> 
> I'm not sure I follow what this one is saying though, does "there" refer to the
> rootfs of the VM which is being built here?
Yes.
> 
> > @@ -75,6 +75,7 @@ d-i preseed/late_command string \\
> >          in-target mkdir -p /boot/efi/EFI/boot; \\
> >          in-target cp /boot/efi/EFI/debian/grubx64.efi
> /boot/efi/EFI/boot/bootx64.efi ;\\
> >          in-target mkdir -p /root/.ssh; \\
> > +        in-target sed -i '/^deb *cdrom/s/^/#/' /etc/apt/sources.list;
> > + \\
> 
> I think a more conventional way to do this would be to use:
>         s/^deb *cdrom/#&/g
> 
> >          in-target sh -c "echo -e '$authkeys'>
> > /root/.ssh/authorized_keys";  END
> >      return $preseed_file;
> > @@ -152,6 +153,7 @@ sub prep () {
> >      more_prepareguest_hvm($ho,$gho, $ram_mb, $disk_mb,
> >                            OnReboot => 'preserve',
> >                            Bios => $r{bios},
> > +                          ExtraConfig => 'nestedhvm=1',
> 
> Please do only for the nested job i.e. using a runvar.
I remembered in previous mail you mentioned as below:
"One good thing about just enabling it from the start is that we get some testing of normal guest installation while nestedhvm happens to be installed, which might help catch regressions."
Therefore, I enabled 'nestedhvm' feature as generally. 
> 
> >                            PostImageHook => sub {
> >          my $cmds = iso_copy_content_from_image($gho, $newiso);
> >          $cmds .=
> > prepare_initrd($initrddir,$newiso,$preseed_file_path);
> > @@ -173,6 +175,8 @@ my $ram_minslop = 100;  my $ram_lots = 5000;  if
> > ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
> >      $ram_mb = $ram_lots;
> > +} elsif ($gn eq 'nested') {
> > +    $ram_mb = 3072;
> 
> I think this ought to be driven from a runvar as well, e.g. <gn>_minram.
> 
> >  } else {
> >      $ram_mb = 768;
> >  }
> 

Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, March 31, 2015 10:23 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case
> 
> On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > 1. This patch adds creation of the nested test job, when job creation
> > procedure is invoked.
> > 2. 'NESTED_OS_IMAGE' is the name of 'Debian ISO Images', which defined
> > in standalone.config.
> 
> It will need to be defined in production-config too, and it will need to be made
> available on the infra, which probably involves you telling us which ISO is
> needed.
> 
> Or even better, use the same value as the existing Debian test, i.e.
> debian-7.2.0-amd64-CD-1.iso which is hardcoded in make-flight but would be
> better off refactored into production-config
>
We create a 'standalone.config' and defined 'export NESTED_OS_IMAGE=debian-7.6.0-amd64-DVD-1.iso' in this config.
I'm sorry, what's 'production-config' used for? It seems that we have not used it before. Could you please make it more clearly?
> > 3. Set nested L1's vif model as e1000 by make-flight.
> >
> > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> 
> I think this needs to go after the next patch, else the recipe doesn't exist yet.
>
I'm sorry, I don't understand your meaning. Could you please make it more clearly? 
 
> > ---
> > Changes in v7:
> > Set L1's vif model as e1000 in runvar by make-flight.
> > ---
> >  make-flight |   21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/make-flight b/make-flight index 8ac3a87..b8f266f 100755
> > --- a/make-flight
> > +++ b/make-flight
> > @@ -204,6 +204,26 @@ do_hvm_win7_x64_tests () {
> >              all_hostflags=$most_hostflags,hvm  }
> >
> > +do_hvm_debian_nested_tests () {
> > +  if [ $xenarch != amd64 ]; then
> > +    return
> > +  fi
> > +  if [ $dom0arch != amd64 ]; then
> > +    return
> > +  fi
> 
> You can do these on a line each, or even combine into one test. i.e.
> 
>     if [ $xenarch != amd64 -o $dom0arch != amd64 ]; then return; fi
> 
> > +
> > +  job_create_test test-$xenarch$kern-$dom0arch-nested test-nested xl \
> > +			$xenarch $dom0arch \
> > +            nested_image=$NESTED_OS_IMAGE \
> > +            nested2_image=$NESTED_OS_IMAGE \
> 
> I think for clarity you should use something like nestedl1 and nestedl2 for the
> runvar names.
> 
> > +            bios=seabios \
> > +            kernbuildjob=build-amd64-pvops \
> > +            kernkind=pvops \
> > +            nested_vifmodel='e1000' \
> > +            device_model_version=qemu-xen \
> > +            all_hostflags=$most_hostflags,hvm }
> > +
> >  do_hvm_debian_test_one () {
> >    testname=$1
> >    bios=$2
> > @@ -430,6 +450,7 @@ test_matrix_do_one () {
> >      done
> >
> >    fi
> > +  do_hvm_debian_nested_tests
> >    do_passthrough_tests
> >  }
> >
> 

Re: [OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested test configuration

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, March 31, 2015 10:14 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested
> test configuration
> 
> On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > 1. In this script, make some appropriate runvars which selecthost
> > would recognise.
> > 2. Prepare the configurations for installing L2 guest VM.
> > 3. Create a lv disk in L0 and hot-attach it to L1, need to restart L1
> > to make the block disk to be recognized by L1 system, then using this
> > disk to create a VG that used for installing L2.
> >
> > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> > ---
> > Changes in v7:
> > Add new ts script for nested configuration according maintainer's reply.
> > ---
> >  ts-nested-setup |   54
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> >  create mode 100755 ts-nested-setup
> >
> > diff --git a/ts-nested-setup b/ts-nested-setup new file mode 100755
> > index 0000000..153c4a3
> > --- /dev/null
> > +++ b/ts-nested-setup
> > @@ -0,0 +1,54 @@
> > +#!/usr/bin/perl -w
> > +# This is part of "osstest", an automated testing framework for Xen.
> > +# Copyright (C) 2015 Intel Inc.
> > +#
> > +# This program is free software: you can redistribute it and/or
> > +modify # it under the terms of the GNU Affero General Public License
> > +as published by # the Free Software Foundation, either version 3 of
> > +the License, or # (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful, #
> > +but WITHOUT ANY WARRANTY; without even the implied warranty of #
> > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the #
> GNU
> > +Affero General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU Affero General Public
> > +License # along with this program.  If not, see
> <http://www.gnu.org/licenses/>.
> > +
> > +use strict qw(vars);
> > +use DBI;
> > +use Osstest;
> > +use Osstest::Debian;
> > +use Osstest::TestSupport;
> > +
> > +tsreadconfig();
> > +our ($ident_l0,$nested_host) = @ARGV;
> 
> You don't used $ident and I think all your uses of $nested_host can be replaced
> by $l1->{Guest} using the result of the ts_get_host_guest.
> 
> > +our ($l0,$l1) = ts_get_host_guest(@ARGV);
> > +
> > +guest_check_ip($l1);
> > +target_cmd_root($l1, "mkdir -p /home/osstest/.ssh && cp
> > +/root/.ssh/authorized_keys /home/osstest/.ssh/"); my $url =
> > +$c{WebspaceUrl}.$c{WebspaceCommon}.$l0->{Name}."_".'overlay.tar';
> > +target_cmd_root($l1, <<END);
> > +    wget -O overlay.tar $url
> > +    tar -xf overlay.tar -C /
> > +    rm overlay.tar -f
> > +    update-rc.d osstest-confirm-booted start 99 2 .
> > +END
> 
> I think the need for this goes away with
> http://article.gmane.org/gmane.comp.emulators.xen.devel/224420
> from my distro test flight series.
> 
> As it happens I was rebasing that series this morning but due to other issues
> I've not managed to run it yet. Once I've managed to at least smoke test I'll CC
> you on the repost.
> 
OK. What's more, for the below codes which is used for starting 'osstest-confirm-booted' script to confirm whether L1 is fully booted after reboot it. I think it's necessary here.
 +target_cmd_root($l1, <<END);
 +    wget -O overlay.tar $url
 +    tar -xf overlay.tar -C /
 +    rm overlay.tar -f
 +    update-rc.d osstest-confirm-booted start 99 2 .
+END
 
> > +store_runvar('nested_l1',$nested_host);
> > +store_runvar("${nested_host}_ip",$l1->{Ip});
> > +store_runvar('multi_reboot_time',5);
> 
> I think you said in a previous patch that this last one isn't needed any more?
> 
> > +my $l2_disk_mb = 20000;
> > +my $l2_lv_name = 'nestedl2';
> > +my $vgname = $l0->{Name};
> > +$vgname .= ".$c{TestHostDomain}" if ($l0->{Suite} =~ m/lenny/);
> > +target_cmd_root($l0, <<END);
> > +    lvremove -f /dev/${vgname}/${l2_lv_name} ||:
> > +    lvcreate -L ${l2_disk_mb}M -n $l2_lv_name $vgname
> > +    dd if=/dev/zero of=/dev/${vgname}/${l2_lv_name} count=10
> > +    xl block-attach $l1->{Name}
> > +/dev/${vgname}/${l2_lv_name},raw,sdb,rw
> 
> This attach won't be persistent over domain destroy, is that expected?
> 
We need create a logical volume in L0 and attach it to L1 for L2 VM installing. Before creating, it will remove the logical volume with the same "$l2_lv_name" that has already existed. 
Yes, after destroyed L1 VM, the logical volume will still exist inside L0, but there is no harm, I think.
> > +END
> > +target_install_packages_norec($l1, qw(lvm2 rsync genisoimage));
> > +target_reboot($l1); target_cmd_root($l1, "pvcreate /dev/xvdb &&
> > +vgcreate ${l2_lv_name}_vg /dev/xvdb");
> 
> Ian.

Re: [OSSTEST Nested PATCH v7 2/6] Edit some testsupport APIs for nested test

[Ian Campbell]

On Wed, 2015-04-01 at 05:56 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Tuesday, March 31, 2015 9:50 PM
> > To: Pang, LongtaoX
> > Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> > Hu, Robert
> > Subject: Re: [OSSTEST Nested PATCH v7 2/6] Edit some testsupport APIs for
> > nested test
> > 
> > On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > > 1. Designate vif model to 'e1000' by make-flight.
> > 
> > Strictly you could s/to 'e1000'// here since the make-flight changes are
> > elsewhere and that would better describe the generic change.
> > 
> Do you mean that I should change the description from "Designate vif
> model to 'e1000' by make-flight" to "Designate vif model by
> make-flight"?

Yes, or even better "Add runvar to allow override of default VIF model".

> > > 2. In L2 installation context, its host (L1) IP address is not queried
> > > from DNS, but after running "ts-nested-setup + host + nested", L1 IP
> > > is stored in runvar.
> > >
> > > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> 

Re: [OSSTEST Nested PATCH v7 3/6] Changes on test step of debain hvm guest install

[Ian Campbell]

On Wed, 2015-04-01 at 08:19 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Tuesday, March 31, 2015 9:56 PM
> > To: Pang, LongtaoX
> > Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> > Hu, Robert
> > Subject: Re: [OSSTEST Nested PATCH v7 3/6] Changes on test step of debain
> > hvm guest install
> > 
> > On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > > 1. Increase disk size to accomodate to nested test requirment.
> > 
> > "accommodate" and "requirement"
> > 
> > Ian, are you OK with this generic change or would you prefer a runvar? I
> > suspect the answer is going to be runvar.
> > 
> So, should I waiting for Ian Jackson's reply and then take next action?

Yes, or you could just make it a runvar because I'm sure Ian wouldn't
object to that.

> > > 2. Since 'Debain-xxx-.iso' image will be stored there, therefore needs
> > > more disk capacity, increase root partition size in preseed generation.
> > 
> > "Debian".
> > 
> > I'm not sure I follow what this one is saying though, does "there" refer to the
> > rootfs of the VM which is being built here?
> Yes.

"...image will be stored in the rootfs of the L1 guest, therefore
needs..." 

Would be cleaner.

> > > +                          ExtraConfig => 'nestedhvm=1',
> > 
> > Please do only for the nested job i.e. using a runvar.

> I remembered in previous mail you mentioned as below:

> "One good thing about just enabling it from the start is that we get
> some testing of normal guest installation while nestedhvm happens to
> be installed, which might help catch regressions."

> Therefore, I enabled 'nestedhvm' feature as generally. 

Sorry, I was ambiguous there, I meant from the start of the nestedhvm
related jobs only, not for every job. I should have said "some testing
of normal guest installation while we install the L1 hypervisor VM" or
something along those lines.

Ian.

Re: [OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested test configuration

[Ian Campbell]

On Wed, 2015-04-01 at 08:45 +0000, Pang, LongtaoX wrote:
> > As it happens I was rebasing that series this morning but due to other issues
> > I've not managed to run it yet. Once I've managed to at least smoke test I'll CC
> > you on the repost.
> > 
> OK. What's more, for the below codes which is used for starting
> 'osstest-confirm-booted' script to confirm whether L1 is fully booted
> after reboot it. I think it's necessary here.

>  +target_cmd_root($l1, <<END);
>  +    wget -O overlay.tar $url
>  +    tar -xf overlay.tar -C /
>  +    rm overlay.tar -f
>  +    update-rc.d osstest-confirm-booted start 99 2 .
> +END

In my distro series I also have some patches refactoring the overlay
stuff, which would mean you could reuse that.
http://article.gmane.org/gmane.comp.emulators.xen.devel/224433
I'll CC you on that one too.

I don't think there would be any harm in adding those overlays for all
guests and enabling the initscript, but Ian may disagree or know
something which I don't.


> > > +my $l2_disk_mb = 20000;
> > > +my $l2_lv_name = 'nestedl2';
> > > +my $vgname = $l0->{Name};
> > > +$vgname .= ".$c{TestHostDomain}" if ($l0->{Suite} =~ m/lenny/);
> > > +target_cmd_root($l0, <<END);
> > > +    lvremove -f /dev/${vgname}/${l2_lv_name} ||:
> > > +    lvcreate -L ${l2_disk_mb}M -n $l2_lv_name $vgname
> > > +    dd if=/dev/zero of=/dev/${vgname}/${l2_lv_name} count=10
> > > +    xl block-attach $l1->{Name}
> > > +/dev/${vgname}/${l2_lv_name},raw,sdb,rw
> > 
> > This attach won't be persistent over domain destroy, is that expected?
> > 
> We need create a logical volume in L0 and attach it to L1 for L2 VM
> installing. Before creating, it will remove the logical volume with
> the same "$l2_lv_name" that has already existed. 
> Yes, after destroyed L1 VM, the logical volume will still exist inside
> L0, but there is no harm, I think.

OK.

Ian./

Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case

[Ian Campbell]

On Wed, 2015-04-01 at 08:27 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Tuesday, March 31, 2015 10:23 PM
> > To: Pang, LongtaoX
> > Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> > Hu, Robert
> > Subject: Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case
> > 
> > On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > > 1. This patch adds creation of the nested test job, when job creation
> > > procedure is invoked.
> > > 2. 'NESTED_OS_IMAGE' is the name of 'Debian ISO Images', which defined
> > > in standalone.config.
> > 
> > It will need to be defined in production-config too, and it will need to be made
> > available on the infra, which probably involves you telling us which ISO is
> > needed.
> > 
> > Or even better, use the same value as the existing Debian test, i.e.
> > debian-7.2.0-amd64-CD-1.iso which is hardcoded in make-flight but would be
> > better off refactored into production-config
> >
> We create a 'standalone.config' and defined 'export NESTED_OS_IMAGE=debian-7.6.0-amd64-DVD-1.iso' in this config.
> I'm sorry, what's 'production-config' used for? It seems that we have not used it before. Could you please make it more clearly?

standalone.config is only used in standalone mode, in production mode
(i.e. on the proper infrastructure and running from cron)
production-config is used instead.

In standalone mode the equivalent to production-config is, by default,
~/.xen-osstest/config or you can set it with the -c option to the
standalone helper script.

Those options area accessed from Perl with $c{Name} and from shell with
`getconfig Name`.

I think you could probably get away with just hardcoding
debian-7.2.0-amd64-CD-1.iso in make-flight.

Note the version I said there is different to the one you used but is
the same as the other instance in make-flight. I think they should be
the same so if you really need 7.6.0 DVD for some reason then I think we
should make everything use that, but I hope that isn't necessary.

Ideally all of the runvars which are assigned an iso image would use the
config, but I don't think you need to be the one to do that.

> > > 3. Set nested L1's vif model as e1000 by make-flight.
> > >
> > > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> > 
> > I think this needs to go after the next patch, else the recipe doesn't exist yet.
> >
> I'm sorry, I don't understand your meaning. Could you please make it more clearly? 

Here you add a job which uses test-nested as the recipe, but you don't
patch sg-run-job to define that recipe until the next patch, so you
should swap the order of the two patches.

Ian.

Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, March 31, 2015 10:23 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case
> 
> On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > Changes in v7:
> > diff --git a/make-flight b/make-flight index 8ac3a87..b8f266f 100755
> > --- a/make-flight
> > +++ b/make-flight
> > @@ -204,6 +204,26 @@ do_hvm_win7_x64_tests () {
> >              all_hostflags=$most_hostflags,hvm  }
> >
> > +do_hvm_debian_nested_tests () {
> > +  if [ $xenarch != amd64 ]; then
> > +    return
> > +  fi
> > +  if [ $dom0arch != amd64 ]; then
> > +    return
> > +  fi
> 
> You can do these on a line each, or even combine into one test. i.e.
> 
>     if [ $xenarch != amd64 -o $dom0arch != amd64 ]; then return; fi
> 
I'm sorry I find that the 'if' condition is not appropriate in v7 patch, it should be 
if [ $xenarch != amd64 -a $dom0arch != amd64 ]; then return; fi
> > +
> > +  job_create_test test-$xenarch$kern-$dom0arch-nested test-nested xl \
> > +			$xenarch $dom0arch \
> > +            nested_image=$NESTED_OS_IMAGE \
> > +            nested2_image=$NESTED_OS_IMAGE \
> 
> I think for clarity you should use something like nestedl1 and nestedl2 for the
> runvar names.
> 
'nested' and 'nested2' are guest name of L1 and L2 guest VM. Since "$specimage" is accessed from "$r{"$gho->{Guest}_image"}" which defined in the function of ' target_put_guest_image'. So, maybe 'nested' and 'nested2' are available here, I think. 
> > +            bios=seabios \
> > +            kernbuildjob=build-amd64-pvops \
> > +            kernkind=pvops \
> > +            nested_vifmodel='e1000' \
> > +            device_model_version=qemu-xen \
> > +            all_hostflags=$most_hostflags,hvm }
> > +

Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case

[Ian Campbell]

On Thu, 2015-04-02 at 08:16 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Tuesday, March 31, 2015 10:23 PM
> > To: Pang, LongtaoX
> > Cc: xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com;
> > Hu, Robert
> > Subject: Re: [OSSTEST Nested PATCH v7 5/6] Add test job for nest test case
> > 
> > On Fri, 2015-03-27 at 19:06 -0400, longtao.pang wrote:
> > > Changes in v7:
> > > diff --git a/make-flight b/make-flight index 8ac3a87..b8f266f 100755
> > > --- a/make-flight
> > > +++ b/make-flight
> > > @@ -204,6 +204,26 @@ do_hvm_win7_x64_tests () {
> > >              all_hostflags=$most_hostflags,hvm  }
> > >
> > > +do_hvm_debian_nested_tests () {
> > > +  if [ $xenarch != amd64 ]; then
> > > +    return
> > > +  fi
> > > +  if [ $dom0arch != amd64 ]; then
> > > +    return
> > > +  fi
> > 
> > You can do these on a line each, or even combine into one test. i.e.
> > 
> >     if [ $xenarch != amd64 -o $dom0arch != amd64 ]; then return; fi
> > 
> I'm sorry I find that the 'if' condition is not appropriate in v7 patch, it should be 
> if [ $xenarch != amd64 -a $dom0arch != amd64 ]; then return; fi

Yes, I think you are right.

> > > +
> > > +  job_create_test test-$xenarch$kern-$dom0arch-nested test-nested xl \
> > > +			$xenarch $dom0arch \
> > > +            nested_image=$NESTED_OS_IMAGE \
> > > +            nested2_image=$NESTED_OS_IMAGE \
> > 
> > I think for clarity you should use something like nestedl1 and nestedl2 for the
> > runvar names.
> > 

> 'nested' and 'nested2' are guest name of L1 and L2 guest VM. Since
> "$specimage" is accessed from "$r{"$gho->{Guest}_image"}" which
> defined in the function of ' target_put_guest_image'. So, maybe
> 'nested' and 'nested2' are available here, I think. 

My point was you should use the clearer names throughout, from point of
definition onwards and in the sg-run-job spec for the recipe too.

Ian.

Re: [OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested test configuration

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Wednesday, April 01, 2015 4:59 PM
> To: Pang, LongtaoX; Ian.Jackson@eu.citrix.com
> Cc: xen-devel@lists.xen.org; wei.liu2@citrix.com; Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested
> test configuration
> 
> On Wed, 2015-04-01 at 08:45 +0000, Pang, LongtaoX wrote:
> > > As it happens I was rebasing that series this morning but due to
> > > other issues I've not managed to run it yet. Once I've managed to at
> > > least smoke test I'll CC you on the repost.
> > >
> > OK. What's more, for the below codes which is used for starting
> > 'osstest-confirm-booted' script to confirm whether L1 is fully booted
> > after reboot it. I think it's necessary here.
> 
> >  +target_cmd_root($l1, <<END);
> >  +    wget -O overlay.tar $url
> >  +    tar -xf overlay.tar -C /
> >  +    rm overlay.tar -f
> >  +    update-rc.d osstest-confirm-booted start 99 2 .
> > +END
> 
> In my distro series I also have some patches refactoring the overlay stuff, which
> would mean you could reuse that.
> http://article.gmane.org/gmane.comp.emulators.xen.devel/224433
> I'll CC you on that one too.
> 
> I don't think there would be any harm in adding those overlays for all guests
> and enabling the initscript, but Ian may disagree or know something which I
> don't.
> 
I have modified and updated the v7 patches that according to your reply. It seems that your patchs[v4 04,05,06] has not been pushed into OSSTest master tree, should I 
waiting for that till these patches pushed or release my v8 nested patches to you firstly? Since we prepared this for a long time.

Re: [OSSTEST Nested PATCH v7 4/6] Add new script to custmize nested test configuration

[Ian Campbell]

On Thu, 2015-04-09 at 07:12 +0000, Pang, LongtaoX wrote:

> I have modified and updated the v7 patches that according to your
> reply. It seems that your patchs[v4 04,05,06] has not been pushed into
> OSSTest master tree, should I waiting for that till these patches
> pushed or release my v8 nested patches to you firstly? Since we
> prepared this for a long time.

FWIW I think the advice in
http://lists.xen.org/archives/html/xen-devel/2015-04/msg01418.html
(which I happened to just write today for other reasons) would apply
here too.

IOW it would be fine to import my patches (or a useful subset) into your
local tree and post your resulting patches building on top, so long as
you explain in the 00/NN intro mail what the prerequisites are.

Ian.