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

[OSSTEST Nested PATCH 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 'nested' input param to differentiate L1 Xen installation 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.
----------------------------------------------------------------
longtao.pang (6):
      parsing grub which has 'submenu' primitive
      Add and expose some testsupport APIs
      Changes on test step of debain hvm guest install in hvm.
      Add new ts-nested-setup script to custmize nested test configuration before     the testing.
      Add test job for nest test case
      Compose the main recipe of test-nested test job.

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

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

[longtao.pang]

From: "longtao.pang" <longtaox.pang@intel.com>

>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.
---
 Osstest/Debian.pm |   52 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index e3e1c90..9fdacd7 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -1,5 +1,6 @@
 # This is part of "osstest", an automated testing framework for Xen.
 # Copyright (C) 2009-2013 Citrix Inc.
+# Copyright (C) 2014-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
@@ -362,26 +363,34 @@ 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
-			 ? qw(Title Hv KernDom0 KernVer)
-			 : qw(Title Hv KernOnly KernVer));
-		if (@missing) {
-		    logm("(skipping entry at $entry->{StartLine};".
-			 " no @missing)");
-		} elsif (defined $want_kernver &&
-			 $entry->{KernVer} ne $want_kernver) {
-		    logm("(skipping entry at $entry->{StartLine};".
-			 " kernel $entry->{KernVer}, not $want_kernver)");
-		} else {
-		    # yes!
-		    last;
-		}
+                    grep { !defined $entry->{$_} }
+                        (defined $xenhopt
+                         ? qw(Title Hv KernDom0 KernVer)
+                         : qw(Title Hv KernOnly KernVer));
+                if (@missing) {
+                    logm("(skipping entry at $entry->{StartLine};".
+                         " no @missing)");
+                } elsif (defined $want_kernver &&
+                         $entry->{KernVer} ne $want_kernver) {
+                    logm("(skipping entry at $entry->{StartLine};".
+                         " kernel $entry->{KernVer}, not $want_kernver)");
+                } else {
+                    # yes!
+                    last;
+                }
                 $entry= undef;
                 next;
             }
@@ -393,21 +402,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;
             }
         }
-- 
1.7.10.4

[OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[longtao.pang]

From: "longtao.pang" <longtaox.pang@intel.com>

1. Designate vif model to 'e1000', otherwise, with default device model,
the L1 eth0 interface disappear, hence xenbridge cannot work.
Maybe this limitation can be removed later after some fix it. For now, we
have to accomodate to it.
2. Since reboot L1 guest VM will take more time to boot up, we increase
multi-times for reboot-confirm-booted if test nested job, and the multi value 
is stored as a runvar in 'ts-nested-setup' script. Added another function 
'guest_editconfig_cd' and expose it, this function bascically changes guest
boot device sequence, alter its on_reboot behavior to restart and enabled 
nestedhvm feature.
3. 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.
---
 Osstest/TestSupport.pm |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index e1ebcb7..57c7167 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -111,6 +111,7 @@ BEGIN {
                       iso_gen_flags_basic
                       iso_copy_content_from_image
                       guest_editconfig_nocd
+                      guest_editconfig_cd
                       );
     %EXPORT_TAGS = ( );
 
@@ -819,6 +820,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].*)$/;
@@ -990,8 +995,9 @@ sub common_toolstack ($) {
 
 sub host_reboot ($) {
     my ($ho) = @_;
+    my $wait_time = $r{multi_reboot_time} ? $r{multi_reboot_time}*40 : 40;
     target_reboot($ho);
-    poll_loop(40,2, 'reboot-confirm-booted', sub {
+    poll_loop($wait_time,2, 'reboot-confirm-booted', sub {
         my $output;
         if (!eval {
             $output= target_cmd_output($ho, <<END, 40);
@@ -1541,7 +1547,7 @@ sub prepareguest_part_xencfg ($$$$$) {
     my $xencfg= <<END;
 name        = '$gho->{Name}'
 memory = ${ram_mb}
-vif         = [ 'type=ioemu,mac=$gho->{Ether}' ]
+vif         = [ 'type=ioemu,model=e1000,mac=$gho->{Ether}' ]
 #
 on_poweroff = '$onpoweroff'
 on_reboot   = '$onreboot'
@@ -2113,4 +2119,15 @@ sub guest_editconfig_nocd ($$) {
     });
 }
 
+sub guest_editconfig_cd ($) {
+    my ($gho) = @_;
+    guest_editconfig($gho->{Host}, $gho, sub {
+        if (m/^\s*boot\s*= '\s*d\s*c\s*'/) {
+            s/dc/cd/;
+        }
+        s/^on_reboot.*/on_reboot='restart'/;
+        s/#nestedhvm/nestedhvm/;
+    });
+}
+
 1;
-- 
1.7.10.4

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

[longtao.pang]

From: "longtao.pang" <longtaox.pang@intel.com>

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. In hvm guest configuration file, add '#nestedhvm=1',
which will later be uncommented by guest_editconfig_cd() after
xen installed in L1, and about to boot into a nested xen environment.
---
 ts-debian-hvm-install |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 449b96c..f334e30 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{ / } \\
@@ -152,6 +152,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 +174,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;
 }
@@ -189,7 +192,12 @@ if ($stage<2) {
     guest_destroy($gho);
 }
 
-guest_editconfig_nocd($gho,$emptyiso);
+if ($gn eq 'nested') {
+    guest_editconfig_cd($gho);
+} else {
+    guest_editconfig_nocd($gho,$emptyiso);
+}
+
 guest_create($gho);
 guest_await_dhcp_tcp($gho,300);
 guest_check_up($gho);
-- 
1.7.10.4

[OSSTEST Nested PATCH 4/6] Add new ts-nested-setup script to custmize nested test configuration before the testing.

[longtao.pang]

From: "longtao.pang" <longtaox.pang@intel.com>

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.
---
 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..69dc407
--- /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 5/6] Add test job for nest test case

[longtao.pang]

From: "longtao.pang" <longtaox.pang@intel.com>

This patch adds creation of the nested test job, when job creation procedure
is invoked.
'NESTED_OS_IMAGE' is the name of 'Debian ISO Images', which defined in
 standalone.config.
---
 make-flight |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/make-flight b/make-flight
index 63b14f5..fb35ac4 100755
--- a/make-flight
+++ b/make-flight
@@ -204,6 +204,25 @@ 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 \
+            device_model_version=qemu-xen \
+            all_hostflags=$most_hostflags,hvm
+}
+
 do_hvm_debian_test_one () {
   testname=$1
   bios=$2
@@ -401,6 +420,7 @@ test_matrix_do_one () {
     done
 
   fi
+  do_hvm_debian_nested_tests
   do_passthrough_tests
 }
 
-- 
1.7.10.4

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

[longtao.pang]

From: "longtao.pang" <longtaox.pang@intel.com>

---
 sg-run-job |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sg-run-job b/sg-run-job
index 94d091b..ababebe 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 1/6] parsing grub which has 'submenu' primitive

[Ian Campbell]

On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> From: "longtao.pang" <longtaox.pang@intel.com>
> 
> 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.

This is missing a Signed-off-by per
http://wiki.xen.org/wiki/Submitting_Xen_Patches#Signing_off_a_patch

I think this also means that the current patching of the grub.cfg which
Wei added for the host level stuff can be reverted.

> ---
>  Osstest/Debian.pm |   52 ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index e3e1c90..9fdacd7 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -1,5 +1,6 @@
>  # This is part of "osstest", an automated testing framework for Xen.
>  # Copyright (C) 2009-2013 Citrix Inc.
> +# Copyright (C) 2014-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
> @@ -362,26 +363,34 @@ 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
> -			 ? qw(Title Hv KernDom0 KernVer)
> -			 : qw(Title Hv KernOnly KernVer));
> -		if (@missing) {
> -		    logm("(skipping entry at $entry->{StartLine};".
> -			 " no @missing)");
> -		} elsif (defined $want_kernver &&
> -			 $entry->{KernVer} ne $want_kernver) {
> -		    logm("(skipping entry at $entry->{StartLine};".
> -			 " kernel $entry->{KernVer}, not $want_kernver)");
> -		} else {
> -		    # yes!
> -		    last;
> -		}
> +                    grep { !defined $entry->{$_} }
> +                        (defined $xenhopt
> +                         ? qw(Title Hv KernDom0 KernVer)
> +                         : qw(Title Hv KernOnly KernVer));
> +                if (@missing) {
> +                    logm("(skipping entry at $entry->{StartLine};".
> +                         " no @missing)");
> +                } elsif (defined $want_kernver &&
> +                         $entry->{KernVer} ne $want_kernver) {
> +                    logm("(skipping entry at $entry->{StartLine};".
> +                         " kernel $entry->{KernVer}, not $want_kernver)");
> +                } else {
> +                    # yes!
> +                    last;
> +                }

Please avoid unnecessary reindentation or code reformatting mixed with
functional changes. Otherwise you just obscure the actual changes you
are making. If something is wrongly indented or formatted then please
fix in another patch first.

As it is I'm afraid this patch is basically unreviewable.

>                  $entry= undef;
>                  next;
>              }
> @@ -393,21 +402,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;
>              }
>          }

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> From: "longtao.pang" <longtaox.pang@intel.com>
> 
> 1. Designate vif model to 'e1000', otherwise, with default device model,
> the L1 eth0 interface disappear, hence xenbridge cannot work.
> Maybe this limitation can be removed later after some fix it. For now, we
> have to accomodate to it.

You have done this unconditionally, which means it affects all guests.
You need to make this configurable by the caller, probably by plumbing
it through in $xopts (a hash of extra options).

I see now you were told this last time around by Ian J, please don't
just resend such things without change either fix them, make an argument
for doing it your way or ask for clarification if you don't understand
the requested change.

> 2. Since reboot L1 guest VM will take more time to boot up, we increase
> multi-times for reboot-confirm-booted if test nested job, and the multi value 
> is stored as a runvar in 'ts-nested-setup' script. Added another function 
> 'guest_editconfig_cd' and expose it, this function bascically changes guest
> boot device sequence, alter its on_reboot behavior to restart and enabled 
> nestedhvm feature.

This looks like two items run together?

The multi_reboot_time thing sounds ok, but it should be called
reboot_time_factor or something like that. In fact I see that Ian
suggested previously that it should have the host ident in it, that
makes sense to me.

The editconfig_cd thing -- yet another thing which Ian questioned and
which it was agreed you would change but you haven't.

I think perhaps you have accidentally resent an older version of the
series. If not then please go back and ensure you have addressed all of
the feedback given on the last iteration before sending another version.

Ian.

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

[Ian Campbell]

On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> This patch set adds nested HVM test case for osstest.

I've now looked at the first two patches in this series and I've found
that in both patches you have consistently not reacted to the review
comments made the first time around, so I'm not going to read the rest
of the series now since it would appear to be a waste of my time if I'm
just going to end up repeating things which were said last time.

Please resend a 3rd version when you have made sure that you have
addressed the previous feedback.

Please also:

      * Use "git send-email --reroll-count=N" (or with older git
        --subject-prefix including vN) to indicate which revision of the
        patch series this is (i.e. here you should have used
        --reroll-count=2 and next time 3)
      * Add a miniture changelog to each patch indication what has
        changed in this iteration, this should go after the commit
        message, S-o-b and a "---" marker.

See the "[PATCH v2] foobar: Add a new trondle calls" example in
http://wiki.xen.org/wiki/Submitting_Xen_Patches#Review.2C_Rinse_.26_Repeat for example of both of these.

In fact please read wiki.xen.org/wiki/Submitting_Xen_Patches for lots of
hints on all of this stuff.

Ian.

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

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Friday, March 20, 2015 12:32 AM
> 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 0/6] Introduction of netsted HVM test job
> 
> On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > This patch set adds nested HVM test case for osstest.
> 
> I've now looked at the first two patches in this series and I've found that in both
> patches you have consistently not reacted to the review comments made the
> first time around, so I'm not going to read the rest of the series now since it
> would appear to be a waste of my time if I'm just going to end up repeating
> things which were said last time.
> 
I am sorry that this version still does not reach the requirement and take your time to review it.
Actually, this version of patches change a lot according Ian Jackson's last useful comments.
> Please resend a 3rd version when you have made sure that you have addressed
> the previous feedback.
> 
Actually, this is the sixth version, the next version will be seventh, I will mark the version number of patches next time.
> Please also:
> 
>       * Use "git send-email --reroll-count=N" (or with older git
>         --subject-prefix including vN) to indicate which revision of the
>         patch series this is (i.e. here you should have used
>         --reroll-count=2 and next time 3)
>       * Add a miniture changelog to each patch indication what has
>         changed in this iteration, this should go after the commit
>         message, S-o-b and a "---" marker.
> 
> See the "[PATCH v2] foobar: Add a new trondle calls" example in
> http://wiki.xen.org/wiki/Submitting_Xen_Patches#Review.2C_Rinse_.26_Repe
> at for example of both of these.
> 
> In fact please read wiki.xen.org/wiki/Submitting_Xen_Patches for lots of hints
> on all of this stuff.
> 
This suggestion is really very useful to me, because I am not familiar with that.
> Ian.
> 

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

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Friday, March 20, 2015 12:17 AM
> 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 1/6] parsing grub which has 'submenu'
> primitive
> 
> On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > From: "longtao.pang" <longtaox.pang@intel.com>
> >
> > 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.
> 
> This is missing a Signed-off-by per
> http://wiki.xen.org/wiki/Submitting_Xen_Patches#Signing_off_a_patch
> 
I will add it next time.
> I think this also means that the current patching of the grub.cfg which Wei
> added for the host level stuff can be reverted.
> 
> > ---
> >  Osstest/Debian.pm |   52
> ++++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 32 insertions(+), 20 deletions(-)
> >
> > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm index
> > e3e1c90..9fdacd7 100644
> > --- a/Osstest/Debian.pm
> > +++ b/Osstest/Debian.pm
> > @@ -1,5 +1,6 @@
> >  # This is part of "osstest", an automated testing framework for Xen.
> >  # Copyright (C) 2009-2013 Citrix Inc.
> > +# Copyright (C) 2014-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 @@ -362,26 +363,34 @@ 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
> > -			 ? qw(Title Hv KernDom0 KernVer)
> > -			 : qw(Title Hv KernOnly KernVer));
> > -		if (@missing) {
> > -		    logm("(skipping entry at $entry->{StartLine};".
> > -			 " no @missing)");
> > -		} elsif (defined $want_kernver &&
> > -			 $entry->{KernVer} ne $want_kernver) {
> > -		    logm("(skipping entry at $entry->{StartLine};".
> > -			 " kernel $entry->{KernVer}, not $want_kernver)");
> > -		} else {
> > -		    # yes!
> > -		    last;
> > -		}
> > +                    grep { !defined $entry->{$_} }
> > +                        (defined $xenhopt
> > +                         ? qw(Title Hv KernDom0 KernVer)
> > +                         : qw(Title Hv KernOnly KernVer));
> > +                if (@missing) {
> > +                    logm("(skipping entry at $entry->{StartLine};".
> > +                         " no @missing)");
> > +                } elsif (defined $want_kernver &&
> > +                         $entry->{KernVer} ne $want_kernver) {
> > +                    logm("(skipping entry at $entry->{StartLine};".
> > +                         " kernel $entry->{KernVer}, not
> $want_kernver)");
> > +                } else {
> > +                    # yes!
> > +                    last;
> > +                }
> 
> Please avoid unnecessary reindentation or code reformatting mixed with
> functional changes. Otherwise you just obscure the actual changes you are
> making. If something is wrongly indented or formatted then please fix in
> another patch first.
> 
> As it is I'm afraid this patch is basically unreviewable.
I will keep the original coding format next time.
> 
> >                  $entry= undef;
> >                  next;
> >              }
> > @@ -393,21 +402,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;
> >              }
> >          }
> 

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs"):
> On Tue, 2015-03-24 at 03:34 +0000, Hu, Robert wrote:
> > > Make it an option to the function which creates the configuration in the
> > > first place. Quoting myself from earlier in this very thread:
> 
> > Then we still need to designate to use e1000 device somewhere; say, in
> > ts-nested-setup, we store this designation in runvar.
> > Then when creating the config, according to your proposal, we plumbing
> > such designation through $xopts, since we see it in runvar. Are you OK
> > with such implementation?
> 
> I'm not sure if this should be a runvar or just something which
> ts-nested-setup knows. That's one for Ian J to decide.

I think this should be in a runvar named after the guest, and should
be set by make-flight.  In principle that would allow us to have jobs
with and without this setting, for example.

I think the needed moving parts for this are:
 * something in prepareguest_part_xencfg which looks up
   the runvar (eg guest_var($gho,'vifmodel') and if not undef
   adds something appropriate to the vif setting
 * corresponding setting in the new jobs in make-flight/mfi-common

Thanks,
Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Tue, 2015-03-24 at 03:34 +0000, Hu, Robert wrote:

> > Make it an option to the function which creates the configuration in the
> > first place. Quoting myself from earlier in this very thread:

> Then we still need to designate to use e1000 device somewhere; say, in
> ts-nested-setup, we store this designation in runvar.
> Then when creating the config, according to your proposal, we plumbing
> such designation through $xopts, since we see it in runvar. Are you OK
> with such implementation?

I'm not sure if this should be a runvar or just something which
ts-nested-setup knows. That's one for Ian J to decide.

> But if like above, I would prefer to altering/manipulating guest configation 
> directly in ts-nested-setup after normal guest is setup. Since this is more
> simple and constrained to nested case only.

IMHO, this should be properly integrated into the configuration
creation, not fixed up later as an adhoc step.

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Tue, 2015-03-24 at 03:25 +0000, Hu, Robert wrote:

> Though no harm as far as I see now, I cannot forcast any harm in the
> future. Anyway, this 'nestedhvm' is for a nested usage. Better to enable
> it when we are actually going enable a nested environment.
> Practically, we can do it in either way. and the first is actually easier to do.

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.

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Tue, 2015-03-24 at 05:13 +0000, Pang, LongtaoX wrote:
> So, maybe I should write some code in 'ts-nested-setup' script to
> implement ssh into L1, edit sources.list and comment out the CDROM
> repo entry.

Please do it in ts-debian-hvm-install, either as a postinst step or,
better (IMHO), by arranging for it to happen during preseeding using the
late_command infrastructure.

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Wei Liu]

On Tue, Mar 24, 2015 at 05:13:31AM +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Tuesday, March 24, 2015 1:37 AM
> > To: Wei Liu
> > Cc: Pang, LongtaoX; Hu, Robert; Ian.Jackson@eu.citrix.com;
> > xen-devel@lists.xen.org
> > Subject: Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some
> > testsupport APIs
> > 
> > On Mon, 2015-03-23 at 17:29 +0000, Wei Liu wrote:
> > > On Mon, Mar 23, 2015 at 04:45:55PM +0000, Ian Campbell wrote:
> > > > On Mon, 2015-03-23 at 16:20 +0000, Pang, LongtaoX wrote:
> > > > > >
> > > > > > > > > The editconfig_cd thing -- yet another thing which Ian
> > > > > > > > > questioned and which it was agreed you would change but you
> > haven't.
> > > > > > > > >
> > > > > > > > For this question, I have sent a mail about it.(2015-03-04)
> > > > > > > > After finishing L1 guest VM installation, we need to change
> > > > > > > > L1 guest boot sequence from ISO image to hard disk, we need
> > > > > > > > modify the "boot=cd" ,
> > > > > > >
> > > > > > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > > > > > sufficient? It removes the CD from the virtual drive, meaning
> > > > > > > that "boot=dc" will fail to boot from d and fallthru to c.
> > > > > > >
> > > > > > > >  also need to enable 'nestedhvm' feature in hvm configure
> > > > > > > > file,
> > > > > > >
> > > > > > > This certainly doesn't belong in a function called
> > > > > > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > > > > > >
> > > > > > > Anyway, it's not clear why you need to edit this into the
> > > > > > > nestedhvm configuration, instead of adding it when the
> > > > > > > configuration is created via more_prepareguest_hvm. What harm
> > > > > > > is there in enabling this during guest install?
> > > > > > >
> > > > > > I will try it.
> > > > > >
> > > > > Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it
> > > > > could boot into L1 OS, but failed to install packages( such as
> > > > > lvm2, rsync, bridge-utils ....) via Debian repo in L1, as below msg:
> > > >
> > > > Oh dear. Things really ought to be tailored on install to use the
> > > > network repositories for the apt sources, not the cdrom.
> > >
> > > When I wrote ts-debian-hvm-install, one of the problems (if I remember
> > > correctly) was that our network infrastructure didn't support booting
> > > EFI from PXE boot. I ended up making that disk image to sort of work
> > > around this.
> > >
> > > >
> > > > Installing from netboot rather than netinst media ought to achieve
> > > > that, I'm not sure with ts-debian-hvm-install uses though or how to
> > > > achieve it via preseeding if it isn't the default for the given media.
> > > >
> > >
> > > Per  https://www.debian.org/releases/stable/example-preseed.txt,
> > > these runes look interesting.
> > >
> > > # Additional repositories, local[0-9] available #d-i
> > > apt-setup/local0/repository string \
> > > #       http://local.server/debian stable main
> > > #d-i apt-setup/local0/comment string local server # Enable deb-src
> > > lines #d-i apt-setup/local0/source boolean true # URL to the public
> > > key of the local repository; you must provide a key # or # apt will
> > > complain about the unauthenticated repository and so the #
> > > sources.list line will be left commented out #d-i apt-setup/local0/key
> > > string http://local.server/key
> > >
> > > Not sure if they will really end up in source.list though.
> > 
> > My expectation is that the existing preseed will have resulted in both http and
> > cdrom entries, and all that is needed is to comment out the cdrom ones so the
> > network ones take precedence.
> > 
> > Lets wait for an answer to my question about what is in sources.list on these
> > VMs before speculating further on how to fix this though.
> > 
> > Ian.
> I have checked the sources.list file in L1 guest, it contains both CDROM repo entry and URL entry(Debian repository mirror location), 
> Such as below:
> deb cdrom:[Debian GNU/Linux 7.6.0 _Wheezy_ - Official amd64 DVD Binary-1 20140712-14:11]/ wheezy contrib main
> deb http://linux-ftp.sh.intel.com//pub/mirrors/debian wheezy main
> deb-src http://linux-ftp.sh.intel.com//pub/mirrors/debian wheezy main
> 
> It seems that CDROM repo entry take effect, but it definitely unavailable, because ISO image is removed.
> If I comment out the CDROM repo entry manually, and then try to 'apt-get install', it works fine.
> For wei's first solution that change boot sequence from cd_disc to HDD, it does works and I have created a 'guest_ediconfig_nocd' function about that in previously patchs, maybe it's not preferred according to Ian Campbell's opinion.

FWIW I think Ian's approach is better. Using same source to install
packages is better than using different sources.

> So, maybe I should write some code in 'ts-nested-setup' script to implement ssh into L1, edit sources.list and comment out the CDROM repo entry. Or, do your have some easy ways?

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, March 24, 2015 1:37 AM
> To: Wei Liu
> Cc: Pang, LongtaoX; Hu, Robert; Ian.Jackson@eu.citrix.com;
> xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some
> testsupport APIs
> 
> On Mon, 2015-03-23 at 17:29 +0000, Wei Liu wrote:
> > On Mon, Mar 23, 2015 at 04:45:55PM +0000, Ian Campbell wrote:
> > > On Mon, 2015-03-23 at 16:20 +0000, Pang, LongtaoX wrote:
> > > > >
> > > > > > > > The editconfig_cd thing -- yet another thing which Ian
> > > > > > > > questioned and which it was agreed you would change but you
> haven't.
> > > > > > > >
> > > > > > > For this question, I have sent a mail about it.(2015-03-04)
> > > > > > > After finishing L1 guest VM installation, we need to change
> > > > > > > L1 guest boot sequence from ISO image to hard disk, we need
> > > > > > > modify the "boot=cd" ,
> > > > > >
> > > > > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > > > > sufficient? It removes the CD from the virtual drive, meaning
> > > > > > that "boot=dc" will fail to boot from d and fallthru to c.
> > > > > >
> > > > > > >  also need to enable 'nestedhvm' feature in hvm configure
> > > > > > > file,
> > > > > >
> > > > > > This certainly doesn't belong in a function called
> > > > > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > > > > >
> > > > > > Anyway, it's not clear why you need to edit this into the
> > > > > > nestedhvm configuration, instead of adding it when the
> > > > > > configuration is created via more_prepareguest_hvm. What harm
> > > > > > is there in enabling this during guest install?
> > > > > >
> > > > > I will try it.
> > > > >
> > > > Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it
> > > > could boot into L1 OS, but failed to install packages( such as
> > > > lvm2, rsync, bridge-utils ....) via Debian repo in L1, as below msg:
> > >
> > > Oh dear. Things really ought to be tailored on install to use the
> > > network repositories for the apt sources, not the cdrom.
> >
> > When I wrote ts-debian-hvm-install, one of the problems (if I remember
> > correctly) was that our network infrastructure didn't support booting
> > EFI from PXE boot. I ended up making that disk image to sort of work
> > around this.
> >
> > >
> > > Installing from netboot rather than netinst media ought to achieve
> > > that, I'm not sure with ts-debian-hvm-install uses though or how to
> > > achieve it via preseeding if it isn't the default for the given media.
> > >
> >
> > Per  https://www.debian.org/releases/stable/example-preseed.txt,
> > these runes look interesting.
> >
> > # Additional repositories, local[0-9] available #d-i
> > apt-setup/local0/repository string \
> > #       http://local.server/debian stable main
> > #d-i apt-setup/local0/comment string local server # Enable deb-src
> > lines #d-i apt-setup/local0/source boolean true # URL to the public
> > key of the local repository; you must provide a key # or # apt will
> > complain about the unauthenticated repository and so the #
> > sources.list line will be left commented out #d-i apt-setup/local0/key
> > string http://local.server/key
> >
> > Not sure if they will really end up in source.list though.
> 
> My expectation is that the existing preseed will have resulted in both http and
> cdrom entries, and all that is needed is to comment out the cdrom ones so the
> network ones take precedence.
> 
> Lets wait for an answer to my question about what is in sources.list on these
> VMs before speculating further on how to fix this though.
> 
> Ian.
I have checked the sources.list file in L1 guest, it contains both CDROM repo entry and URL entry(Debian repository mirror location), 
Such as below:
deb cdrom:[Debian GNU/Linux 7.6.0 _Wheezy_ - Official amd64 DVD Binary-1 20140712-14:11]/ wheezy contrib main
deb http://linux-ftp.sh.intel.com//pub/mirrors/debian wheezy main
deb-src http://linux-ftp.sh.intel.com//pub/mirrors/debian wheezy main

It seems that CDROM repo entry take effect, but it definitely unavailable, because ISO image is removed.
If I comment out the CDROM repo entry manually, and then try to 'apt-get install', it works fine.
For wei's first solution that change boot sequence from cd_disc to HDD, it does works and I have created a 'guest_ediconfig_nocd' function about that in previously patchs, maybe it's not preferred according to Ian Campbell's opinion.
So, maybe I should write some code in 'ts-nested-setup' script to implement ssh into L1, edit sources.list and comment out the CDROM repo entry. Or, do your have some easy ways?

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Hu, Robert]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Monday, March 23, 2015 5:50 PM
> To: Hu, Robert
> Cc: Pang, LongtaoX; xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com;
> wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport
> APIs
> 
> On Mon, 2015-03-23 at 06:31 +0000, Hu, Robert wrote:
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > Sent: Friday, March 20, 2015 8:20 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 2/6] Add and expose some testsupport
> > > APIs
> > >
> > > On Fri, 2015-03-20 at 12:09 +0000, Pang, LongtaoX wrote:
> > > > Add xen-devel in mail loop.
> > >
> > > Here is what I wrong in reply to the private version without noticing
> > > that it was private.
> > >
> > > On Fri, 2015-03-20 at 11:59 +0000, Pang, LongtaoX wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > > > Sent: Friday, March 20, 2015 12:27 AM
> > > > > 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 2/6] Add and expose some
> testsupport
> > > > > APIs
> > > > >
> > > > > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > > > > From: "longtao.pang" <longtaox.pang@intel.com>
> > > > > >
> > > > > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > > > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > > > > Maybe this limitation can be removed later after some fix it. For now,
> > > > > > we have to accomodate to it.
> > > > >
> > > > > You have done this unconditionally, which means it affects all guests.
> > > > > You need to make this configurable by the caller, probably by plumbing it
> > > > > through in $xopts (a hash of extra options).
> > > > >
> > > > > I see now you were told this last time around by Ian J, please don't just
> > > resend
> > > > > such things without change either fix them, make an argument for doing
> it
> > > your
> > > > > way or ask for clarification if you don't understand the requested
> change.
> > > > >
> > > >
> > > > Thanks for your advice, I will try it. But, do you have any idea about below
> > > issue that confused me?
> > > > After L1 Debian hvm guest boot into XEN kernel, it failed to load 8139cp
> > > driver(Realtek RTL-8139),
> > > > that cause L1 guest's network unavailable, and I have to specify
> > > 'model=e1000' to make L1's network available.
> > > > The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).
> > >
> > > Could just be a bug in Debian's kernel. Without more information it's
> > > rather hard to say.
> > >
> > Yes, it's hard to identify the root cause and we don't plan to dig out why here.
> > How about we move this designation to ts-nested-setup, after first step the
> normal
> > guest was created, we modify guest's configuration then?
> 
> Make it an option to the function which creates the configuration in the
> first place. Quoting myself from earlier in this very thread:
Then we still need to designate to use e1000 device somewhere; say, in
ts-nested-setup, we store this designation in runvar.
Then when creating the config, according to your proposal, we plumbing
such designation through $xopts, since we see it in runvar. Are you OK
with such implementation?
But if like above, I would prefer to altering/manipulating guest configation 
directly in ts-nested-setup after normal guest is setup. Since this is more
simple and constrained to nested case only.
> 
> > You have done this unconditionally, which means it affects all guests.
> > You need to make this configurable by the caller, probably by plumbing
> > it through in $xopts (a hash of extra options).
> 
> > The prerequisite here is to get configuration reloaded when guest 'reboot'.
> Do you
> > know any approach to achieve this? seems 'restart' action on reboot event
> > doesn’t read new configuration.
> 
> Look at hos ts-debian-hvm-install does it, it uses OnReboot=preserve
> when calling more_prepareguest_hvm, then it waits for the guest to
> reboot (which ends up preserved), then it manually destroys it, rewrites
> the config and restarts the guest.
> 
> AFAICT this is exactly what you need.
Then we're going to substitute ts-host-reboot with guest destroy and create
guest. Are you OK with this?
> 
> > > Anyway, it's not clear why you need to edit this into the nestedhvm
> > > configuration, instead of adding it when the configuration is created
> > > via more_prepareguest_hvm. What harm is there in enabling this during
> > > guest install?
> > Seems no harm if when create a normal guest with 'nestedhvm' in its config.
> > So
> > 1. we have this option enabled in guest configure ever since.
> > 2. We add in this primitive after first step (normal guest setup),
> > in ts-nested-setup.
> >
> > Which would you prefer?
> 
> If there is no harm in it then I don't see why you wouldn't just do #1.
> 
> Ian.
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Hu, Robert]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Monday, March 23, 2015 5:50 PM
> To: Hu, Robert
> Cc: Pang, LongtaoX; xen-devel@lists.xen.org; Ian.Jackson@eu.citrix.com;
> wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport
> APIs
> 
> On Mon, 2015-03-23 at 06:31 +0000, Hu, Robert wrote:
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > Sent: Friday, March 20, 2015 8:20 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 2/6] Add and expose some testsupport
> > > APIs
> > >
> > > On Fri, 2015-03-20 at 12:09 +0000, Pang, LongtaoX wrote:
> > > > Add xen-devel in mail loop.
> > >
> > > Here is what I wrong in reply to the private version without noticing
> > > that it was private.
> > >
> > > On Fri, 2015-03-20 at 11:59 +0000, Pang, LongtaoX wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > > > Sent: Friday, March 20, 2015 12:27 AM
> > > > > 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 2/6] Add and expose some
> testsupport
> > > > > APIs
> > > > >
> > > > > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > > > > From: "longtao.pang" <longtaox.pang@intel.com>
> > > > > >
> > > > > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > > > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > > > > Maybe this limitation can be removed later after some fix it. For now,
> > > > > > we have to accomodate to it.
> > > > >
> > > > > You have done this unconditionally, which means it affects all guests.
> > > > > You need to make this configurable by the caller, probably by plumbing it
> > > > > through in $xopts (a hash of extra options).
> > > > >
> > > > > I see now you were told this last time around by Ian J, please don't just
> > > resend
> > > > > such things without change either fix them, make an argument for doing
> it
> > > your
> > > > > way or ask for clarification if you don't understand the requested
> change.
> > > > >
> > > >
> > > > Thanks for your advice, I will try it. But, do you have any idea about below
> > > issue that confused me?
> > > > After L1 Debian hvm guest boot into XEN kernel, it failed to load 8139cp
> > > driver(Realtek RTL-8139),
> > > > that cause L1 guest's network unavailable, and I have to specify
> > > 'model=e1000' to make L1's network available.
> > > > The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).
> > >
> > > Could just be a bug in Debian's kernel. Without more information it's
> > > rather hard to say.
> > >
> > Yes, it's hard to identify the root cause and we don't plan to dig out why here.
> > How about we move this designation to ts-nested-setup, after first step the
> normal
> > guest was created, we modify guest's configuration then?
> 
> Make it an option to the function which creates the configuration in the
> first place. Quoting myself from earlier in this very thread:
> 
> > You have done this unconditionally, which means it affects all guests.
> > You need to make this configurable by the caller, probably by plumbing
> > it through in $xopts (a hash of extra options).
> 
> > The prerequisite here is to get configuration reloaded when guest 'reboot'.
> Do you
> > know any approach to achieve this? seems 'restart' action on reboot event
> > doesn’t read new configuration.
> 
> Look at hos ts-debian-hvm-install does it, it uses OnReboot=preserve
> when calling more_prepareguest_hvm, then it waits for the guest to
> reboot (which ends up preserved), then it manually destroys it, rewrites
> the config and restarts the guest.
> 
> AFAICT this is exactly what you need.
> 
> > > Anyway, it's not clear why you need to edit this into the nestedhvm
> > > configuration, instead of adding it when the configuration is created
> > > via more_prepareguest_hvm. What harm is there in enabling this during
> > > guest install?
> > Seems no harm if when create a normal guest with 'nestedhvm' in its config.
> > So
> > 1. we have this option enabled in guest configure ever since.
> > 2. We add in this primitive after first step (normal guest setup),
> > in ts-nested-setup.
> >
> > Which would you prefer?
> 
> If there is no harm in it then I don't see why you wouldn't just do #1.
Though no harm as far as I see now, I cannot forcast any harm in the
future. Anyway, this 'nestedhvm' is for a nested usage. Better to enable
it when we are actually going enable a nested environment.
Practically, we can do it in either way. and the first is actually easier to do.
> 
> Ian.
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Mon, 2015-03-23 at 17:29 +0000, Wei Liu wrote:
> On Mon, Mar 23, 2015 at 04:45:55PM +0000, Ian Campbell wrote:
> > On Mon, 2015-03-23 at 16:20 +0000, Pang, LongtaoX wrote:
> > > > 
> > > > > > > The editconfig_cd thing -- yet another thing which Ian questioned
> > > > > > > and which it was agreed you would change but you haven't.
> > > > > > >
> > > > > > For this question, I have sent a mail about it.(2015-03-04) After
> > > > > > finishing L1 guest VM installation, we need to change L1 guest boot
> > > > > > sequence from ISO image to hard disk, we need modify the "boot=cd" ,
> > > > >
> > > > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > > > sufficient? It removes the CD from the virtual drive, meaning that
> > > > > "boot=dc" will fail to boot from d and fallthru to c.
> > > > >
> > > > > >  also need to enable 'nestedhvm' feature in hvm configure file,
> > > > >
> > > > > This certainly doesn't belong in a function called
> > > > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > > > >
> > > > > Anyway, it's not clear why you need to edit this into the nestedhvm
> > > > > configuration, instead of adding it when the configuration is created
> > > > > via more_prepareguest_hvm. What harm is there in enabling this during
> > > > > guest install?
> > > > >
> > > > I will try it.
> > > > 
> > > Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it
> > > could boot into L1 OS, but failed to install packages( such as lvm2,
> > > rsync, bridge-utils ....) via Debian repo in L1, as below msg:
> > 
> > Oh dear. Things really ought to be tailored on install to use the
> > network repositories for the apt sources, not the cdrom.
> 
> When I wrote ts-debian-hvm-install, one of the problems (if I remember
> correctly) was that our network infrastructure didn't support booting
> EFI from PXE boot. I ended up making that disk image to sort of work
> around this.
> 
> > 
> > Installing from netboot rather than netinst media ought to achieve that,
> > I'm not sure with ts-debian-hvm-install uses though or how to achieve it
> > via preseeding if it isn't the default for the given media.
> > 
> 
> Per  https://www.debian.org/releases/stable/example-preseed.txt,
> these runes look interesting.
> 
> # Additional repositories, local[0-9] available
> #d-i apt-setup/local0/repository string \
> #       http://local.server/debian stable main
> #d-i apt-setup/local0/comment string local server
> # Enable deb-src lines
> #d-i apt-setup/local0/source boolean true
> # URL to the public key of the local repository; you must provide a key
> # or
> # apt will complain about the unauthenticated repository and so the
> # sources.list line will be left commented out
> #d-i apt-setup/local0/key string http://local.server/key
> 
> Not sure if they will really end up in source.list though.

My expectation is that the existing preseed will have resulted in both
http and cdrom entries, and all that is needed is to comment out the
cdrom ones so the network ones take precedence.

Lets wait for an answer to my question about what is in sources.list on
these VMs before speculating further on how to fix this though.

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Mon, 2015-03-23 at 17:19 +0000, Wei Liu wrote:
> On Mon, Mar 23, 2015 at 04:20:08PM +0000, Pang, LongtaoX wrote:
> > > 
> > > > > > The editconfig_cd thing -- yet another thing which Ian questioned
> > > > > > and which it was agreed you would change but you haven't.
> > > > > >
> > > > > For this question, I have sent a mail about it.(2015-03-04) After
> > > > > finishing L1 guest VM installation, we need to change L1 guest boot
> > > > > sequence from ISO image to hard disk, we need modify the "boot=cd" ,
> > > >
> > > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > > sufficient? It removes the CD from the virtual drive, meaning that
> > > > "boot=dc" will fail to boot from d and fallthru to c.
> > > >
> > > > >  also need to enable 'nestedhvm' feature in hvm configure file,
> > > >
> > > > This certainly doesn't belong in a function called
> > > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > > >
> > > > Anyway, it's not clear why you need to edit this into the nestedhvm
> > > > configuration, instead of adding it when the configuration is created
> > > > via more_prepareguest_hvm. What harm is there in enabling this during
> > > > guest install?
> > > >
> > > I will try it.
> > > 
> > Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it could boot into L1 OS, but failed to install packages( such as lvm2, rsync, bridge-utils ....) via Debian repo in L1, as below msg:
> > root@nested:~# apt-get install lvm2 --no-install-recommends -y
> > Reading package lists... Done
> > Building dependency tree
> > Reading state information... Done
> > The following extra packages will be installed:
> >   libdevmapper-event1.02.1 libreadline5
> > The following NEW packages will be installed:
> >   libdevmapper-event1.02.1 libreadline5 lvm2
> > 0 upgraded, 3 newly installed, 0 to remove and 0 not upgraded.
> > Need to get 0 B/767 kB of archives.
> > After this operation, 1,521 kB of additional disk space will be used.
> > Media change: please insert the disc labeled
> >  'Debian GNU/Linux 7.6.0 _Wheezy_ - Official amd64 DVD Binary-1 20140712-14:11'
> > in the drive '/media/cdrom/' and press enter
> > 
> > I checked the 'sources.list' file in L1, because L1 Debian OS assume that ISO Image as repo, but the ISO image does not exist actually.
> > Since '$gho->{Rimage}' is replaced as ' $emptyiso' in ' guest_ediconfig_nocd', maybe it is not sufficient for nested test.
> > If keep to re-use ' guest_ediconfig_nocd' , is there any approach to setting guest's repo in osstest system? 
> > 
> > 
> 
> Here are my 2 cents. Might be an alternative solution to your problem.
> 
> The reason that we call guest_editconfig_nocd is to prevent the guest
> from booting from CDROM again.
> 
> You can probably invent a function like guest_editconfig_boot_from_hd
> (name subject to improvement) to switch "boot=dc" (which is the default
> value in more_prepareguest_hvm) to "boot=cd".
> 
> That way you can boot from hard drive while at the same time keep the
> CDROM filled.

That might work, but only so long as all of our dependencies happen to
be on the install media which was used. So the preference IMHO should be
to enable the network apt repositories in the guest and get rid of any
cdrom related ones. This will also make the guest more consistent with
the L0 host installations.

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Wei Liu]

On Mon, Mar 23, 2015 at 04:45:55PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-23 at 16:20 +0000, Pang, LongtaoX wrote:
> > > 
> > > > > > The editconfig_cd thing -- yet another thing which Ian questioned
> > > > > > and which it was agreed you would change but you haven't.
> > > > > >
> > > > > For this question, I have sent a mail about it.(2015-03-04) After
> > > > > finishing L1 guest VM installation, we need to change L1 guest boot
> > > > > sequence from ISO image to hard disk, we need modify the "boot=cd" ,
> > > >
> > > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > > sufficient? It removes the CD from the virtual drive, meaning that
> > > > "boot=dc" will fail to boot from d and fallthru to c.
> > > >
> > > > >  also need to enable 'nestedhvm' feature in hvm configure file,
> > > >
> > > > This certainly doesn't belong in a function called
> > > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > > >
> > > > Anyway, it's not clear why you need to edit this into the nestedhvm
> > > > configuration, instead of adding it when the configuration is created
> > > > via more_prepareguest_hvm. What harm is there in enabling this during
> > > > guest install?
> > > >
> > > I will try it.
> > > 
> > Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it
> > could boot into L1 OS, but failed to install packages( such as lvm2,
> > rsync, bridge-utils ....) via Debian repo in L1, as below msg:
> 
> Oh dear. Things really ought to be tailored on install to use the
> network repositories for the apt sources, not the cdrom.

When I wrote ts-debian-hvm-install, one of the problems (if I remember
correctly) was that our network infrastructure didn't support booting
EFI from PXE boot. I ended up making that disk image to sort of work
around this.

> 
> Installing from netboot rather than netinst media ought to achieve that,
> I'm not sure with ts-debian-hvm-install uses though or how to achieve it
> via preseeding if it isn't the default for the given media.
> 

Per  https://www.debian.org/releases/stable/example-preseed.txt,
these runes look interesting.

# Additional repositories, local[0-9] available
#d-i apt-setup/local0/repository string \
#       http://local.server/debian stable main
#d-i apt-setup/local0/comment string local server
# Enable deb-src lines
#d-i apt-setup/local0/source boolean true
# URL to the public key of the local repository; you must provide a key
# or
# apt will complain about the unauthenticated repository and so the
# sources.list line will be left commented out
#d-i apt-setup/local0/key string http://local.server/key

Not sure if they will really end up in source.list though.

Wei.

> What does /etc/apt/sources.list contain for your installed L1 guest?
> Does it contain both cdrom and http entries?
> 
> Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Wei Liu]

On Mon, Mar 23, 2015 at 04:20:08PM +0000, Pang, LongtaoX wrote:
> > 
> > > > > The editconfig_cd thing -- yet another thing which Ian questioned
> > > > > and which it was agreed you would change but you haven't.
> > > > >
> > > > For this question, I have sent a mail about it.(2015-03-04) After
> > > > finishing L1 guest VM installation, we need to change L1 guest boot
> > > > sequence from ISO image to hard disk, we need modify the "boot=cd" ,
> > >
> > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > sufficient? It removes the CD from the virtual drive, meaning that
> > > "boot=dc" will fail to boot from d and fallthru to c.
> > >
> > > >  also need to enable 'nestedhvm' feature in hvm configure file,
> > >
> > > This certainly doesn't belong in a function called
> > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > >
> > > Anyway, it's not clear why you need to edit this into the nestedhvm
> > > configuration, instead of adding it when the configuration is created
> > > via more_prepareguest_hvm. What harm is there in enabling this during
> > > guest install?
> > >
> > I will try it.
> > 
> Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it could boot into L1 OS, but failed to install packages( such as lvm2, rsync, bridge-utils ....) via Debian repo in L1, as below msg:
> root@nested:~# apt-get install lvm2 --no-install-recommends -y
> Reading package lists... Done
> Building dependency tree
> Reading state information... Done
> The following extra packages will be installed:
>   libdevmapper-event1.02.1 libreadline5
> The following NEW packages will be installed:
>   libdevmapper-event1.02.1 libreadline5 lvm2
> 0 upgraded, 3 newly installed, 0 to remove and 0 not upgraded.
> Need to get 0 B/767 kB of archives.
> After this operation, 1,521 kB of additional disk space will be used.
> Media change: please insert the disc labeled
>  'Debian GNU/Linux 7.6.0 _Wheezy_ - Official amd64 DVD Binary-1 20140712-14:11'
> in the drive '/media/cdrom/' and press enter
> 
> I checked the 'sources.list' file in L1, because L1 Debian OS assume that ISO Image as repo, but the ISO image does not exist actually.
> Since '$gho->{Rimage}' is replaced as ' $emptyiso' in ' guest_ediconfig_nocd', maybe it is not sufficient for nested test.
> If keep to re-use ' guest_ediconfig_nocd' , is there any approach to setting guest's repo in osstest system? 
> 
> 

Here are my 2 cents. Might be an alternative solution to your problem.

The reason that we call guest_editconfig_nocd is to prevent the guest
from booting from CDROM again.

You can probably invent a function like guest_editconfig_boot_from_hd
(name subject to improvement) to switch "boot=dc" (which is the default
value in more_prepareguest_hvm) to "boot=cd".

That way you can boot from hard drive while at the same time keep the
CDROM filled.

Wei.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Mon, 2015-03-23 at 16:20 +0000, Pang, LongtaoX wrote:
> > 
> > > > > The editconfig_cd thing -- yet another thing which Ian questioned
> > > > > and which it was agreed you would change but you haven't.
> > > > >
> > > > For this question, I have sent a mail about it.(2015-03-04) After
> > > > finishing L1 guest VM installation, we need to change L1 guest boot
> > > > sequence from ISO image to hard disk, we need modify the "boot=cd" ,
> > >
> > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > sufficient? It removes the CD from the virtual drive, meaning that
> > > "boot=dc" will fail to boot from d and fallthru to c.
> > >
> > > >  also need to enable 'nestedhvm' feature in hvm configure file,
> > >
> > > This certainly doesn't belong in a function called
> > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > >
> > > Anyway, it's not clear why you need to edit this into the nestedhvm
> > > configuration, instead of adding it when the configuration is created
> > > via more_prepareguest_hvm. What harm is there in enabling this during
> > > guest install?
> > >
> > I will try it.
> > 
> Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it
> could boot into L1 OS, but failed to install packages( such as lvm2,
> rsync, bridge-utils ....) via Debian repo in L1, as below msg:

Oh dear. Things really ought to be tailored on install to use the
network repositories for the apt sources, not the cdrom.

Installing from netboot rather than netinst media ought to achieve that,
I'm not sure with ts-debian-hvm-install uses though or how to achieve it
via preseeding if it isn't the default for the given media.

What does /etc/apt/sources.list contain for your installed L1 guest?
Does it contain both cdrom and http entries?

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Pang, LongtaoX]

> 
> > > > The editconfig_cd thing -- yet another thing which Ian questioned
> > > > and which it was agreed you would change but you haven't.
> > > >
> > > For this question, I have sent a mail about it.(2015-03-04) After
> > > finishing L1 guest VM installation, we need to change L1 guest boot
> > > sequence from ISO image to hard disk, we need modify the "boot=cd" ,
> >
> > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > sufficient? It removes the CD from the virtual drive, meaning that
> > "boot=dc" will fail to boot from d and fallthru to c.
> >
> > >  also need to enable 'nestedhvm' feature in hvm configure file,
> >
> > This certainly doesn't belong in a function called
> > guest_editconfig_cd, since it has nothing to do with cds at all.
> >
> > Anyway, it's not clear why you need to edit this into the nestedhvm
> > configuration, instead of adding it when the configuration is created
> > via more_prepareguest_hvm. What harm is there in enabling this during
> > guest install?
> >
> I will try it.
> 
Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it could boot into L1 OS, but failed to install packages( such as lvm2, rsync, bridge-utils ....) via Debian repo in L1, as below msg:
root@nested:~# apt-get install lvm2 --no-install-recommends -y
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following extra packages will be installed:
  libdevmapper-event1.02.1 libreadline5
The following NEW packages will be installed:
  libdevmapper-event1.02.1 libreadline5 lvm2
0 upgraded, 3 newly installed, 0 to remove and 0 not upgraded.
Need to get 0 B/767 kB of archives.
After this operation, 1,521 kB of additional disk space will be used.
Media change: please insert the disc labeled
 'Debian GNU/Linux 7.6.0 _Wheezy_ - Official amd64 DVD Binary-1 20140712-14:11'
in the drive '/media/cdrom/' and press enter

I checked the 'sources.list' file in L1, because L1 Debian OS assume that ISO Image as repo, but the ISO image does not exist actually.
Since '$gho->{Rimage}' is replaced as ' $emptyiso' in ' guest_ediconfig_nocd', maybe it is not sufficient for nested test.
If keep to re-use ' guest_ediconfig_nocd' , is there any approach to setting guest's repo in osstest system? 

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Mon, 2015-03-23 at 06:31 +0000, Hu, Robert wrote:
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Friday, March 20, 2015 8:20 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 2/6] Add and expose some testsupport
> > APIs
> > 
> > On Fri, 2015-03-20 at 12:09 +0000, Pang, LongtaoX wrote:
> > > Add xen-devel in mail loop.
> > 
> > Here is what I wrong in reply to the private version without noticing
> > that it was private.
> > 
> > On Fri, 2015-03-20 at 11:59 +0000, Pang, LongtaoX wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > > Sent: Friday, March 20, 2015 12:27 AM
> > > > 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 2/6] Add and expose some testsupport
> > > > APIs
> > > >
> > > > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > > > From: "longtao.pang" <longtaox.pang@intel.com>
> > > > >
> > > > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > > > Maybe this limitation can be removed later after some fix it. For now,
> > > > > we have to accomodate to it.
> > > >
> > > > You have done this unconditionally, which means it affects all guests.
> > > > You need to make this configurable by the caller, probably by plumbing it
> > > > through in $xopts (a hash of extra options).
> > > >
> > > > I see now you were told this last time around by Ian J, please don't just
> > resend
> > > > such things without change either fix them, make an argument for doing it
> > your
> > > > way or ask for clarification if you don't understand the requested change.
> > > >
> > >
> > > Thanks for your advice, I will try it. But, do you have any idea about below
> > issue that confused me?
> > > After L1 Debian hvm guest boot into XEN kernel, it failed to load 8139cp
> > driver(Realtek RTL-8139),
> > > that cause L1 guest's network unavailable, and I have to specify
> > 'model=e1000' to make L1's network available.
> > > The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).
> > 
> > Could just be a bug in Debian's kernel. Without more information it's
> > rather hard to say.
> > 
> Yes, it's hard to identify the root cause and we don't plan to dig out why here.
> How about we move this designation to ts-nested-setup, after first step the normal
> guest was created, we modify guest's configuration then?

Make it an option to the function which creates the configuration in the
first place. Quoting myself from earlier in this very thread:

> You have done this unconditionally, which means it affects all guests.
> You need to make this configurable by the caller, probably by plumbing
> it through in $xopts (a hash of extra options).

> The prerequisite here is to get configuration reloaded when guest 'reboot'. Do you
> know any approach to achieve this? seems 'restart' action on reboot event
> doesn’t read new configuration.

Look at hos ts-debian-hvm-install does it, it uses OnReboot=preserve
when calling more_prepareguest_hvm, then it waits for the guest to
reboot (which ends up preserved), then it manually destroys it, rewrites
the config and restarts the guest.

AFAICT this is exactly what you need.

> > Anyway, it's not clear why you need to edit this into the nestedhvm
> > configuration, instead of adding it when the configuration is created
> > via more_prepareguest_hvm. What harm is there in enabling this during
> > guest install?
> Seems no harm if when create a normal guest with 'nestedhvm' in its config.
> So
> 1. we have this option enabled in guest configure ever since.
> 2. We add in this primitive after first step (normal guest setup),
> in ts-nested-setup.
> 
> Which would you prefer?

If there is no harm in it then I don't see why you wouldn't just do #1.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Hu, Robert]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Friday, March 20, 2015 8:20 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 2/6] Add and expose some testsupport
> APIs
> 
> On Fri, 2015-03-20 at 12:09 +0000, Pang, LongtaoX wrote:
> > Add xen-devel in mail loop.
> 
> Here is what I wrong in reply to the private version without noticing
> that it was private.
> 
> On Fri, 2015-03-20 at 11:59 +0000, Pang, LongtaoX wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > Sent: Friday, March 20, 2015 12:27 AM
> > > 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 2/6] Add and expose some testsupport
> > > APIs
> > >
> > > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > > From: "longtao.pang" <longtaox.pang@intel.com>
> > > >
> > > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > > Maybe this limitation can be removed later after some fix it. For now,
> > > > we have to accomodate to it.
> > >
> > > You have done this unconditionally, which means it affects all guests.
> > > You need to make this configurable by the caller, probably by plumbing it
> > > through in $xopts (a hash of extra options).
> > >
> > > I see now you were told this last time around by Ian J, please don't just
> resend
> > > such things without change either fix them, make an argument for doing it
> your
> > > way or ask for clarification if you don't understand the requested change.
> > >
> >
> > Thanks for your advice, I will try it. But, do you have any idea about below
> issue that confused me?
> > After L1 Debian hvm guest boot into XEN kernel, it failed to load 8139cp
> driver(Realtek RTL-8139),
> > that cause L1 guest's network unavailable, and I have to specify
> 'model=e1000' to make L1's network available.
> > The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).
> 
> Could just be a bug in Debian's kernel. Without more information it's
> rather hard to say.
> 
Yes, it's hard to identify the root cause and we don't plan to dig out why here.
How about we move this designation to ts-nested-setup, after first step the normal
guest was created, we modify guest's configuration then?
The prerequisite here is to get configuration reloaded when guest 'reboot'. Do you
know any approach to achieve this? seems 'restart' action on reboot event
doesn’t read new configuration.
> >
> > > > 2. Since reboot L1 guest VM will take more time to boot up, we
> > > > increase multi-times for reboot-confirm-booted if test nested job, and
> > > > the multi value is stored as a runvar in 'ts-nested-setup' script.
> > > > Added another function 'guest_editconfig_cd' and expose it, this
> > > > function bascically changes guest boot device sequence, alter its
> > > > on_reboot behavior to restart and enabled nestedhvm feature.
> > >
> > > This looks like two items run together?
> > >
> > > The multi_reboot_time thing sounds ok, but it should be called
> > > reboot_time_factor or something like that. In fact I see that Ian suggested
> > > previously that it should have the host ident in it, that makes sense to me.
> > >
> > I will try it. Also, how do you handle below question after reboot
> > host OS during running OSSTest job?
> > After finishing L0 and L1 host installation, the OSs will take a lot
> > time(about 150s) to start MTA service and NTP service. I know that,
> > the poll_loop timeout is 40s of 'reboot-confirm-booted',
> > that's why timeout happened when calling 'host_reboot' function after reboot
> host OS.
> 
> I'm afraid I don't know what you are asking here.
> 
> > > The editconfig_cd thing -- yet another thing which Ian questioned and which
> it
> > > was agreed you would change but you haven't.
> > >
> > For this question, I have sent a mail about it.(2015-03-04)
> > After finishing L1 guest VM installation, we need to change L1 guest boot
> sequence from ISO image to hard disk,
> > we need modify the "boot=cd" ,
> 
> Do you? As Ian asked before, why is guest_editconfig_nocd  not
> sufficient? It removes the CD from the virtual drive, meaning that
> "boot=dc" will fail to boot from d and fallthru to c.
> 
> >  also need to enable 'nestedhvm' feature in hvm configure file,
> 
> This certainly doesn't belong in a function called guest_editconfig_cd,
> since it has nothing to do with cds at all.
> 
> Anyway, it's not clear why you need to edit this into the nestedhvm
> configuration, instead of adding it when the configuration is created
> via more_prepareguest_hvm. What harm is there in enabling this during
> guest install?
Seems no harm if when create a normal guest with 'nestedhvm' in its config.
So
1. we have this option enabled in guest configure ever since.
2. We add in this primitive after first step (normal guest setup),
in ts-nested-setup.

Which would you prefer?
> 
> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Fri, 2015-03-20 at 12:59 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Friday, March 20, 2015 8:20 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 2/6] Add and expose some testsupport
> > APIs
> > 
> > On Fri, 2015-03-20 at 12:09 +0000, Pang, LongtaoX wrote:
> > > Add xen-devel in mail loop.
> > 
> > Here is what I wrong in reply to the private version without noticing that it was
> > private.
> > 
> > On Fri, 2015-03-20 at 11:59 +0000, Pang, LongtaoX wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > > Sent: Friday, March 20, 2015 12:27 AM
> > > > 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 2/6] Add and expose some
> > > > testsupport APIs
> > > >
> > > > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > > > From: "longtao.pang" <longtaox.pang@intel.com>
> > > > >
> > > > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > > > Maybe this limitation can be removed later after some fix it. For
> > > > > now, we have to accomodate to it.
> > > >
> > > > You have done this unconditionally, which means it affects all guests.
> > > > You need to make this configurable by the caller, probably by
> > > > plumbing it through in $xopts (a hash of extra options).
> > > >
> > > > I see now you were told this last time around by Ian J, please don't
> > > > just resend such things without change either fix them, make an
> > > > argument for doing it your way or ask for clarification if you don't
> > understand the requested change.
> > > >
> > >
> > > Thanks for your advice, I will try it. But, do you have any idea about below
> > issue that confused me?
> > > After L1 Debian hvm guest boot into XEN kernel, it failed to load
> > > 8139cp driver(Realtek RTL-8139), that cause L1 guest's network unavailable,
> > and I have to specify 'model=e1000' to make L1's network available.
> > > The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).
> > 
> > Could just be a bug in Debian's kernel. Without more information it's rather
> > hard to say.
> > 
> > >
> > > > > 2. Since reboot L1 guest VM will take more time to boot up, we
> > > > > increase multi-times for reboot-confirm-booted if test nested job,
> > > > > and the multi value is stored as a runvar in 'ts-nested-setup' script.
> > > > > Added another function 'guest_editconfig_cd' and expose it, this
> > > > > function bascically changes guest boot device sequence, alter its
> > > > > on_reboot behavior to restart and enabled nestedhvm feature.
> > > >
> > > > This looks like two items run together?
> > > >
> > > > The multi_reboot_time thing sounds ok, but it should be called
> > > > reboot_time_factor or something like that. In fact I see that Ian
> > > > suggested previously that it should have the host ident in it, that makes
> > sense to me.
> > > >
> > > I will try it. Also, how do you handle below question after reboot
> > > host OS during running OSSTest job?
> > > After finishing L0 and L1 host installation, the OSs will take a lot
> > > time(about 150s) to start MTA service and NTP service. I know that,
> > > the poll_loop timeout is 40s of 'reboot-confirm-booted', that's why
> > > timeout happened when calling 'host_reboot' function after reboot host OS.
> > 
> > I'm afraid I don't know what you are asking here.
> > 
> When rebooting Debian L0 or Debian L1 guest, during booting, it will
> take a lot of time(about 150s) to starting MTA and NTP service, and
> then boot into Debian OS.

That normally suggests that there is something wrong with your network
setup, perhaps forward or reverse DNS. IMHO it shouldn't ever take 150s
to do this for a native/L0 Debian.

For example
http://www.chiark.greenend.org.uk/~xensrcts/logs/36514/test-amd64-amd64-xl/serial-gall-mite.log shows:
        Mar 18 21:42:39.935554 Using makefile-style concurrent boot in runlevel 2.
        Mar 18 21:42:39.951952 Starting rpcbind daemon...Already running..
        Mar 18 21:42:39.995954 Starting NFS common utilities: statd idmapd.
        Mar 18 21:42:40.019569 Starting enhanced syslogd: rsyslogd.
        Mar 18 21:42:40.191582 Starting ACPI services....
        Mar 18 21:42:40.315569 Starting web server: apache2.
        Mar 18 21:42:40.539998 Starting deferred execution scheduler: atd.
        Mar 18 21:42:40.708028 Starting periodic command scheduler: cron.
        Mar 18 21:42:40.876030 Starting NTP server: ntpd.
        Mar 18 21:42:40.876078 Starting OpenBSD Secure Shell server: sshd.
        Mar 18 21:42:41.052041 Starting MTA: exim4.
        Mar 18 21:42:41.275639 
        Mar 18 21:42:42.351734 Debian GNU/Linux 7 gall-mite ttyS0
        Mar 18 21:42:42.363737 
        Mar 18 21:42:42.363769 gall-mite login: 
i.e. a few seconds. Even a really slow machine shouldn't be taking 150s.

I doubt L1 should be noticeably different from L0 in this (L2 might be a
different matter but AFAICT you aren't talking about L2 here).

You need to figure out what is in your environment that makes it take so
long to start up these services, not work around the issue by fudging
the timeouts in osstest.

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Friday, March 20, 2015 8:20 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 2/6] Add and expose some testsupport
> APIs
> 
> On Fri, 2015-03-20 at 12:09 +0000, Pang, LongtaoX wrote:
> > Add xen-devel in mail loop.
> 
> Here is what I wrong in reply to the private version without noticing that it was
> private.
> 
> On Fri, 2015-03-20 at 11:59 +0000, Pang, LongtaoX wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > Sent: Friday, March 20, 2015 12:27 AM
> > > 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 2/6] Add and expose some
> > > testsupport APIs
> > >
> > > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > > From: "longtao.pang" <longtaox.pang@intel.com>
> > > >
> > > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > > Maybe this limitation can be removed later after some fix it. For
> > > > now, we have to accomodate to it.
> > >
> > > You have done this unconditionally, which means it affects all guests.
> > > You need to make this configurable by the caller, probably by
> > > plumbing it through in $xopts (a hash of extra options).
> > >
> > > I see now you were told this last time around by Ian J, please don't
> > > just resend such things without change either fix them, make an
> > > argument for doing it your way or ask for clarification if you don't
> understand the requested change.
> > >
> >
> > Thanks for your advice, I will try it. But, do you have any idea about below
> issue that confused me?
> > After L1 Debian hvm guest boot into XEN kernel, it failed to load
> > 8139cp driver(Realtek RTL-8139), that cause L1 guest's network unavailable,
> and I have to specify 'model=e1000' to make L1's network available.
> > The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).
> 
> Could just be a bug in Debian's kernel. Without more information it's rather
> hard to say.
> 
> >
> > > > 2. Since reboot L1 guest VM will take more time to boot up, we
> > > > increase multi-times for reboot-confirm-booted if test nested job,
> > > > and the multi value is stored as a runvar in 'ts-nested-setup' script.
> > > > Added another function 'guest_editconfig_cd' and expose it, this
> > > > function bascically changes guest boot device sequence, alter its
> > > > on_reboot behavior to restart and enabled nestedhvm feature.
> > >
> > > This looks like two items run together?
> > >
> > > The multi_reboot_time thing sounds ok, but it should be called
> > > reboot_time_factor or something like that. In fact I see that Ian
> > > suggested previously that it should have the host ident in it, that makes
> sense to me.
> > >
> > I will try it. Also, how do you handle below question after reboot
> > host OS during running OSSTest job?
> > After finishing L0 and L1 host installation, the OSs will take a lot
> > time(about 150s) to start MTA service and NTP service. I know that,
> > the poll_loop timeout is 40s of 'reboot-confirm-booted', that's why
> > timeout happened when calling 'host_reboot' function after reboot host OS.
> 
> I'm afraid I don't know what you are asking here.
> 
When rebooting Debian L0 or Debian L1 guest, during booting, it will take a lot of time(about 150s) to starting MTA and NTP service, and then boot into Debian OS. But, only boot into OS completely, ' osstest-confirm-booted' shell script in '/etc/init.d' will start and generate 'reboot-confirm-booted' file at '/dev/shm' directory, only if the ' reboot-confirm-booted' file exist, then the function of 'host_reboot' is called successfully.
As 'host_reboot' function in TestSupport.pm, the 'poll_loop' timeout is 40s, in my osstest env, after reboot L0 or L1 and then calling 'host_reboot' function, always cause the 'poll_loop' timeout and exit test. 

> > > The editconfig_cd thing -- yet another thing which Ian questioned
> > > and which it was agreed you would change but you haven't.
> > >
> > For this question, I have sent a mail about it.(2015-03-04) After
> > finishing L1 guest VM installation, we need to change L1 guest boot
> > sequence from ISO image to hard disk, we need modify the "boot=cd" ,
> 
> Do you? As Ian asked before, why is guest_editconfig_nocd  not sufficient? It
> removes the CD from the virtual drive, meaning that "boot=dc" will fail to boot
> from d and fallthru to c.
> 
> >  also need to enable 'nestedhvm' feature in hvm configure file,
> 
> This certainly doesn't belong in a function called guest_editconfig_cd, since it
> has nothing to do with cds at all.
> 
> Anyway, it's not clear why you need to edit this into the nestedhvm
> configuration, instead of adding it when the configuration is created via
> more_prepareguest_hvm. What harm is there in enabling this during guest
> install?
> 
I will try it.

> Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Ian Campbell]

On Fri, 2015-03-20 at 12:09 +0000, Pang, LongtaoX wrote:
> Add xen-devel in mail loop.

Here is what I wrong in reply to the private version without noticing
that it was private.

On Fri, 2015-03-20 at 11:59 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Friday, March 20, 2015 12:27 AM
> > 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 2/6] Add and expose some testsupport
> > APIs
> > 
> > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > From: "longtao.pang" <longtaox.pang@intel.com>
> > >
> > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > Maybe this limitation can be removed later after some fix it. For now,
> > > we have to accomodate to it.
> > 
> > You have done this unconditionally, which means it affects all guests.
> > You need to make this configurable by the caller, probably by plumbing it
> > through in $xopts (a hash of extra options).
> > 
> > I see now you were told this last time around by Ian J, please don't just resend
> > such things without change either fix them, make an argument for doing it your
> > way or ask for clarification if you don't understand the requested change.
> > 
> 
> Thanks for your advice, I will try it. But, do you have any idea about below issue that confused me?
> After L1 Debian hvm guest boot into XEN kernel, it failed to load 8139cp driver(Realtek RTL-8139), 
> that cause L1 guest's network unavailable, and I have to specify 'model=e1000' to make L1's network available.
> The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).

Could just be a bug in Debian's kernel. Without more information it's
rather hard to say.

> 
> > > 2. Since reboot L1 guest VM will take more time to boot up, we
> > > increase multi-times for reboot-confirm-booted if test nested job, and
> > > the multi value is stored as a runvar in 'ts-nested-setup' script.
> > > Added another function 'guest_editconfig_cd' and expose it, this
> > > function bascically changes guest boot device sequence, alter its
> > > on_reboot behavior to restart and enabled nestedhvm feature.
> > 
> > This looks like two items run together?
> > 
> > The multi_reboot_time thing sounds ok, but it should be called
> > reboot_time_factor or something like that. In fact I see that Ian suggested
> > previously that it should have the host ident in it, that makes sense to me.
> > 
> I will try it. Also, how do you handle below question after reboot
> host OS during running OSSTest job?
> After finishing L0 and L1 host installation, the OSs will take a lot
> time(about 150s) to start MTA service and NTP service. I know that,
> the poll_loop timeout is 40s of 'reboot-confirm-booted', 
> that's why timeout happened when calling 'host_reboot' function after reboot host OS.

I'm afraid I don't know what you are asking here.

> > The editconfig_cd thing -- yet another thing which Ian questioned and which it
> > was agreed you would change but you haven't.
> > 
> For this question, I have sent a mail about it.(2015-03-04)  
> After finishing L1 guest VM installation, we need to change L1 guest boot sequence from ISO image to hard disk, 
> we need modify the "boot=cd" ,

Do you? As Ian asked before, why is guest_editconfig_nocd  not
sufficient? It removes the CD from the virtual drive, meaning that
"boot=dc" will fail to boot from d and fallthru to c.

>  also need to enable 'nestedhvm' feature in hvm configure file,

This certainly doesn't belong in a function called guest_editconfig_cd,
since it has nothing to do with cds at all.

Anyway, it's not clear why you need to edit this into the nestedhvm
configuration, instead of adding it when the configuration is created
via more_prepareguest_hvm. What harm is there in enabling this during
guest install?

Ian.

Re: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

[Pang, LongtaoX]

Add xen-devel in mail loop.

> -----Original Message-----
> From: Pang, LongtaoX
> Sent: Friday, March 20, 2015 7:59 PM
> To: 'Ian Campbell'
> Cc: Ian.Jackson@eu.citrix.com; wei.liu2@citrix.com; Hu, Robert
> Subject: RE: [OSSTEST Nested PATCH 2/6] Add and expose some testsupport
> APIs
> 
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Friday, March 20, 2015 12:27 AM
> > 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 2/6] Add and expose some
> > testsupport APIs
> >
> > On Tue, 2015-03-17 at 14:16 -0400, longtao.pang wrote:
> > > From: "longtao.pang" <longtaox.pang@intel.com>
> > >
> > > 1. Designate vif model to 'e1000', otherwise, with default device
> > > model, the L1 eth0 interface disappear, hence xenbridge cannot work.
> > > Maybe this limitation can be removed later after some fix it. For
> > > now, we have to accomodate to it.
> >
> > You have done this unconditionally, which means it affects all guests.
> > You need to make this configurable by the caller, probably by plumbing
> > it through in $xopts (a hash of extra options).
> >
> > I see now you were told this last time around by Ian J, please don't
> > just resend such things without change either fix them, make an
> > argument for doing it your way or ask for clarification if you don't understand
> the requested change.
> >
> 
Thanks for your advice, I will try it. But, do you have any idea about below issue that confused me?
After L1 Debian hvm guest boot into XEN kernel, it failed to load 8139cp driver(Realtek RTL-8139), that cause L1 guest's network unavailable, and I have
to specify 'model=e1000' to make L1's network available.
The issue does not exist in RHEL6u5 OS(L0 and L1 are both RHEL6u5 OS).
> 
> > > 2. Since reboot L1 guest VM will take more time to boot up, we
> > > increase multi-times for reboot-confirm-booted if test nested job,
> > > and the multi value is stored as a runvar in 'ts-nested-setup' script.
> > > Added another function 'guest_editconfig_cd' and expose it, this
> > > function bascically changes guest boot device sequence, alter its
> > > on_reboot behavior to restart and enabled nestedhvm feature.
> >
> > This looks like two items run together?
> >
> > The multi_reboot_time thing sounds ok, but it should be called
> > reboot_time_factor or something like that. In fact I see that Ian
> > suggested previously that it should have the host ident in it, that makes
> sense to me.
> >
I will try it. Also, how do you handle below question after reboot host OS during running OSSTest job?
After finishing L0 and L1 host installation, the OSs will take a lot time(about 150s) to start MTA service and NTP service. 
I know that, the poll_loop timeout is 40s of 'reboot-confirm-booted', that's why timeout happened when calling 'host_reboot' function after reboot host OS.

> 
> > The editconfig_cd thing -- yet another thing which Ian questioned and
> > which it was agreed you would change but you haven't.
> >
For this question, I have sent a mail about it.(2015-03-04) 
After finishing L1 guest VM installation, we need to change L1 guest boot sequence from ISO image to hard disk, we need modify the "boot=cd" , also need to enable 'nestedhvm' feature in hvm configure file, So, we added 'guest_editconfig_cd' function.
Since, 'guest_editconfig_nocd' could not get this point, if we change it, will affect all guest, that not our expected.
+sub guest_editconfig_cd ($) {
+    my ($gho) = @_;
+    guest_editconfig($gho->{Host}, $gho, sub {
+        if (m/^\s*boot\s*= '\s*d\s*c\s*'/) {
+            s/dc/cd/;
+        }
+        s/^on_reboot.*/on_reboot='restart'/;
+        s/#nestedhvm/nestedhvm/;
+    });
+}
> > I think perhaps you have accidentally resent an older version of the
> > series. If not then please go back and ensure you have addressed all
> > of the feedback given on the last iteration before sending another version.
> >
> > Ian.
> >