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

[OSSTEST Nested PATCH v11 0/7] 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.

Below patches have passed pretest and been committed; therefore not included 
in this v11 patch set.
(git repo: git://xenbits.xen.org/people/ianc/osstest.git)

    commit 699c911e59241350fc210633aba3f53afafee872
    Refactor installation of overlays
    
    commit 2545fc639107b2290236bf2ff8e9304b895ebee0
    Edit some APIs in TestSupport.pm for nested test
    
    commit 155bdb33b7c0227c1fb8b49ee54610c7a466b59b
    Move the code for setting memory size into prep()

v11 patches are based on 'nestedhvm-v10-pretest-reduced' branch.
(git repo: git://xenbits.xen.org/people/ianc/osstest.git)

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' which will be used for
           nested test. The directory path of 'Debian ISO Images' which used
           for installing HVM guest VM could be defined in 
           '~/.xen-osstest/config'. 
        1. run './standalone-reset' to generate standalone.db firstly then run 
           'build-amd64' job and then 'build-amd64-pvops', to prepare xen
           installation tarball and hvm guest kernel.
        2. run 'test-amd64-amd6-qemuu-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 reuse 'ts-debian-hvm-install' for both L1 installation
and L2 installation, use 'nestedl1' as L1's guestname and identity 
and use 'nestedl2 as L2's guestname.
It also reuses 'ts-xen-install' with L1's identity 'nestedl1' input parameter 
to differentiate from L0 Xen installation.
This patch series has been tested on test machines of amd64 arch,
Debian-7.2.0-amd64 as guests OS, with hvm domain0 of Linux kernel 3.18.5, 
in standalone mode.
Also, we use linux-stable tree as domain0 kernel source.
----------------------------------------------------------------
Ian Campbell (1):
      grub: remove patch to disable submenu from 20_linux_xen overlay
longtao.pang (6):
      Parsing grub which has 'submenu' primitive
      Changes to support '/boot' leading paths of kernel, xen, in grub
      Changes on test step of Debian hvm guest install
      Add new script to customize nested test configuration
      Compose the main recipe of nested test job
      Add test job for nest test case

 Osstest/Debian.pm               |   32 +++++++++++++-----
 make-flight                     |   31 +++++++++++++++++
 overlay/etc/grub.d/20_linux_xen |    4 ++-
 sg-run-job                      |   11 ++++++
 ts-debian-hvm-install           |   18 +++++++++-
 ts-nested-setup                 |   75 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 160 insertions(+), 11 deletions(-)
 create mode 100755 ts-nested-setup

[OSSTEST Nested PATCH v11 1/7] grub: remove patch to disable submenu from 20_linux_xen overlay

[longtao.pang]

setupboot_grub2 now supports submenus, so we can reduce our delta vs
upstream a bit.

I started by extracting 20_linux_xen from
http://snapshot.debian.org/archive/debian/20130703T094657Z/pool/main/g/grub2/grub-common_1.99-27%2Bdeb7u2_amd64.deb
and then applying the patch at
http://savannah.gnu.org/file/grub.patch?file_id=32276 (the patch from
grub bug #42420 at http://savannah.gnu.org/bugs/?43420) and
reinstating the comment at the top of the file (modified to drop the
reference to the Debian bug.

This left me with some spurious changes:

    @@ -93,7 +93,7 @@ linux_entry ()
           if test ! -e "${xen_dirname}/${xenpolicy}" ; then
              return
           fi
    -      xen_args=`echo $xen_args flask=enforcing`
    +      xen_args=`echo $xen_args flask_enabled=1 flask_enforcing=1`
           if ${recovery} ; then
              title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and
Linux %s (recovery mode)")"
           else
    @@ -137,7 +137,6 @@ EOF
            echo    '$message'
            module  ${rel_dirname}/${xenpolicy}
     EOF
    -  fi
       cat << EOF
     }
     EOF

I think these are bugs in the patch in the grub BTS, which were fixed
while iterating over the XSM series in osstest but didn't make it into
the upstream version, the fixes to those bugs are reverted byu the
above. So I have manually reverted them.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei.Lui@citrix.com
Cc: longtaox.pang@intel.com
---
 overlay/etc/grub.d/20_linux_xen |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/overlay/etc/grub.d/20_linux_xen b/overlay/etc/grub.d/20_linux_xen
index 5315e2a..aaead1b 100755
--- a/overlay/etc/grub.d/20_linux_xen
+++ b/overlay/etc/grub.d/20_linux_xen
@@ -1,7 +1,7 @@
 #! /bin/sh
 
 # Copied from the identically named file in grub-common 1.99-27+deb7u2.
-# This version fixed Debian bug #690538 and GRUB bug #43420.
+# This version fixes GRUB bug #43420.
 
 set -e
 
@@ -173,6 +173,7 @@ while [ "x${xen_list}" != "x" ] ; do
     xen_dirname=`dirname ${current_xen}`
     rel_xen_dirname=`make_system_path_relative_to_its_root $xen_dirname`
     xen_version=`echo $xen_basename | sed -e "s,.gz$,,g;s,^xen-,,g"`
+    echo "submenu \"Xen ${xen_version}\" {"
     while [ "x$list" != "x" ] ; do
 	linux=`version_find_latest $list`
 	echo "Found linux image: $linux" >&2
@@ -214,5 +215,6 @@ while [ "x${xen_list}" != "x" ] ; do
 
 	list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`
     done
+    echo "}"
     xen_list=`echo $xen_list | tr ' ' '\n' | grep -vx $current_xen | tr '\n' ' '`
 done
-- 
1.7.10.4

[OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive

[longtao.pang]

Now auto-gen kernel grub2 config file's boot menu entries can have
2-level hierarchy, containing 'submenu' primitive, which is comprised by
several sub-menuentries. Xen boot entries are grouped into such kind of
'submenu' block. This patch adds setupboot_grub2() ability to handle
such new grub.cfg format

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v11:
1. Ian corrects our previous implementation of submenu handling. When
selecting submenu's menuentry, it shall be the 2-level format of 'M>N',
M is the 'submenu' entry number in first level, N is the sub-menuentry
numbering inside 'submenu' block, starting from 0.
Ian's correcting patch:
http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg03108.html.
Based on Ian's correcting patch, we dogmatically alternatively use
'pop/push' to handle array of @offsets.
---
 Osstest/Debian.pm |   24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 46388d8..b1d1043 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -405,12 +405,22 @@ sub setupboot_grub2 ($$$$) {
     my $parsemenu= sub {
         my $f= bl_getmenu_open($ho, $rmenu, "$stash/$ho->{Name}--grub.cfg.1");
     
-        my $count= 0;
+        my @offsets = (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;
+                    pop @offsets;
+                    $offsets[$#offsets]++;
+                    next;
+                }
                 my (@missing) =
                     grep { !defined $entry->{$_} } 
 		        (defined $xenhopt
@@ -438,8 +448,12 @@ sub setupboot_grub2 ($$$$) {
             }
             if (m/^menuentry\s+[\'\"](.*)[\'\"].*\{\s*$/) {
                 die $entry->{StartLine} if $entry;
-                $entry= { Title => $1, StartLine => $., Number => $count };
-                $count++;
+                $entry= { Title => $1, StartLine => $., MenuEntryPath => join ">", @offsets };
+                $offsets[$#offsets]++;
+            }
+            if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
+                $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
+                push @offsets,(0);
             }
             if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
                 die unless $entry;
@@ -500,7 +514,7 @@ sub setupboot_grub2 ($$$$) {
             }
             print ::EO <<END or die $!;
 
-GRUB_DEFAULT=$entry->{Number}
+GRUB_DEFAULT="$entry->{MenuEntryPath}"
 END
 
             print ::EO <<END or die $! if defined $xenhopt;
-- 
1.7.10.4

[OSSTEST Nested PATCH v11 3/7] Changes to support '/boot' leading paths of kernel, xen, in grub

[longtao.pang]

Support situations of grub that have vmlinuz and other things starting
with path of '/boot' rather than '/'.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
 Osstest/Debian.pm |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index b1d1043..62c4061 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -455,21 +455,21 @@ sub setupboot_grub2 ($$$$) {
                 $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
                 push @offsets,(0);
             }
-            if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
+            if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
                 die unless $entry;
                 $entry->{Hv}= $1;
             }
-            if (m/^\s*multiboot\s*\/(vmlinu[xz]-(\S+))/) {
+            if (m/^\s*multiboot\s*(?:\/boot)?\/(vmlinu[xz]-(\S+))/) {
                 die unless $entry;
                 $entry->{KernOnly}= $1;
                 $entry->{KernVer}= $2;
             }
-            if (m/^\s*module\s*\/(vmlinu[xz]-(\S+))/) {
+            if (m/^\s*module\s*(?:\/boot)?\/(vmlinu[xz]-(\S+))/) {
                 die unless $entry;
                 $entry->{KernDom0}= $1;
                 $entry->{KernVer}= $2;
             }
-            if (m/^\s*module\s*\/(initrd\S+)/) {
+            if (m/^\s*module\s*(?:\/boot)?\/(initrd\S+)/) {
                 $entry->{Initrd}= $1;
             }
 	    if (m/^\s*module\s*\/(xenpolicy\S+)/) {
-- 
1.7.10.4

[OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[longtao.pang]

1. The default disk size for guest is '10000M' which is not sufficient
for nested HVM guest, using larger disk size for nested guest
to accommodate to nested test requirement, the specific disk_size is
defined by make-flight.
2. In L1 installation context, assign more memory (defined in runvar) to
it; Since it acts as a nested hypervisor anyway.
3. Comment out CDROM entry in sources.list to make HTTP URL entry
available for L1 hvm guest.
4. Enable nestedhvm feature in 'ExtraConfig' for nested job.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
---
Changes in v11:
1. Since the size of debian-7.2.0-amd64-CD-1.iso is just 624M, 
it's no need to extend nested L1's rootfs size, keep the original
size as '5000M'.
2. Add another use of preseed_hook_command to add new command of 
"in-target sed -i 's/^deb *cdrom/#&/g' /etc/apt/sources.list".
---
 ts-debian-hvm-install |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index ea2d1ad..66ba5b0 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -92,6 +92,13 @@ if [ -e \$b/debian/grubx64.efi ] ; then
 fi
 END
 
+preseed_hook_command($gho, 'late_command', '', <<END);
+#!/bin/sh
+set -ex
+
+in-target sed -i 's/^deb *cdrom/#&/g' /etc/apt/sources.list
+END
+
     $preseed_file .= preseed_hook_cmds();
 
     return $preseed_file;
@@ -166,6 +173,10 @@ sub prep () {
     target_putfilecontents_root_stash($ho, 10, preseed(),
                                       $preseed_file_path);
 
+    my $extra_config='';
+    $extra_config .="nestedhvm=1\n"
+        if guest_var($gho,"enable_nestedhvm",'false') eq 'true';
+
     # If host has >8G free memory, create a guest with 4G memory to catch
     # any error that triggers cross 4G boundary
     my $host_freemem_mb = host_get_free_memory($ho);
@@ -174,13 +185,18 @@ sub prep () {
     if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
         $ram_mb = $ram_lots;
     } else {
-        $ram_mb = 768;
+        # Use guest_var to get specific memsize, or will use default '768'
+        $ram_mb= guest_var($gho,'memsize',768);
     }
     logm("Host has $host_freemem_mb MB free memory, setting guest memory size to $ram_mb MB");
 
+    # Use guest_var to get specific disk size, or will use default $disk_mb
+    $disk_mb= guest_var($gho,'disksize',$disk_mb);
+
     more_prepareguest_hvm($ho,$gho, $ram_mb, $disk_mb,
                           OnReboot => 'preserve',
                           Bios => $r{bios},
+                          ExtraConfig => $extra_config,
                           PostImageHook => sub {
         my $cmds = iso_copy_content_from_image($gho, $newiso);
         $cmds .= prepare_initrd($initrddir,$newiso,$preseed_file_path);
-- 
1.7.10.4

[OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration

[longtao.pang]

1. In this script, make some appropriate runvars which selecthost would
recognise.
2. Prepare the configurations for installing L2 guest VM.
3. Create a lv disk in L0 and hot-attach it to L1; Inside L1, using this
new added disk to create a VG which will be used for installing L2 guest.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 ts-nested-setup |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100755 ts-nested-setup

diff --git a/ts-nested-setup b/ts-nested-setup
new file mode 100755
index 0000000..60ab795
--- /dev/null
+++ b/ts-nested-setup
@@ -0,0 +1,75 @@
+#!/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();
+# Pass L0's ident and L1's guestname
+our ($l0_ident,$l1_gn) = @ARGV;
+our ($l0,$l1) = ts_get_host_guest($l0_ident,$l1_gn);
+
+guest_check_ip($l1);
+
+# L1 guest's ident is same as guestname
+our $l1_ident = $l1->{Guest};
+
+store_runvar($l1_ident,$l1->{Guest});
+store_runvar("${l1_ident}_ip",$l1->{Ip});
+
+target_cmd_root($l1, "update-rc.d osstest-confirm-booted start 99 2 .");
+
+target_install_packages_norec($l1, qw(lvm2 rsync genisoimage));
+
+# We need to attach an extra disk to the L1 guest to be used as L2
+# guest storage.
+#
+# When running in a nested HVM environment the L1 domain is acting
+# as both a guest to L0 and a host to L2 guests and therefore potentially
+# sees connections to two independent xenstore instances, one provided by
+# the L0 host and one which is provided by the L1 instance of xenstore.
+#
+# Unfortunately the kernel is not capable of dealing with this and is only
+# able to cope with a single xenstore connection. Since the L1 toolstack and
+# L2 guests absolutely require xenstore to function we therefore cannot use
+# the L0 xenstore and therefore cannot use PV devices (xvdX etc) in the L1
+# guest and must use emulated devices (sdX etc).
+#
+# However at the moment we have not yet rebooted L1 into Xen and so it does
+# have PV devices available and sdb actually appears as xvdb. We could
+# disable the Xen platform device and use emulated devices for the install
+# phase too but that would be needlessly slow.
+
+my $vgname = $l1->{Vg};
+my $guest_storage_lv_name = "${l1_ident}_guest_storage";
+my $guest_storage_lv_size = guest_var($l1,'guest_storage_size',undef);
+die "guest_storage_lv_size is undefined" unless $guest_storage_lv_size;
+my $guest_storage_lvdev = "/dev/${vgname}/${guest_storage_lv_name}";
+
+die "toolstack $r{toolstack}" unless $r{toolstack} eq "xl";
+target_cmd_root($l0, <<END);
+    lvremove -f $guest_storage_lvdev ||:
+    lvcreate -L ${guest_storage_lv_size}M -n $guest_storage_lv_name $vgname
+    dd if=/dev/zero of=$guest_storage_lvdev count=10
+    xl block-attach $l1->{Name} ${guest_storage_lvdev},raw,sdb,rw
+END
+
+# Create a vg in L1 guest and vg name is ${l1_gn}-disk
+target_cmd_root($l1, "pvcreate /dev/xvdb && vgcreate ${l1_gn}-disk /dev/xvdb");
-- 
1.7.10.4

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

[longtao.pang]

The ident and guestname are same of 'nestedl1' for L1 guest VM.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 sg-run-job |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sg-run-job b/sg-run-job
index d53fd83..679761a 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -299,6 +299,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 + nestedl1
+    run-ts . = ts-nested-setup + host + nestedl1
+    run-ts . = ts-xen-install nestedl1
+    run-ts . = ts-host-reboot nestedl1
+    run-ts . = ts-debian-hvm-install nestedl1 nestedl2
+    run-ts . = ts-guest-stop nestedl1 nestedl2
+    run-ts . = ts-guest-destroy + host + nestedl1
+}
+
 proc test-guest-migr {g} {
     if {[catch { run-ts . = ts-migrate-support-check + host $g }]} return
 
-- 
1.7.10.4

[OSSTEST Nested PATCH v11 7/7] Add test job for nest test case

[longtao.pang]

1. This patch adds creation of the nested test job, when job creation
procedure is invoked.
2. Set nested L1's vif model, nestedhvm feature, set specific disk
size and memory size for nested test by make-flight.

Signed-off-by: longtao.pang <longtaox.pang@intel.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
 make-flight |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/make-flight b/make-flight
index 8a1fceb..318c2db 100755
--- a/make-flight
+++ b/make-flight
@@ -210,6 +210,36 @@ do_hvm_win7_x64_tests () {
             all_hostflags=$most_hostflags,hvm
 }
 
+do_hvm_debian_nested_tests () {
+  bios=$1
+
+  if [ $xenarch != amd64 -o $dom0arch != amd64 \
+      -o "x$qemuu_suffix" != "x-qemuu" ]; then
+    return
+  fi
+
+  case $xenbranch in
+    xen-3.*-testing)      return;;
+    xen-4.0-testing)      return;;
+    xen-4.1-testing)      return;;
+    xen-4.2-testing)      return;;
+    xen-4.3-testing)      return;;
+  esac
+
+  job_create_test test-$xenarch$kern-$dom0arch$qemuu_suffix-nested \
+            test-nested xl $xenarch $dom0arch $qemuu_runvar \
+            nestedl1_image=debian-7.2.0-amd64-CD-1.iso \
+            nestedl1_vifmodel='e1000' \
+            nestedl1_disksize='15000' \
+            nestedl1_memsize='3072' \
+            nestedl1_enable_nestedhvm='true' \
+            nestedl1_guest_storage_size='20000' \
+            nestedl2_image=debian-7.2.0-amd64-CD-1.iso \
+            nestedl2_disksize='15000' \
+            bios=$bios
+            all_hostflags=$most_hostflags,hvm
+}
+
 do_hvm_debian_test_one () {
   testname=$1
   bios=$2
@@ -425,6 +455,7 @@ test_matrix_do_one () {
     do_hvm_rhel6_tests
 
     do_hvm_debian_tests
+    do_hvm_debian_nested_tests seabios
 
   done # qemuu_suffix
 
-- 
1.7.10.4

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

[Ian Campbell]

On Tue, 2015-05-26 at 17:08 +0800, longtao.pang wrote:
> ----------------------------------------------------------------
> Ian Campbell (1):
>       grub: remove patch to disable submenu from 20_linux_xen overlay
> longtao.pang (6):
>       Parsing grub which has 'submenu' primitive
>       Changes to support '/boot' leading paths of kernel, xen, in grub

Thanks, I've applied these three to osstest's pretest. In the end I
didn't think the incremental change to the parsing patch needed Ian's
input once it changed to use push/pop.

In the "Parsing..." patch I made the same coding style fixup as last
time, as well as dropping the comment which I mentioned.

I reordered to apply my grub one last since it relies on the first
patch.

I've pushed the new pretest to:
        git://xenbits.xen.org/people/ianc/osstest.git nestedhvm-v11-pretest 
        
as well. 

I'll review the remainder of v11 shortly.

Ian.

Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Ian Campbell]

On Tue, 2015-05-26 at 17:08 +0800, longtao.pang wrote:
> 1. The default disk size for guest is '10000M' which is not sufficient
> for nested HVM guest, using larger disk size for nested guest
> to accommodate to nested test requirement, the specific disk_size is
> defined by make-flight.
> 2. In L1 installation context, assign more memory (defined in runvar) to
> it; Since it acts as a nested hypervisor anyway.
> 3. Comment out CDROM entry in sources.list to make HTTP URL entry
> available for L1 hvm guest.
> 4. Enable nestedhvm feature in 'ExtraConfig' for nested job.
> 
> Signed-off-by: longtao.pang <longtaox.pang@intel.com>

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

One query:
[...]
> @@ -174,13 +185,18 @@ sub prep () {
>      if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
>          $ram_mb = $ram_lots;
>      } else {
> -        $ram_mb = 768;
> +        # Use guest_var to get specific memsize, or will use default '768'
> +        $ram_mb= guest_var($gho,'memsize',768);

I think this only happens if the host has less than "$ram_lots * 2 +
$ram_minslop" (==10100M) free, otherwise you get $ram_lots (5000M),
which might be less than the runvar asked for...

Perhaps what we really want (maybe in a followup patch is):

        $ram_mb = guest_var($gho,'memsize',undef);
        if (!$ram_mb) {
             if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
                  $ram_mb = $ram_lots;
             } else {
                  $ram_mb = 768;
             }
        }
?

Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Monday, June 08, 2015 6:31 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 v11 4/7] Changes on test step of Debian hvm
> guest install
> 
> On Tue, 2015-05-26 at 17:08 +0800, longtao.pang wrote:
> > 1. The default disk size for guest is '10000M' which is not sufficient
> > for nested HVM guest, using larger disk size for nested guest
> > to accommodate to nested test requirement, the specific disk_size is
> > defined by make-flight.
> > 2. In L1 installation context, assign more memory (defined in runvar) to
> > it; Since it acts as a nested hypervisor anyway.
> > 3. Comment out CDROM entry in sources.list to make HTTP URL entry
> > available for L1 hvm guest.
> > 4. Enable nestedhvm feature in 'ExtraConfig' for nested job.
> >
> > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> One query:
> [...]
> > @@ -174,13 +185,18 @@ sub prep () {
> >      if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
> >          $ram_mb = $ram_lots;
> >      } else {
> > -        $ram_mb = 768;
> > +        # Use guest_var to get specific memsize, or will use default '768'
> > +        $ram_mb= guest_var($gho,'memsize',768);
> 
> I think this only happens if the host has less than "$ram_lots * 2 +
> $ram_minslop" (==10100M) free, otherwise you get $ram_lots (5000M),
> which might be less than the runvar asked for...
> 
For nested job, the specific 'memsize' for L1 guest is 3072M which is defined by make-flight.
If the "$ram_lots" equal to 5000M which is larger than 3072M, that's suitable for nested L1 guest.
The condition for nested L1 guest's memory size that should not be less than 3072M, that's why we 
add the code of "$ram_mb =guest_var($gho,'memsize',768)" here.
> Perhaps what we really want (maybe in a followup patch is):
> 
>         $ram_mb = guest_var($gho,'memsize',undef);
>         if (!$ram_mb) {
>              if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
>                   $ram_mb = $ram_lots;
>              } else {
>                   $ram_mb = 768;
>              }
>         }
> ?
> 
So, I think maybe it's no need to change that.
Please correct me, if I am wrong.

Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Ian Campbell]

On Tue, 2015-06-09 at 05:29 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Monday, June 08, 2015 6:31 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 v11 4/7] Changes on test step of Debian hvm
> > guest install
> > 
> > On Tue, 2015-05-26 at 17:08 +0800, longtao.pang wrote:
> > > 1. The default disk size for guest is '10000M' which is not sufficient
> > > for nested HVM guest, using larger disk size for nested guest
> > > to accommodate to nested test requirement, the specific disk_size is
> > > defined by make-flight.
> > > 2. In L1 installation context, assign more memory (defined in runvar) to
> > > it; Since it acts as a nested hypervisor anyway.
> > > 3. Comment out CDROM entry in sources.list to make HTTP URL entry
> > > available for L1 hvm guest.
> > > 4. Enable nestedhvm feature in 'ExtraConfig' for nested job.
> > >
> > > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > One query:
> > [...]
> > > @@ -174,13 +185,18 @@ sub prep () {
> > >      if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
> > >          $ram_mb = $ram_lots;
> > >      } else {
> > > -        $ram_mb = 768;
> > > +        # Use guest_var to get specific memsize, or will use default '768'
> > > +        $ram_mb= guest_var($gho,'memsize',768);
> > 
> > I think this only happens if the host has less than "$ram_lots * 2 +
> > $ram_minslop" (==10100M) free, otherwise you get $ram_lots (5000M),
> > which might be less than the runvar asked for...
> > 
> For nested job, the specific 'memsize' for L1 guest is 3072M which is defined by make-flight.
> If the "$ram_lots" equal to 5000M which is larger than 3072M, that's suitable for nested L1 guest.
> The condition for nested L1 guest's memory size that should not be less than 3072M, that's why we 
> add the code of "$ram_mb =guest_var($gho,'memsize',768)" here.

I was talking about the general case, which is broken for any guest
configured to have RAM > $ram_lots.


> > Perhaps what we really want (maybe in a followup patch is):
> > 
> >         $ram_mb = guest_var($gho,'memsize',undef);
> >         if (!$ram_mb) {
> >              if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
> >                   $ram_mb = $ram_lots;
> >              } else {
> >                   $ram_mb = 768;
> >              }
> >         }
> > ?
> > 
> So, I think maybe it's no need to change that.
> Please correct me, if I am wrong.

Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, June 09, 2015 4:08 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 v11 4/7] Changes on test step of Debian hvm
> guest install
> 
> On Tue, 2015-06-09 at 05:29 +0000, Pang, LongtaoX wrote:
> >
> >
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > Sent: Monday, June 08, 2015 6:31 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 v11 4/7] Changes on test step of Debian
> hvm
> > > guest install
> > >
> > > On Tue, 2015-05-26 at 17:08 +0800, longtao.pang wrote:
> > > > 1. The default disk size for guest is '10000M' which is not sufficient
> > > > for nested HVM guest, using larger disk size for nested guest
> > > > to accommodate to nested test requirement, the specific disk_size is
> > > > defined by make-flight.
> > > > 2. In L1 installation context, assign more memory (defined in runvar) to
> > > > it; Since it acts as a nested hypervisor anyway.
> > > > 3. Comment out CDROM entry in sources.list to make HTTP URL entry
> > > > available for L1 hvm guest.
> > > > 4. Enable nestedhvm feature in 'ExtraConfig' for nested job.
> > > >
> > > > Signed-off-by: longtao.pang <longtaox.pang@intel.com>
> > >
> > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > >
> > > One query:
> > > [...]
> > > > @@ -174,13 +185,18 @@ sub prep () {
> > > >      if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
> > > >          $ram_mb = $ram_lots;
> > > >      } else {
> > > > -        $ram_mb = 768;
> > > > +        # Use guest_var to get specific memsize, or will use default '768'
> > > > +        $ram_mb= guest_var($gho,'memsize',768);
> > >
> > > I think this only happens if the host has less than "$ram_lots * 2 +
> > > $ram_minslop" (==10100M) free, otherwise you get $ram_lots (5000M),
> > > which might be less than the runvar asked for...
> > >
> > For nested job, the specific 'memsize' for L1 guest is 3072M which is defined by
> make-flight.
> > If the "$ram_lots" equal to 5000M which is larger than 3072M, that's suitable
> for nested L1 guest.
> > The condition for nested L1 guest's memory size that should not be less than
> 3072M, that's why we
> > add the code of "$ram_mb =guest_var($gho,'memsize',768)" here.
> 
> I was talking about the general case, which is broken for any guest
> configured to have RAM > $ram_lots.
> 
Thanks Ian, I get your point now. 
> 
> > > Perhaps what we really want (maybe in a followup patch is):
> > >
> > >         $ram_mb = guest_var($gho,'memsize',undef);
> > >         if (!$ram_mb) {
> > >              if ($host_freemem_mb > $ram_lots * 2 + $ram_minslop) {
> > >                   $ram_mb = $ram_lots;
> > >              } else {
> > >                   $ram_mb = 768;
> > >              }
> > >         }
> > > ?
> > >
> > So, I think maybe it's no need to change that.
> > Please correct me, if I am wrong.
> 
Yes, your above patch is more suitable here and it's necessary to change the code.
Could you please change this query in your branch, so that I will not summit another version of nested patch to you?

Re: [OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive

[Ian Jackson]

longtao.pang writes ("[OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive"):
> Now auto-gen kernel grub2 config file's boot menu entries can have
> 2-level hierarchy, containing 'submenu' primitive, which is comprised by
> several sub-menuentries. Xen boot entries are grouped into such kind of
> 'submenu' block. This patch adds setupboot_grub2() ability to handle
> such new grub.cfg format

Parsing submenus is good, but I have some style quibbles with your
patch I'm afraid.

>          while (<$f>) {
>              next if m/^\s*\#/ || !m/\S/;
>              if (m/^\s*\}\s*$/) {
> -                die unless $entry;
> +                die unless $entry || $submenu;
> +                if(!defined $entry && defined $submenu){

$entry and $submenu are either some ref, or undef, so you don't need
`defined'.  You can just use them as if they were booleans in the if,
just as you did above.  (I'm not sure that `!defined $entry' adds much
given the previous line, but that's maybe a matter or taste.)

Missing spaces after if and inside `){'.

> +                    logm("Met end of a submenu starting from ".
> +                        "$submenu->{StartLine}. ".
> +                        "Our want kern is $want_kernver");
> +                    $submenu=undef;

Missing space after or around `='.  (Other places too.)

> +                    pop @offsets;
> +                    $offsets[$#offsets]++;
> +                    next;
> +                }
>                  my (@missing) =
>                      grep { !defined $entry->{$_} } 
>  		        (defined $xenhopt
> @@ -438,8 +448,12 @@ sub setupboot_grub2 ($$$$) {
>              }
>              if (m/^menuentry\s+[\'\"](.*)[\'\"].*\{\s*$/) {
>                  die $entry->{StartLine} if $entry;
> -                $entry= { Title => $1, StartLine => $., Number => $count };
> -                $count++;
> +                $entry= { Title => $1, StartLine => $., MenuEntryPath => join ">", @offsets };

Needs rewrapping.  Probably best to expand the { } into a multi-line
structure.

> +                $offsets[$#offsets]++;
> +            }
> +            if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> +                $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };

Unless I'm mistaken, the MenuEntryPath of a $submenu is never used ?
Not setting it would avoid (a) a need to rewrap and (b) me complaining
that you have open-coded the join twice.

Thanks,
Ian.

Re: [OSSTEST Nested PATCH v11 3/7] Changes to support '/boot' leading paths of kernel, xen, in grub

[Ian Jackson]

longtao.pang writes ("[OSSTEST Nested PATCH v11 3/7] Changes to support '/boot' leading paths of kernel, xen, in grub"):
> Support situations of grub that have vmlinuz and other things starting
> with path of '/boot' rather than '/'.

The insertion of (?:\/boot) is fine, but:

> -            if (m/^\s*multiboot\s*\/(xen\-[0-9][-+.0-9a-z]*\S+)/) {
> +            if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {

Here you change the regexp for the xen image, without any explanation.

Ian.

Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Ian Jackson]

longtao.pang writes ("[OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install"):
...
> 1. The default disk size for guest is '10000M' which is not sufficient
> for nested HVM guest, using larger disk size for nested guest
> to accommodate to nested test requirement, the specific disk_size is
> defined by make-flight.
> 2. In L1 installation context, assign more memory (defined in runvar) to
> it; Since it acts as a nested hypervisor anyway.
> 3. Comment out CDROM entry in sources.list to make HTTP URL entry
> available for L1 hvm guest.
> 4. Enable nestedhvm feature in 'ExtraConfig' for nested job.

I think this should be 3 separate patches:

  - Allow runvars to specify guest disk and ram size (turning
    previous values into defaults)

  - Comment out CDROM entry.  This needs better motivation.  I think
    your motivation is that the CDROM is removed and that therefore
    this line does not work ?

  - Honour $xopts{ExtraConfig} and use it to enable nestedhvm.


> +    my $extra_config='';
> +    $extra_config .="nestedhvm=1\n"
> +        if guest_var($gho,"enable_nestedhvm",'false') eq 'true';

Idiom elsewhere is   =~ m/true/   rather than  eq 'true'
although maybe Wei will provide a guest_var_boolean (see my comments
on his "[PATCH OSSTEST v3] Stubdom test case".

> +    # Use guest_var to get specific disk size, or will use default $disk_mb
> +    $disk_mb= guest_var($gho,'disksize',$disk_mb);

I would prefer this comment and the one about RAM to be expressed,
instead, like this:

    sub more_prepareguest_hvm ($$$$;@) {
        my ($ho, $gho, $ram_mb, $disk_mb, %xopts) = @_;
  +     # $ram_mb and $disk_mb are defaults, used if runvars don't say


Thanks,
Ian.

Re: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration

[Ian Jackson]

longtao.pang writes ("[OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration"):
> 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; Inside L1, using this
> new added disk to create a VG which will be used for installing L2 guest.
...

Thanks.  This is roughly the right approach.  I have some minor
comments.


> +target_cmd_root($l1, "update-rc.d osstest-confirm-booted start 99 2 .");

This is an open coded copy of the code from ts-host-install.  It
should be broken out, I think.


> +target_install_packages_norec($l1, qw(lvm2 rsync genisoimage));

Do we need to install genisoimage here ?  The guest install scripts do
it.  Or are we just doing it here for efficiency reasons ?


> +# We need to attach an extra disk to the L1 guest to be used as L2
> +# guest storage.
> +#
> +# When running in a nested HVM environment the L1 domain is acting
> +# as both a guest to L0 and a host to L2 guests and therefore potentially
> +# sees connections to two independent xenstore instances, one provided by
> +# the L0 host and one which is provided by the L1 instance of xenstore.
> +#
> +# Unfortunately the kernel is not capable of dealing with this and is only
> +# able to cope with a single xenstore connection. Since the L1 toolstack and
> +# L2 guests absolutely require xenstore to function we therefore cannot use
> +# the L0 xenstore and therefore cannot use PV devices (xvdX etc) in the L1
> +# guest and must use emulated devices (sdX etc).
> +#
> +# However at the moment we have not yet rebooted L1 into Xen and so it does
> +# have PV devices available and sdb actually appears as xvdb. We could
> +# disable the Xen platform device and use emulated devices for the install
> +# phase too but that would be needlessly slow.
> +
> +my $vgname = $l1->{Vg};
> +my $guest_storage_lv_name = "${l1_ident}_guest_storage";
> +my $guest_storage_lv_size = guest_var($l1,'guest_storage_size',undef);
> +die "guest_storage_lv_size is undefined" unless $guest_storage_lv_size;
> +my $guest_storage_lvdev = "/dev/${vgname}/${guest_storage_lv_name}";
> +
> +die "toolstack $r{toolstack}" unless $r{toolstack} eq "xl";
> +target_cmd_root($l0, <<END);
> +    lvremove -f $guest_storage_lvdev ||:
> +    lvcreate -L ${guest_storage_lv_size}M -n $guest_storage_lv_name $vgname
> +    dd if=/dev/zero of=$guest_storage_lvdev count=10
> +    xl block-attach $l1->{Name} ${guest_storage_lvdev},raw,sdb,rw
> +END
> +
> +# Create a vg in L1 guest and vg name is ${l1_gn}-disk
> +target_cmd_root($l1, "pvcreate /dev/xvdb && vgcreate ${l1_gn}-disk /dev/xvdb");

We would avoid having to mention /dev/xvdb if we created the VG in the
host, before doing block-attach.  I'm not sure whether that's an
improvement.


Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

longtao.pang writes ("[OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> The ident and guestname are same of 'nestedl1' for L1 guest VM.

This is going in the right direction.

I think arrangements need to be made to capture the logs for
nestedl1.

> +proc need-hosts/test-nested {} {return host}
> +proc run-job/test-nested {} {
> +    run-ts . = ts-debian-hvm-install + host + nestedl1
> +    run-ts . = ts-nested-setup + host + nestedl1
> +    run-ts . = ts-xen-install nestedl1
> +    run-ts . = ts-host-reboot nestedl1
> +    run-ts . = ts-debian-hvm-install nestedl1 nestedl2
> +    run-ts . = ts-guest-stop nestedl1 nestedl2
> +    run-ts . = ts-guest-destroy + host + nestedl1
> +}

Very much a matter of taste, but can I suggest that `l1' and `l2'
might be better idents/guest names names ?

I think some extra +s in the l2 install and start operations might be
useful, because the testid probably doesn't need to mention nestedl1.

I think you probably want to run leak-check on the L1.

Do you not want to run the steps in test-guest ?  Perhaps it would be
better to add a host ident argument to test-guest{,-migr,-nomigr} ?

Thanks,
Ian.

Re: [OSSTEST Nested PATCH v11 7/7] Add test job for nest test case

[Ian Jackson]

longtao.pang writes ("[OSSTEST Nested PATCH v11 7/7] Add test job for nest test case"):
> 1. This patch adds creation of the nested test job, when job creation
> procedure is invoked.
> 2. Set nested L1's vif model, nestedhvm feature, set specific disk
> size and memory size for nested test by make-flight.
...

This is nearly right, I think.

> +  job_create_test test-$xenarch$kern-$dom0arch$qemuu_suffix-nested \
> +            test-nested xl $xenarch $dom0arch $qemuu_runvar \
> +            nestedl1_image=debian-7.2.0-amd64-CD-1.iso \
> +            nestedl1_vifmodel='e1000' \
> +            nestedl1_disksize='15000' \
> +            nestedl1_memsize='3072' \
> +            nestedl1_enable_nestedhvm='true' \
> +            nestedl1_guest_storage_size='20000' \
> +            nestedl2_image=debian-7.2.0-amd64-CD-1.iso \
> +            nestedl2_disksize='15000' \
> +            bios=$bios
> +            all_hostflags=$most_hostflags,hvm

Some observations:
 * The \ should be aligned to a column at the RHS.  If you did that
   you would have noticed that:
 * You are missing a \
 * Most of the '' are unnecessary.
 * Why do you specify the l2 disk size explicitly ?

Thanks,
Ian.

Re: [OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive

[Robert Hu]

On Wed, 2015-06-10 at 14:30 +0100, Ian Jackson wrote:
> longtao.pang writes ("[OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive"):
> > Now auto-gen kernel grub2 config file's boot menu entries can have
> > 2-level hierarchy, containing 'submenu' primitive, which is comprised by
> > several sub-menuentries. Xen boot entries are grouped into such kind of
> > 'submenu' block. This patch adds setupboot_grub2() ability to handle
> > such new grub.cfg format
> 
> Parsing submenus is good, but I have some style quibbles with your
> patch I'm afraid.
> 
> >          while (<$f>) {
> >              next if m/^\s*\#/ || !m/\S/;
> >              if (m/^\s*\}\s*$/) {
> > -                die unless $entry;
> > +                die unless $entry || $submenu;
> > +                if(!defined $entry && defined $submenu){
> 
> $entry and $submenu are either some ref, or undef, so you don't need
> `defined'.  You can just use them as if they were booleans in the if,
> just as you did above.  (I'm not sure that `!defined $entry' adds much
> given the previous line, but that's maybe a matter or taste.)
> 
> Missing spaces after if and inside `){'.
OK, to refine these.
> 
> > +                    logm("Met end of a submenu starting from ".
> > +                        "$submenu->{StartLine}. ".
> > +                        "Our want kern is $want_kernver");
> > +                    $submenu=undef;
> 
> Missing space after or around `='.  (Other places too.)
Wasn't aware of this rule. To refine this in next version patch.
> 
> > +                    pop @offsets;
> > +                    $offsets[$#offsets]++;
> > +                    next;
> > +                }
> >                  my (@missing) =
> >                      grep { !defined $entry->{$_} } 
> >  		        (defined $xenhopt
> > @@ -438,8 +448,12 @@ sub setupboot_grub2 ($$$$) {
> >              }
> >              if (m/^menuentry\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> >                  die $entry->{StartLine} if $entry;
> > -                $entry= { Title => $1, StartLine => $., Number => $count };
> > -                $count++;
> > +                $entry= { Title => $1, StartLine => $., MenuEntryPath => join ">", @offsets };
> 
> Needs rewrapping.  Probably best to expand the { } into a multi-line
> structure.
OK, to divide it into 3 lines.
> 
> > +                $offsets[$#offsets]++;
> > +            }
> > +            if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> > +                $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> 
> Unless I'm mistaken, the MenuEntryPath of a $submenu is never used ?
> Not setting it would avoid (a) a need to rewrap and (b) me complaining
> that you have open-coded the join twice.
Actually this contribution from Ian Campbell.
Hi Ian C., would you agree if I simply remove the 'MenuEntryPath' here? 
> 
> Thanks,
> Ian.

Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, June 10, 2015 9:41 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> Robert
> Subject: Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm
> guest install
> 
> longtao.pang writes ("[OSSTEST Nested PATCH v11 4/7] Changes on test step of
> Debian hvm guest install"):
> ...
> > 1. The default disk size for guest is '10000M' which is not sufficient
> > for nested HVM guest, using larger disk size for nested guest
> > to accommodate to nested test requirement, the specific disk_size is
> > defined by make-flight.
> > 2. In L1 installation context, assign more memory (defined in runvar) to
> > it; Since it acts as a nested hypervisor anyway.
> > 3. Comment out CDROM entry in sources.list to make HTTP URL entry
> > available for L1 hvm guest.
> > 4. Enable nestedhvm feature in 'ExtraConfig' for nested job.
> 
> I think this should be 3 separate patches:
> 
>   - Allow runvars to specify guest disk and ram size (turning
>     previous values into defaults)
> 
>   - Comment out CDROM entry.  This needs better motivation.  I think
>     your motivation is that the CDROM is removed and that therefore
>     this line does not work ?
> 
>   - Honour $xopts{ExtraConfig} and use it to enable nestedhvm.
> 
OK, we will try it.
> 
> > +    my $extra_config='';
> > +    $extra_config .="nestedhvm=1\n"
> > +        if guest_var($gho,"enable_nestedhvm",'false') eq 'true';
> 
> Idiom elsewhere is   =~ m/true/   rather than  eq 'true'
> although maybe Wei will provide a guest_var_boolean (see my comments
> on his "[PATCH OSSTEST v3] Stubdom test case".
> 
Yes, we will try it.
> > +    # Use guest_var to get specific disk size, or will use default $disk_mb
> > +    $disk_mb= guest_var($gho,'disksize',$disk_mb);
> 
> I would prefer this comment and the one about RAM to be expressed,
> instead, like this:
> 
>     sub more_prepareguest_hvm ($$$$;@) {
>         my ($ho, $gho, $ram_mb, $disk_mb, %xopts) = @_;
>   +     # $ram_mb and $disk_mb are defaults, used if runvars don't say
> 
I am sorry, do you mean that I should add another more comment you preferred 
in 'more_prepareguest_hvm ($$$$;@)' function inside TestSupport.pm?
> 
> Thanks,
> Ian.

Re: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, June 10, 2015 9:59 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> Robert
> Subject: Re: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested
> test configuration
> 
> longtao.pang writes ("[OSSTEST Nested PATCH v11 5/7] Add new script to
> customize nested test configuration"):
> > 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; Inside L1, using this
> > new added disk to create a VG which will be used for installing L2 guest.
> ...
> 
> Thanks.  This is roughly the right approach.  I have some minor
> comments.
> 
> 
> > +target_cmd_root($l1, "update-rc.d osstest-confirm-booted start 99 2 .");
> 
> This is an open coded copy of the code from ts-host-install.  It
> should be broken out, I think.
> 
I am sorry, could you please make it more clear? Thanks.

> 
> > +target_install_packages_norec($l1, qw(lvm2 rsync genisoimage));
> 
> Do we need to install genisoimage here ?  The guest install scripts do
> it.  Or are we just doing it here for efficiency reasons ?
In fact, there is no need to install ' genisoimage' here.
It's just for efficiency reason, since if install this package here, it will not 
be installed in 'ts-debian-hvm-install' script again.
So, do you prefer to discard it or keep it here? I think there is no harmful to
install this package here.
> 
> > +# We need to attach an extra disk to the L1 guest to be used as L2
> > +# guest storage.
> > +#
> > +# When running in a nested HVM environment the L1 domain is acting
> > +# as both a guest to L0 and a host to L2 guests and therefore potentially
> > +# sees connections to two independent xenstore instances, one provided by
> > +# the L0 host and one which is provided by the L1 instance of xenstore.
> > +#
> > +# Unfortunately the kernel is not capable of dealing with this and is only
> > +# able to cope with a single xenstore connection. Since the L1 toolstack and
> > +# L2 guests absolutely require xenstore to function we therefore cannot use
> > +# the L0 xenstore and therefore cannot use PV devices (xvdX etc) in the L1
> > +# guest and must use emulated devices (sdX etc).
> > +#
> > +# However at the moment we have not yet rebooted L1 into Xen and so it does
> > +# have PV devices available and sdb actually appears as xvdb. We could
> > +# disable the Xen platform device and use emulated devices for the install
> > +# phase too but that would be needlessly slow.
> > +
> > +my $vgname = $l1->{Vg};
> > +my $guest_storage_lv_name = "${l1_ident}_guest_storage";
> > +my $guest_storage_lv_size = guest_var($l1,'guest_storage_size',undef);
> > +die "guest_storage_lv_size is undefined" unless $guest_storage_lv_size;
> > +my $guest_storage_lvdev = "/dev/${vgname}/${guest_storage_lv_name}";
> > +
> > +die "toolstack $r{toolstack}" unless $r{toolstack} eq "xl";
> > +target_cmd_root($l0, <<END);
> > +    lvremove -f $guest_storage_lvdev ||:
> > +    lvcreate -L ${guest_storage_lv_size}M -n $guest_storage_lv_name
> $vgname
> > +    dd if=/dev/zero of=$guest_storage_lvdev count=10
> > +    xl block-attach $l1->{Name} ${guest_storage_lvdev},raw,sdb,rw
> > +END
> > +
> > +# Create a vg in L1 guest and vg name is ${l1_gn}-disk
> > +target_cmd_root($l1, "pvcreate /dev/xvdb && vgcreate ${l1_gn}-disk
> /dev/xvdb");
> 
> We would avoid having to mention /dev/xvdb if we created the VG in the
> host, before doing block-attach.  I'm not sure whether that's an
> improvement.
> 
I am sorry, I cannot get your point. Could you please make it more clear? Thanks.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 7/7] Add test job for nest test case

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, June 10, 2015 11:46 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> Robert
> Subject: Re: [OSSTEST Nested PATCH v11 7/7] Add test job for nest test case
> 
> longtao.pang writes ("[OSSTEST Nested PATCH v11 7/7] Add test job for nest test
> case"):
> > 1. This patch adds creation of the nested test job, when job creation
> > procedure is invoked.
> > 2. Set nested L1's vif model, nestedhvm feature, set specific disk
> > size and memory size for nested test by make-flight.
> ...
> 
> This is nearly right, I think.
> 
> > +  job_create_test test-$xenarch$kern-$dom0arch$qemuu_suffix-nested \
> > +            test-nested xl $xenarch $dom0arch $qemuu_runvar \
> > +            nestedl1_image=debian-7.2.0-amd64-CD-1.iso \
> > +            nestedl1_vifmodel='e1000' \
> > +            nestedl1_disksize='15000' \
> > +            nestedl1_memsize='3072' \
> > +            nestedl1_enable_nestedhvm='true' \
> > +            nestedl1_guest_storage_size='20000' \
> > +            nestedl2_image=debian-7.2.0-amd64-CD-1.iso \
> > +            nestedl2_disksize='15000' \
> > +            bios=$bios
> > +            all_hostflags=$most_hostflags,hvm
> 
> Some observations:
>  * The \ should be aligned to a column at the RHS.  If you did that
>    you would have noticed that:
>  * You are missing a \
>  * Most of the '' are unnecessary.
Thanks for pointing out these.
>  * Why do you specify the l2 disk size explicitly ?
Just for some configure rule in our lab, we assign at least '15000M' disk size for nested L2 guest, 
since it's no harmful, I think.
Actually, it works fine if not specify the l2 disk size and using default '10000M'.
So, you prefer to use default disk size for l2 guest, right?
> Thanks,
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, June 10, 2015 11:42 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> Robert
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> longtao.pang writes ("[OSSTEST Nested PATCH v11 6/7] Compose the main recipe
> of nested test job"):
> > The ident and guestname are same of 'nestedl1' for L1 guest VM.
> 
> This is going in the right direction.
> 
> I think arrangements need to be made to capture the logs for
> nestedl1.
> 
I am sorry, what logs do you mean? I noticed that there is one 'logs/standalone' directory 
inside osstest repo. There are job logs that already have run.

> > +proc need-hosts/test-nested {} {return host}
> > +proc run-job/test-nested {} {
> > +    run-ts . = ts-debian-hvm-install + host + nestedl1
> > +    run-ts . = ts-nested-setup + host + nestedl1
> > +    run-ts . = ts-xen-install nestedl1
> > +    run-ts . = ts-host-reboot nestedl1
> > +    run-ts . = ts-debian-hvm-install nestedl1 nestedl2
> > +    run-ts . = ts-guest-stop nestedl1 nestedl2
> > +    run-ts . = ts-guest-destroy + host + nestedl1
> > +}
> 
> Very much a matter of taste, but can I suggest that `l1' and `l2'
> might be better idents/guest names names ?
> 
Yes, I will try it.

> I think some extra +s in the l2 install and start operations might be
> useful, because the testid probably doesn't need to mention nestedl1.
> 
I am sorry, do you mean that I should add '+' for l2 installation, 
such as ' ts-debian-hvm-install + nestedl1 + nestedl2' ? 

> I think you probably want to run leak-check on the L1.
> 
I have noticed the logs which printed on screen during job running, the leak-check will
be run on L1 at the end of testing.
Or, do you mean that I should run leak-check on L1 after the L1 HVM guest finishing installation?

> Do you not want to run the steps in test-guest ?  Perhaps it would be
> better to add a host ident argument to test-guest{,-migr,-nomigr} ?
> 
We do not plan to run the steps in test-guest currently.
> Thanks,
> Ian.

Re: [OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive

[Ian Campbell]

On Thu, 2015-06-11 at 11:17 +0800, Robert Hu wrote:

> > Unless I'm mistaken, the MenuEntryPath of a $submenu is never used ?
> > Not setting it would avoid (a) a need to rewrap and (b) me complaining
> > that you have open-coded the join twice.
> Actually this contribution from Ian Campbell.
> Hi Ian C., would you agree if I simply remove the 'MenuEntryPath' here? 

I left it as a debugging aid, since it shows up in Dumper($submenu)
which is convenient to sprinkle around while debuggiung. I don't mind if
it is removed or kept though.

Note that several patches from this series are already in osstest
production:

b77a6a2 Changes to support '/boot' leading paths of kernel, xen, in grub
997385f Parsing grub which has 'submenu' primitive
155bdb3 Move the code for setting memory size into prep()
2545fc6 Edit some APIs in TestSupport.pm for nested test
699c911 Refactor installation of overlays

So a incremental patch is what is needed here.

I fixed one or two issues as I committed, e.g. :
> > Missing spaces after if and inside `){'.
> OK, to refine these.

Worth double checking which I caught though.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Thu, 2015-06-11 at 07:41 +0000, Pang, LongtaoX wrote:
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Wednesday, June 10, 2015 11:42 PM
> > To: Pang, LongtaoX
> > Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> > Robert
> > Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> > test job
> > 
> > longtao.pang writes ("[OSSTEST Nested PATCH v11 6/7] Compose the main recipe
> > of nested test job"):
> > > The ident and guestname are same of 'nestedl1' for L1 guest VM.
> > 
> > This is going in the right direction.
> > 
> > I think arrangements need to be made to capture the logs for
> > nestedl1.
> > 
> I am sorry, what logs do you mean?

I think the ones gathered from a host by ts-logs-capture, which should
be made to run against an L1 hypervisor and added to the job recipe.

> > I think some extra +s in the l2 install and start operations might be
> > useful, because the testid probably doesn't need to mention nestedl1.
> > 
> I am sorry, do you mean that I should add '+' for l2 installation, 
> such as ' ts-debian-hvm-install + nestedl1 + nestedl2' ? 

You can use standalone --dry-run to see all the testid's generated by
your job and then adjust the +'s until they are as desired (in this case
Ian is suggesting to omit nestedl1 from the testid).

Ian.

Re: [OSSTEST Nested PATCH v11 2/7] Parsing grub which has 'submenu' primitive

[Robert Hu]

On Thu, 2015-06-11 at 09:37 +0100, Ian Campbell wrote:
> On Thu, 2015-06-11 at 11:17 +0800, Robert Hu wrote:
> 
> > > Unless I'm mistaken, the MenuEntryPath of a $submenu is never used ?
> > > Not setting it would avoid (a) a need to rewrap and (b) me complaining
> > > that you have open-coded the join twice.
> > Actually this contribution from Ian Campbell.
> > Hi Ian C., would you agree if I simply remove the 'MenuEntryPath' here? 
> 
> I left it as a debugging aid, since it shows up in Dumper($submenu)
> which is convenient to sprinkle around while debuggiung. I don't mind if
> it is removed or kept though.
Forgive me that I'm to remove it, otherwise I don't know how to fix the
"open-coded" criticism.
> 
> Note that several patches from this series are already in osstest
> production:
> 
> b77a6a2 Changes to support '/boot' leading paths of kernel, xen, in grub
> 997385f Parsing grub which has 'submenu' primitive
> 155bdb3 Move the code for setting memory size into prep()
> 2545fc6 Edit some APIs in TestSupport.pm for nested test
> 699c911 Refactor installation of overlays
> 
> So a incremental patch is what is needed here.
Sure.
> 
> I fixed one or two issues as I committed, e.g. :
> > > Missing spaces after if and inside `){'.
> > OK, to refine these.
> 
> Worth double checking which I caught though.
> 
> Ian.
> 

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Thursday, June 11, 2015 4:40 PM
> To: Pang, LongtaoX
> Cc: Ian Jackson; xen-devel@lists.xen.org; wei.liu2@citrix.com; Hu, Robert
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> On Thu, 2015-06-11 at 07:41 +0000, Pang, LongtaoX wrote:
> >
> > > -----Original Message-----
> > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > > Sent: Wednesday, June 10, 2015 11:42 PM
> > > To: Pang, LongtaoX
> > > Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com;
> Hu,
> > > Robert
> > > Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of
> nested
> > > test job
> > >
> > > longtao.pang writes ("[OSSTEST Nested PATCH v11 6/7] Compose the main
> recipe
> > > of nested test job"):
> > > > The ident and guestname are same of 'nestedl1' for L1 guest VM.
> > >
> > > This is going in the right direction.
> > >
> > > I think arrangements need to be made to capture the logs for
> > > nestedl1.
> > >
> > I am sorry, what logs do you mean?
> 
> I think the ones gathered from a host by ts-logs-capture, which should
> be made to run against an L1 hypervisor and added to the job recipe.
> 
I have checked nested job testid, as below:
#./standalone run-job --simulate -h dummy test-amd64-amd64-qemuu-nested | grep testid
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 1 testid build-check(1) ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 2 testid hosts-allocate ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 3 testid host-install(3) ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 4 testid host-ping-check-native ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 5 testid xen-install ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 6 testid xen-boot ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 7 testid host-ping-check-xen ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 8 testid leak-check/basis(8) ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 9 testid debian-hvm-install/nestedl1 ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 10 testid nested-setup/nestedl1 ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 11 testid xen-install/nestedl1 ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 12 testid host-reboot/nestedl1 ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 13 testid debian-hvm-install/nestedl1/nestedl2 ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 14 testid guest-stop/nestedl1/nestedl2 ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 15 testid guest-destroy/nestedl1 ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 16 testid leak-check/check ==========
2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 17 testid capture-logs(17) ==========

Sorry, I am confused, ' capture-logs' has already been added in job, do we need to add it again?

> > > I think some extra +s in the l2 install and start operations might be
> > > useful, because the testid probably doesn't need to mention nestedl1.
> > >
> > I am sorry, do you mean that I should add '+' for l2 installation,
> > such as ' ts-debian-hvm-install + nestedl1 + nestedl2' ?
> 
> You can use standalone --dry-run to see all the testid's generated by
> your job and then adjust the +'s until they are as desired (in this case
> Ian is suggesting to omit nestedl1 from the testid).
> 
Thanks Ian C.
So, the expect requirement is like below in 'sg-run-job', right? (I will try to change 
idents/guest names as `l1' and `l2' later as Ian Jackson's suggestion)
proc need-hosts/test-nested {} {return host}
proc run-job/test-nested {} {
    run-ts . = ts-debian-hvm-install + host nestedl1
    run-ts . = ts-nested-setup + host nestedl1
    run-ts . = ts-xen-install nestedl1
    run-ts . = ts-host-reboot nestedl1
    run-ts . = ts-debian-hvm-install nestedl1 nestedl2
    run-ts . = ts-guest-stop nestedl1 nestedl2
    run-ts . = ts-guest-destroy + host nestedl1
}
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Thu, 2015-06-11 at 09:52 +0000, Pang, LongtaoX wrote:
> I have checked nested job testid, as below:
> #./standalone run-job --simulate -h dummy test-amd64-amd64-qemuu-nested | grep testid
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 1 testid build-check(1) ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 2 testid hosts-allocate ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 3 testid host-install(3) ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 4 testid host-ping-check-native ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 5 testid xen-install ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 6 testid xen-boot ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 7 testid host-ping-check-xen ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 8 testid leak-check/basis(8) ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 9 testid debian-hvm-install/nestedl1 ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 10 testid nested-setup/nestedl1 ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 11 testid xen-install/nestedl1 ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 12 testid host-reboot/nestedl1 ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 13 testid debian-hvm-install/nestedl1/nestedl2 ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 14 testid guest-stop/nestedl1/nestedl2 ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 15 testid guest-destroy/nestedl1 ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 16 testid leak-check/check ==========
> 2015-06-11 09:46:37 Z standalone.test-amd64-amd64-qemuu-nested ========== 17 testid capture-logs(17) ==========
> 
> Sorry, I am confused, ' capture-logs' has already been added in job, do we need to add it again?

It's running against the L0 though isn't it?

I think you want both L0 and L1 hypervisors to have their logs
collected.

> 
> > > > I think some extra +s in the l2 install and start operations might be
> > > > useful, because the testid probably doesn't need to mention nestedl1.
> > > >
> > > I am sorry, do you mean that I should add '+' for l2 installation,
> > > such as ' ts-debian-hvm-install + nestedl1 + nestedl2' ?
> > 
> > You can use standalone --dry-run to see all the testid's generated by
> > your job and then adjust the +'s until they are as desired (in this case
> > Ian is suggesting to omit nestedl1 from the testid).
> > 
> Thanks Ian C.
> So, the expect requirement is like below in 'sg-run-job', right?

I'm not sure, if that produces the correct testid then it is correct.

>  (I will try to change 
> idents/guest names as `l1' and `l2' later as Ian Jackson's suggestion)
> proc need-hosts/test-nested {} {return host}
> proc run-job/test-nested {} {
>     run-ts . = ts-debian-hvm-install + host nestedl1
>     run-ts . = ts-nested-setup + host nestedl1
>     run-ts . = ts-xen-install nestedl1
>     run-ts . = ts-host-reboot nestedl1
>     run-ts . = ts-debian-hvm-install nestedl1 nestedl2
>     run-ts . = ts-guest-stop nestedl1 nestedl2
>     run-ts . = ts-guest-destroy + host nestedl1
> }
> > Ian.
> 

Re: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install

[Ian Jackson]

Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 4/7] Changes on test step of Debian hvm guest install"):
> [Ian Jackson:]
> > longtao.pang writes ("[OSSTEST Nested PATCH v11 4/7] Changes on test step of
> > > +    # Use guest_var to get specific disk size, or will use default $disk_mb
> > > +    $disk_mb= guest_var($gho,'disksize',$disk_mb);
> > 
> > I would prefer this comment and the one about RAM to be expressed,
> > instead, like this:
> > 
> >     sub more_prepareguest_hvm ($$$$;@) {
> >         my ($ho, $gho, $ram_mb, $disk_mb, %xopts) = @_;
> >   +     # $ram_mb and $disk_mb are defaults, used if runvars don't say
> > 
> I am sorry, do you mean that I should add another more comment you preferred 
> in 'more_prepareguest_hvm ($$$$;@)' function inside TestSupport.pm?

I mean that you should add this comment instead of the ones next to
the assignments to $disk_mb and $ram_mb.

Interface comments (which in this case document a non-obvious feature
of the function's behaviour) are much better than code comments (which
in this case would simply say the same thing as the code).

Ian.

Re: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration

[Ian Jackson]

Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration"):
> [Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]]
> > longtao.pang writes ("[OSSTEST Nested PATCH v11 5/7] Add new script to
> > > +target_cmd_root($l1, "update-rc.d osstest-confirm-booted start 99 2 .");
> > 
> > This is an open coded copy of the code from ts-host-install.  It
> > should be broken out, I think.
> > 
> I am sorry, could you please make it more clear? Thanks.

I mean that your patch introduces an additional copy of this line:

    target_cmd_root($l1, "update-rc.d osstest-confirm-booted start 99 2 .");

If we should ever change the way this works, we would then need to
change it in two places.

I think you should move this out into its own function, and call it
from both places.  I'm not sure exactly what it shoudl be called, but
maybe `host_install_postboot_complete' ?

> > > +target_install_packages_norec($l1, qw(lvm2 rsync genisoimage));
> > 
> > Do we need to install genisoimage here ?  The guest install scripts do
> > it.  Or are we just doing it here for efficiency reasons ?
> 
> In fact, there is no need to install ' genisoimage' here.  It's just
> for efficiency reason, since if install this package here, it will
> not be installed in 'ts-debian-hvm-install' script again.  So, do
> you prefer to discard it or keep it here? I think there is no
> harmful to install this package here.

It's not harmful, indeed.  I think it would be worth adding a comment
saying that these are installed here merely for efficiency reasons.

> > We would avoid having to mention /dev/xvdb if we created the VG in the
> > host, before doing block-attach.  I'm not sure whether that's an
> > improvement.
> 
> I am sorry, I cannot get your point. Could you please make it more
> clear? Thanks.

As you explain in the comment, you have to mention /dev/xvdb here even
though after rebooting into Xen this will be /dev/sdb.

This wrinkle would become invisible if you did pvcreate and vgcreate
in the L0 (before attach), rather than the L1 (after attach).
Because, then none of your operations on L1 would not need to name the
physical device.

Ian.

Re: [OSSTEST Nested PATCH v11 7/7] Add test job for nest test case

[Ian Jackson]

Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 7/7] Add test job for nest test case"):
...
> >  * Why do you specify the l2 disk size explicitly ?
> 
> Just for some configure rule in our lab, we assign at least '15000M' disk size for nested L2 guest, 

I see.  What's the reasoning behind that rule, do you know ?  If there
is a good reason then it might apply here too.

Otherwise,

> since it's no harmful, I think.
> Actually, it works fine if not specify the l2 disk size and using default '10000M'.
> So, you prefer to use default disk size for l2 guest, right?

Yes, if there is no reason, we should use the default.

Thanks,
Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
...

(Ian C has answered some of your comments.  On the others:)

> > I think you probably want to run leak-check on the L1.
> > 
> I have noticed the logs which printed on screen during job running, the leak-check will
> be run on L1 at the end of testing.
> Or, do you mean that I should run leak-check on L1 after the L1 HVM guest finishing installation?

Maybe I am confused.

I think that with your patches, leak-check would be run on the L0
only.

But leak-check ought to be run on the L1 as well.  The basis would be
after nested-setup, xen-install and reboot I think (similar to the
position for the L0 basis).

leak-check compares the set of objects present at the `leak-check
check' step with the set of objects present at the `basis' step, and
the check fails if there are any new objects.  For this purpose,
objects includes domains, corefiles, etc.

> > Do you not want to run the steps in test-guest ?  Perhaps it would be
> > better to add a host ident argument to test-guest{,-migr,-nomigr} ?
> > 
> We do not plan to run the steps in test-guest currently.

Why not ?

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Robert Hu]

On Thu, 2015-06-11 at 16:19 +0100, Ian Jackson wrote:
> Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> ...
> 
> (Ian C has answered some of your comments.  On the others:)
> 
> > > I think you probably want to run leak-check on the L1.
> > > 
> > I have noticed the logs which printed on screen during job running, the leak-check will
> > be run on L1 at the end of testing.
> > Or, do you mean that I should run leak-check on L1 after the L1 HVM guest finishing installation?
> 
> Maybe I am confused.
> 
> I think that with your patches, leak-check would be run on the L0
> only.
> 
> But leak-check ought to be run on the L1 as well.  The basis would be
> after nested-setup, xen-install and reboot I think (similar to the
> position for the L0 basis).
> 
> leak-check compares the set of objects present at the `leak-check
> check' step with the set of objects present at the `basis' step, and
> the check fails if there are any new objects.  For this purpose,
> objects includes domains, corefiles, etc.
> 
> > > Do you not want to run the steps in test-guest ?  Perhaps it would be
> > > better to add a host ident argument to test-guest{,-migr,-nomigr} ?
> > > 
> > We do not plan to run the steps in test-guest currently.
> 
> Why not ?
Hi Ian J., because nested Xen isn't that matured at present; it is
currently 'tech preview' phase. We now just add some sanity test case to
defend current work fruits. We can add more complicated test cases later
as nested Xen development moves forward.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Thursday, June 11, 2015 11:15 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> Robert
> Subject: RE: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested
> test configuration
> 
> Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 5/7] Add new script to
> customize nested test configuration"):
> > [Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]]
> 
> > > We would avoid having to mention /dev/xvdb if we created the VG in the
> > > host, before doing block-attach.  I'm not sure whether that's an
> > > improvement.
> >
> > I am sorry, I cannot get your point. Could you please make it more
> > clear? Thanks.
> 
> As you explain in the comment, you have to mention /dev/xvdb here even
> though after rebooting into Xen this will be /dev/sdb.
> 
> This wrinkle would become invisible if you did pvcreate and vgcreate
> in the L0 (before attach), rather than the L1 (after attach).
> Because, then none of your operations on L1 would not need to name the
> physical device.
> 
Thanks for your explanation.
For nested job, we will install L2 guest inside L1 reuse 'ts-debian-hvm-install' script.
But according to recent code design, inside L1 HVM guest, there is no vg group, so that we need to
create a vg group for L2 installation.
So, inside L0, we create storage lv(lvcreate -L ${guest_storage_lv_size}M -n $guest_storage_lv_name $vgname), 
attach this lv to L1. Then inside L1, using the attached disk to create a pv(pvcreate /dev/xvdb),
then create a vg(vgcreate ${l1_gn}-disk /dev/xvdb) base on the pv. Then, using this vg for installing L2.
I think '/dev/xvdb' is necessary when create vg inside L1, using command 'vgcreate ${l1_gn}-disk /dev/xvdb' .
Please correct me if I am wrong.

> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Thursday, June 11, 2015 11:20 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> Robert
> Subject: RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main
> recipe of nested test job"):
> ...
> 
> (Ian C has answered some of your comments.  On the others:)
> 
> > > I think you probably want to run leak-check on the L1.
> > >
> > I have noticed the logs which printed on screen during job running, the
> leak-check will
> > be run on L1 at the end of testing.
> > Or, do you mean that I should run leak-check on L1 after the L1 HVM guest
> finishing installation?
> 
> Maybe I am confused.
> 
> I think that with your patches, leak-check would be run on the L0
> only.
> 
> But leak-check ought to be run on the L1 as well.  The basis would be
> after nested-setup, xen-install and reboot I think (similar to the
> position for the L0 basis).
> 
> leak-check compares the set of objects present at the `leak-check
> check' step with the set of objects present at the `basis' step, and
> the check fails if there are any new objects.  For this purpose,
> objects includes domains, corefiles, etc.
> 
OK, so the recipe in sg-run-job should be like below, please correct me if something wrong.
proc need-hosts/test-nested {} {return host}
proc run-job/test-nested {} {
    run-ts . = ts-debian-hvm-install + host nestedl1
    run-ts . = ts-nested-setup + host nestedl1
    run-ts . = ts-xen-install nestedl1
    run-ts . = ts-host-reboot nestedl1
    run-ts . = ts-leak-check nestedl1 + basis
    run-ts . = ts-debian-hvm-install nestedl1 nestedl2
    run-ts . = ts-guest-stop nestedl1 nestedl2
    run-ts . = ts-leak-check nestedl1 + check
    run-ts . = ts-logs-capture nestedl1
    run-ts . = ts-guest-destroy + host nestedl1
}
> > > Do you not want to run the steps in test-guest ?  Perhaps it would be
> > > better to add a host ident argument to test-guest{,-migr,-nomigr} ?
> > >
> > We do not plan to run the steps in test-guest currently.
> 
> Why not ?
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested test configuration

[Ian Campbell]

On Fri, 2015-06-12 at 03:46 +0000, Pang, LongtaoX wrote:
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Thursday, June 11, 2015 11:15 PM
> > To: Pang, LongtaoX
> > Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> > Robert
> > Subject: RE: [OSSTEST Nested PATCH v11 5/7] Add new script to customize nested
> > test configuration
> > 
> > Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 5/7] Add new script to
> > customize nested test configuration"):
> > > [Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]]
> > 
> > > > We would avoid having to mention /dev/xvdb if we created the VG in the
> > > > host, before doing block-attach.  I'm not sure whether that's an
> > > > improvement.
> > >
> > > I am sorry, I cannot get your point. Could you please make it more
> > > clear? Thanks.
> > 
> > As you explain in the comment, you have to mention /dev/xvdb here even
> > though after rebooting into Xen this will be /dev/sdb.
> > 
> > This wrinkle would become invisible if you did pvcreate and vgcreate
> > in the L0 (before attach), rather than the L1 (after attach).
> > Because, then none of your operations on L1 would not need to name the
> > physical device.
> > 
> Thanks for your explanation.
> For nested job, we will install L2 guest inside L1 reuse 'ts-debian-hvm-install' script.
> But according to recent code design, inside L1 HVM guest, there is no vg group, so that we need to
> create a vg group for L2 installation.
> So, inside L0, we create storage lv(lvcreate -L ${guest_storage_lv_size}M -n $guest_storage_lv_name $vgname), 
> attach this lv to L1. Then inside L1, using the attached disk to create a pv(pvcreate /dev/xvdb),
> then create a vg(vgcreate ${l1_gn}-disk /dev/xvdb) base on the pv. Then, using this vg for installing L2.
> I think '/dev/xvdb' is necessary when create vg inside L1, using command 'vgcreate ${l1_gn}-disk /dev/xvdb' .
> Please correct me if I am wrong.

I think Ian J's suggestion is that instead of creating the ${l1_gn}-disk
VG via commands on /dev/xvdb in the L1 you can create it via commands on
${guest_storage_lvdev} in L0.

e.g. instead of
+target_cmd_root($l1, "pvcreate /dev/xvdb && vgcreate ${l1_gn}-disk /dev/xvdb");
you can instead do:
+target_cmd_root($l0, "pvcreate  ${guest_storage_lvdev} && vgcreate ${l1_gn}-disk /dev/xvdb");
before you do the block-attach.

That way the VG is already present on the device when it appears in L1
and L1 will just find it.

This avoids any confusion about sdb vs xvdb naming.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Fri, 2015-06-12 at 11:42 +0800, Robert Hu wrote:
> Hi Ian J., because nested Xen isn't that matured at present; it is
> currently 'tech preview' phase. We now just add some sanity test case to
> defend current work fruits. We can add more complicated test cases later
> as nested Xen development moves forward.

I don't think it needs doing as part of this series, but I do think it
would be worth adding the standard suite of tests steps to L2 guests
soon after.

The way osstest deals with test failures i.e. classifying them into "has
always failed" vs. "regression" means that it is fine to add tests for
features which aren't mature yet, since they will fall into the "has
always failed" set and not block pushes. In fact it is in some sense
good to do this since it gives a concrete set of (necessary but not
necessarily sufficient) things which need to be fixed for the feature to
reach maturity.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Robert Hu]

On Fri, 2015-06-12 at 09:44 +0100, Ian Campbell wrote:
> On Fri, 2015-06-12 at 11:42 +0800, Robert Hu wrote:
> > Hi Ian J., because nested Xen isn't that matured at present; it is
> > currently 'tech preview' phase. We now just add some sanity test case to
> > defend current work fruits. We can add more complicated test cases later
> > as nested Xen development moves forward.
> 
> I don't think it needs doing as part of this series, but I do think it
> would be worth adding the standard suite of tests steps to L2 guests
> soon after.
Agree. We can separately add those part in a later patch series.  
> 
> The way osstest deals with test failures i.e. classifying them into "has
> always failed" vs. "regression" means that it is fine to add tests for
> features which aren't mature yet, since they will fall into the "has
> always failed" set and not block pushes. In fact it is in some sense
> good to do this since it gives a concrete set of (necessary but not
> necessarily sufficient) things which need to be fixed for the feature to
> reach maturity.
We would like this test job's failure to be marked 'regression', which
shall block related breaking code's commitment.
We can consider to contribute more code later, including proved/success
test cases (which will fall into regression as this case), and fail test
cases (which shall soon be fixed/developed).
> 
> Ian.
> 

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Fri, 2015-06-12 at 17:00 +0800, Robert Hu wrote:
> On Fri, 2015-06-12 at 09:44 +0100, Ian Campbell wrote:
> > On Fri, 2015-06-12 at 11:42 +0800, Robert Hu wrote:
> > > Hi Ian J., because nested Xen isn't that matured at present; it is
> > > currently 'tech preview' phase. We now just add some sanity test case to
> > > defend current work fruits. We can add more complicated test cases later
> > > as nested Xen development moves forward.
> > 
> > I don't think it needs doing as part of this series, but I do think it
> > would be worth adding the standard suite of tests steps to L2 guests
> > soon after.
> Agree. We can separately add those part in a later patch series.  
> > 
> > The way osstest deals with test failures i.e. classifying them into "has
> > always failed" vs. "regression" means that it is fine to add tests for
> > features which aren't mature yet, since they will fall into the "has
> > always failed" set and not block pushes. In fact it is in some sense
> > good to do this since it gives a concrete set of (necessary but not
> > necessarily sufficient) things which need to be fixed for the feature to
> > reach maturity.
> We would like this test job's failure to be marked 'regression', which
> shall block related breaking code's commitment.

This is all done automatically, by comparison with previous flights.

In essence if it passed last time then it will be considered a
regression if it fails and for as long as it keeps failing (unless it is
manually overridden via a forced push).

So there is nothing to be "marked".

> We can consider to contribute more code later, including proved/success
> test cases (which will fall into regression as this case), and fail test
> cases (which shall soon be fixed/developed).

You should just add the cases, await the first results and investigate
if it is not a pass as expected.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
...
> > leak-check compares the set of objects present at the `leak-check
> > check' step with the set of objects present at the `basis' step, and
> > the check fails if there are any new objects.  For this purpose,
> > objects includes domains, corefiles, etc.
> > 
> OK, so the recipe in sg-run-job should be like below, please correct me if something wrong.
> proc need-hosts/test-nested {} {return host}
> proc run-job/test-nested {} {

This is roughly right, but thinking about it, you want ts-logs-capture
to run even if the previous steps fail.

I think it might be better to reuse (subvert?) the existing machinery
in sg-run-job, by adding the l1 to need_xen_hosts.

Maybe something like

  proc add-xen-host-retrospectively {ident} {
      global need_xen_hosts
      ts-leak-check $ident + basis
      lappend need_xen_hosts $ident
  }

?

And then call

  add-xen-host-retrospectively l1

at the appropriate point.

If you do this then the main run-job proc will automatically do the
leak-check and the logs-capture for you.


Thinking about this leads me to ask another question.  Suppose that a
bug causes the l1 to lock up completely.  ts-logs-capture will attempt
to hard reboot a locked-up host.  If it can't fetch any logs, it calls
    target_reboot_hard($ho);

What will that do if $ho refers to the l1 ?  It relies on the power
method.  Does your nested l1 "host" have a power method ?

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Ian Jackson writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> Thinking about this leads me to ask another question.  Suppose that a
> bug causes the l1 to lock up completely.  ts-logs-capture will attempt
> to hard reboot a locked-up host.  If it can't fetch any logs, it calls
>     target_reboot_hard($ho);
> 
> What will that do if $ho refers to the l1 ?  It relies on the power
> method.  Does your nested l1 "host" have a power method ?

Oh, and (sorry) another thought: in principle it would be possible for
the l0 and l1 toolstacks to be different.

AFAICT your current series doesn't support this ?

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Robert Hu]

On Fri, 2015-06-12 at 16:27 +0100, Ian Jackson wrote:
> Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> ...
> > > leak-check compares the set of objects present at the `leak-check
> > > check' step with the set of objects present at the `basis' step, and
> > > the check fails if there are any new objects.  For this purpose,
> > > objects includes domains, corefiles, etc.
> > > 
> > OK, so the recipe in sg-run-job should be like below, please correct me if something wrong.
> > proc need-hosts/test-nested {} {return host}
> > proc run-job/test-nested {} {
> 
> This is roughly right, but thinking about it, you want ts-logs-capture
> to run even if the previous steps fail.
> 
> I think it might be better to reuse (subvert?) the existing machinery
> in sg-run-job, by adding the l1 to need_xen_hosts.
> 
> Maybe something like
> 
>   proc add-xen-host-retrospectively {ident} {
>       global need_xen_hosts
>       ts-leak-check $ident + basis
>       lappend need_xen_hosts $ident
>   }
> 
> ?
> 
> And then call
> 
>   add-xen-host-retrospectively l1
> 
> at the appropriate point.
Thanks Ian J.. Since I'm not familiar with tcl and your sg-run-job
framework, does here 'appropriate point' refers to before
 	per-host-ts .       =(*)             {ts-leak-check basis}
in proc run-job {job}? but then l1 doesn't exist yet I'm afraid.
If after that point, the l1 has missd check basis step.
> 
> If you do this then the main run-job proc will automatically do the
> leak-check and the logs-capture for you.
> 
> 
> Thinking about this leads me to ask another question.  Suppose that a
> bug causes the l1 to lock up completely.  ts-logs-capture will attempt
> to hard reboot a locked-up host.  If it can't fetch any logs, it calls
>     target_reboot_hard($ho);
> 
> What will that do if $ho refers to the l1 ?  It relies on the power
> method.  Does your nested l1 "host" have a power method ?
I'm afraid l1 won't like normal hosts has power cycle operations. Maybe
we need to simulate it?
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Robert Hu]

On Fri, 2015-06-12 at 16:28 +0100, Ian Jackson wrote:
> Ian Jackson writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> > Thinking about this leads me to ask another question.  Suppose that a
> > bug causes the l1 to lock up completely.  ts-logs-capture will attempt
> > to hard reboot a locked-up host.  If it can't fetch any logs, it calls
> >     target_reboot_hard($ho);
> > 
> > What will that do if $ho refers to the l1 ?  It relies on the power
> > method.  Does your nested l1 "host" have a power method ?
> 
> Oh, and (sorry) another thought: in principle it would be possible for
> the l0 and l1 toolstacks to be different.
> 
> AFAICT your current series doesn't support this ?
No it doesn't. Actually we support only xl tool stack.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Sun, 2015-06-14 at 20:51 +0800, Robert Hu wrote:
> On Fri, 2015-06-12 at 16:27 +0100, Ian Jackson wrote:
> > Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> > > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > ...
> > > > leak-check compares the set of objects present at the `leak-check
> > > > check' step with the set of objects present at the `basis' step, and
> > > > the check fails if there are any new objects.  For this purpose,
> > > > objects includes domains, corefiles, etc.
> > > > 
> > > OK, so the recipe in sg-run-job should be like below, please correct me if something wrong.
> > > proc need-hosts/test-nested {} {return host}
> > > proc run-job/test-nested {} {
> > 
> > This is roughly right, but thinking about it, you want ts-logs-capture
> > to run even if the previous steps fail.
> > 
> > I think it might be better to reuse (subvert?) the existing machinery
> > in sg-run-job, by adding the l1 to need_xen_hosts.
> > 
> > Maybe something like
> > 
> >   proc add-xen-host-retrospectively {ident} {
> >       global need_xen_hosts
> >       ts-leak-check $ident + basis
> >       lappend need_xen_hosts $ident
> >   }
> > 
> > ?
> > 
> > And then call
> > 
> >   add-xen-host-retrospectively l1
> > 
> > at the appropriate point.
> Thanks Ian J.. Since I'm not familiar with tcl and your sg-run-job
> framework, does here 'appropriate point' refers to before
>  	per-host-ts .       =(*)             {ts-leak-check basis}
> in proc run-job {job}? but then l1 doesn't exist yet I'm afraid.
> If after that point, the l1 has missd check basis step.

Note that Ian included a ts-leak-check in his
add-xen-host-retrospectively function, so you needn't worry about this,
you should do it jsut after the L1 has booted into Xen, I think.

What you get automatically here is the final leak check (i.e. the
comparison against the basis).

> > If you do this then the main run-job proc will automatically do the
> > leak-check and the logs-capture for you.
> > 
> > 
> > Thinking about this leads me to ask another question.  Suppose that a
> > bug causes the l1 to lock up completely.  ts-logs-capture will attempt
> > to hard reboot a locked-up host.  If it can't fetch any logs, it calls
> >     target_reboot_hard($ho);
> > 
> > What will that do if $ho refers to the l1 ?  It relies on the power
> > method.  Does your nested l1 "host" have a power method ?
> I'm afraid l1 won't like normal hosts has power cycle operations. Maybe
> we need to simulate it?

Perhaps arrange for an appropriate PowerMethod for "hosts which are
actually guests"?

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Monday, June 15, 2015 5:08 PM
> To: Hu, Robert
> Cc: Ian Jackson; Pang, LongtaoX; xen-devel@lists.xen.org; wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> On Sun, 2015-06-14 at 20:51 +0800, Robert Hu wrote:
> > On Fri, 2015-06-12 at 16:27 +0100, Ian Jackson wrote:
> > > Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the
> main recipe of nested test job"):
> > > > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > > ...
> > > > > leak-check compares the set of objects present at the `leak-check
> > > > > check' step with the set of objects present at the `basis' step, and
> > > > > the check fails if there are any new objects.  For this purpose,
> > > > > objects includes domains, corefiles, etc.
> > > > >
> > > > OK, so the recipe in sg-run-job should be like below, please correct me if
> something wrong.
> > > > proc need-hosts/test-nested {} {return host}
> > > > proc run-job/test-nested {} {
> > >
> > > This is roughly right, but thinking about it, you want ts-logs-capture
> > > to run even if the previous steps fail.
> > >
> > > I think it might be better to reuse (subvert?) the existing machinery
> > > in sg-run-job, by adding the l1 to need_xen_hosts.
> > >
> > > Maybe something like
> > >
> > >   proc add-xen-host-retrospectively {ident} {
> > >       global need_xen_hosts
> > >       ts-leak-check $ident + basis
> > >       lappend need_xen_hosts $ident
> > >   }
> > >
> > > ?
> > >
> > > And then call
> > >
> > >   add-xen-host-retrospectively l1
> > >
> > > at the appropriate point.
> > Thanks Ian J.. Since I'm not familiar with tcl and your sg-run-job
> > framework, does here 'appropriate point' refers to before
> >  	per-host-ts .       =(*)             {ts-leak-check basis}
> > in proc run-job {job}? but then l1 doesn't exist yet I'm afraid.
> > If after that point, the l1 has missd check basis step.
> 
> Note that Ian included a ts-leak-check in his
> add-xen-host-retrospectively function, so you needn't worry about this,
> you should do it jsut after the L1 has booted into Xen, I think.
> 
> What you get automatically here is the final leak check (i.e. the
> comparison against the basis).
Thanks Ian C.
So, I modify the nested job recipe in sg-run-job as below.
proc add-xen-host-retrospectively {ident} {
    global need_xen_hosts
    run-ts . = ts-leak-check basis $ident
    lappend need_xen_hosts $ident
}

proc need-hosts/test-nested {} {return host}
proc run-job/test-nested {} {
    run-ts . = ts-debian-hvm-install + host nestedl1
    run-ts . = ts-nested-setup + host nestedl1
    run-ts . = ts-xen-install nestedl1
    run-ts . = ts-host-reboot nestedl1
    add-xen-host-retrospectively nestedl1
    run-ts . = ts-debian-hvm-install nestedl1 nestedl2
    run-ts . = ts-guest-stop nestedl1 nestedl2
    run-ts . = ts-guest-destroy + host nestedl1
}

After executing command ' ./standalone run-job --simulate -h dummy test-amd64-amd64-qemuu-nested | grep testid',
get below information:
root@OSSTEST:~/v11_pretest_3/osstest_v11# ./standalone run-job --simulate -h dummy test-amd64-amd64-qemuu-nested | grep testid                
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 1 testid build-check(1) ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 2 testid hosts-allocate ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 3 testid host-install(3) ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 4 testid host-ping-check-native ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 5 testid xen-install ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 6 testid xen-boot ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 7 testid host-ping-check-xen ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 8 testid leak-check/basis(8) ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 9 testid debian-hvm-install ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 10 testid nested-setup ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 11 testid xen-install/nestedl1 ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 12 testid host-reboot/nestedl1 ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 13 testid leak-check/basis/nestedl1 ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 14 testid debian-hvm-install/nestedl1/nestedl2 ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 15 testid guest-stop/nestedl1/nestedl2 ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 16 testid guest-destroy ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 17 testid leak-check/check ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 18 testid leak-check/check/nestedl1 ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 19 testid capture-logs(19) ==========
2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 20 testid capture-logs/nestedl1(20) ==========
root@OSSTEST:~/v11_pretest_3/osstest_v11#

But, for testid of '18', I think it will failed to execute 'leak-check/check/nestedl1', since 
'nestedl1' has been destroyed via the action of 'run-ts . = ts-guest-destroy + host nestedl1'. 
Please correct me if I am wrong.

Another question, after execute testid of '13'(leak-check/basis/nestedl1), the testing will fail and exit with below message.
Something wrong with my test environment?
2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xl list
2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xenstore-ls -fp
2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp /var/run /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
find: `/var/core': No such file or directory
2015-06-17 08:10:26 Z command nonzero waitstatus 256: timeout 60 ssh -o StrictHostKeyChecking=no -o BatchMode=yes -o ConnectTimeout=100 -o ServerAliveInterval=100 -o PasswordAuthentication=no -o ChallengeResponseAuthentication=no -o UserKnownHostsFile=tmp/t.known_hosts_standalone.test-amd64-amd64-qemuu-nested root@192.168.199.25 find /tmp /var/run /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
BROKEN: listing/checking leakable objects: status 256 at Osstest/TestSupport.pm line 400.
; marked standalone.test-amd64-amd64-qemuu-nested broken at Osstest/TestSupport.pm line 218.
+ rc=2
+ date -u +%Y-%m-%d %H:%M:%S Z exit status 2
2015-06-17 08:10:26 Z exit status 2
+ exit 2
2015-06-17 08:10:26 Z standalone.test-amd64-amd64-qemuu-nested 13 status status fail
> 
> > > If you do this then the main run-job proc will automatically do the
> > > leak-check and the logs-capture for you.
> > >
> > >
> > > Thinking about this leads me to ask another question.  Suppose that a
> > > bug causes the l1 to lock up completely.  ts-logs-capture will attempt
> > > to hard reboot a locked-up host.  If it can't fetch any logs, it calls
> > >     target_reboot_hard($ho);
> > >
> > > What will that do if $ho refers to the l1 ?  It relies on the power
> > > method.  Does your nested l1 "host" have a power method ?
> > I'm afraid l1 won't like normal hosts has power cycle operations. Maybe
> > we need to simulate it?
> 
> Perhaps arrange for an appropriate PowerMethod for "hosts which are
> actually guests"?
> 
I think maybe we need to refactor 'power_cycle' function in TestSupport.pm. I have not try it, something like below?
sub power_cycle ($) {
    my ($ho) = @_;
+    if (guest_var($ho,"enable_nestedhvm",'') =~ m/true/) {
+        guest_destroy($ho);
+        guest_create($gho);
+        guest_await_dhcp_tcp($gho,300);
+        guest_check_up($gho);
+    } else {
        $mjobdb->host_check_allocated($ho);
        die "refusing to set power state for host $ho->{Name}".
            " possibly shared with other jobs\n"
            if $ho->{SharedMaybeOthers};
        power_state($ho, 0);
        power_cycle_sleep($ho);
        power_state($ho, 1);
+    }
}
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> > >
> > > Thinking about this leads me to ask another question.  Suppose that a
> > > bug causes the l1 to lock up completely.  ts-logs-capture will attempt
> > > to hard reboot a locked-up host.  If it can't fetch any logs, it calls
> > >     target_reboot_hard($ho);
> > >
> > > What will that do if $ho refers to the l1 ?  It relies on the power
> > > method.  Does your nested l1 "host" have a power method ?
> > I'm afraid l1 won't like normal hosts has power cycle operations. Maybe
> > we need to simulate it?
> 
> Perhaps arrange for an appropriate PowerMethod for "hosts which are
> actually guests"?
> 
Update.
I think maybe we need to refactor 'power_cycle' function in TestSupport.pm. I have not try it, something like below?
sub power_cycle ($) {
    my ($ho) = @_;
+    if (guest_var($ho,"enable_nestedhvm",'') =~ m/true/) {
+        guest_destroy($ho);
+        guest_create($ho);
+        guest_await_dhcp_tcp($ho,300);
+        guest_check_up($ho);
+    } else {
        $mjobdb->host_check_allocated($ho);
        die "refusing to set power state for host $ho->{Name}".
            " possibly shared with other jobs\n"
            if $ho->{SharedMaybeOthers};
        power_state($ho, 0);
        power_cycle_sleep($ho);
        power_state($ho, 1);
+    }
}
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Wed, 2015-06-17 at 08:54 +0000, Pang, LongtaoX wrote:

> After executing command ' ./standalone run-job --simulate -h dummy test-amd64-amd64-qemuu-nested | grep testid',
> get below information:
> root@OSSTEST:~/v11_pretest_3/osstest_v11# ./standalone run-job --simulate -h dummy test-amd64-amd64-qemuu-nested | grep testid                
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 1 testid build-check(1) ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 2 testid hosts-allocate ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 3 testid host-install(3) ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 4 testid host-ping-check-native ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 5 testid xen-install ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 6 testid xen-boot ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 7 testid host-ping-check-xen ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 8 testid leak-check/basis(8) ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 9 testid debian-hvm-install ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 10 testid nested-setup ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 11 testid xen-install/nestedl1 ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 12 testid host-reboot/nestedl1 ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 13 testid leak-check/basis/nestedl1 ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 14 testid debian-hvm-install/nestedl1/nestedl2 ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 15 testid guest-stop/nestedl1/nestedl2 ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 16 testid guest-destroy ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 17 testid leak-check/check ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 18 testid leak-check/check/nestedl1 ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 19 testid capture-logs(19) ==========
> 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested ========== 20 testid capture-logs/nestedl1(20) ==========
> root@OSSTEST:~/v11_pretest_3/osstest_v11#
> 
> But, for testid of '18', I think it will failed to execute 'leak-check/check/nestedl1', since 
> 'nestedl1' has been destroyed via the action of 'run-ts . = ts-guest-destroy + host nestedl1'. 
> Please correct me if I am wrong.

I think you are correct, the logs capture will fail too.

I'll leave it to Ian to suggest a solution since it will no doubt
involve some tcl plumbing (I'd be inclined to record 'hosts which are
actually guests' somewhere and have the infra clean them up
automatically after doing leak check and log collection).

> Another question, after execute testid of '13'(leak-check/basis/nestedl1), the testing will fail and exit with below message.
> Something wrong with my test environment?
> 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xl list
> 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xenstore-ls -fp
> 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp /var/run /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> find: `/var/core': No such file or directory

/var/core is created by ts-host-install. I think the tail end of the
function sub in there which does that and populates /etc/sysctl.conf
and /etc/security/limits.d/coredumps.conf should be refactored probably
to be alongside the osstest-confirm-booted thing which IIRC you are
already going to refactor in the next version.

> > Perhaps arrange for an appropriate PowerMethod for "hosts which are
> > actually guests"?
> > 
> I think maybe we need to refactor 'power_cycle' function in TestSupport.pm. I have not try it, something like below?

I was thinking more along the lines of creating Osstest/PDU/guest.pm
with the appropriate methods calling out to toolstack($l0)->foo, setting
$ho->{Power} = 'guest $l1guestname' somewhere and allowing
power_cycle_host_setup to do it's thing.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Wednesday, June 17, 2015 5:35 PM
> To: Pang, LongtaoX
> Cc: Hu, Robert; Ian Jackson; xen-devel@lists.xen.org; wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> On Wed, 2015-06-17 at 08:54 +0000, Pang, LongtaoX wrote:
> 
> > After executing command ' ./standalone run-job --simulate -h dummy
> test-amd64-amd64-qemuu-nested | grep testid',
> > get below information:
> > root@OSSTEST:~/v11_pretest_3/osstest_v11# ./standalone run-job --simulate -h
> dummy test-amd64-amd64-qemuu-nested | grep testid
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 1 testid build-check(1) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 2 testid hosts-allocate ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 3 testid host-install(3) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 4 testid host-ping-check-native ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 5 testid xen-install ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 6 testid xen-boot ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 7 testid host-ping-check-xen ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 8 testid leak-check/basis(8) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 9 testid debian-hvm-install ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 10 testid nested-setup ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 11 testid xen-install/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 12 testid host-reboot/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 13 testid leak-check/basis/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 14 testid debian-hvm-install/nestedl1/nestedl2 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 15 testid guest-stop/nestedl1/nestedl2 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 16 testid guest-destroy ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 17 testid leak-check/check ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 18 testid leak-check/check/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 19 testid capture-logs(19) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 20 testid capture-logs/nestedl1(20) ==========
> > root@OSSTEST:~/v11_pretest_3/osstest_v11#
> >
> > But, for testid of '18', I think it will failed to execute 'leak-check/check/nestedl1',
> since
> > 'nestedl1' has been destroyed via the action of 'run-ts . = ts-guest-destroy + host
> nestedl1'.
> > Please correct me if I am wrong.
> 
> I think you are correct, the logs capture will fail too.
> 
> I'll leave it to Ian to suggest a solution since it will no doubt
> involve some tcl plumbing (I'd be inclined to record 'hosts which are
> actually guests' somewhere and have the infra clean them up
> automatically after doing leak check and log collection).
> 
> > Another question, after execute testid of '13'(leak-check/basis/nestedl1), the
> testing will fail and exit with below message.
> > Something wrong with my test environment?
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xl list
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xenstore-ls -fp
> > 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp /var/run
> /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> > find: `/var/core': No such file or directory
> 
> /var/core is created by ts-host-install. I think the tail end of the
> function sub in there which does that and populates /etc/sysctl.conf
> and /etc/security/limits.d/coredumps.conf should be refactored probably
> to be alongside the osstest-confirm-booted thing which IIRC you are
> already going to refactor in the next version.
> 
I reviewed related code in 'ts-host-install'. But I am not very clear, the below code will be executed 
in 'ts-debian-hvm-install' too? Or refactor 'osstest-confirm-booted' and the below action should be finished inside this shell script? 
Since, it need '/var/core' directory in L1, please correct me.

    target_cmd_root($ho, 'mkdir -p /var/core');
    target_editfile_root($ho, '/etc/sysctl.conf',
        sub { target_editfile_kvp_replace(
                  "kernel.core_pattern",
                  # %p==pid,%e==executable name,%t==timestamp
                  "/var/core/%t.%p.%e.core") });
    target_cmd_root($ho, "sysctl --load /etc/sysctl.conf");
    my $coredumps_conf = <<'END';
#<domain>      <type>  <item>       <value>
*               soft    core         -1
root            soft    core         -1
END
    target_putfilecontents_root_stash($ho,10,$coredumps_conf,
                                '/etc/security/limits.d/coredumps.conf');

> > > Perhaps arrange for an appropriate PowerMethod for "hosts which are
> > > actually guests"?
> > >
> > I think maybe we need to refactor 'power_cycle' function in TestSupport.pm. I
> have not try it, something like below?
> 
> I was thinking more along the lines of creating Osstest/PDU/guest.pm
> with the appropriate methods calling out to toolstack($l0)->foo, setting
> $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> power_cycle_host_setup to do it's thing.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> I'll leave it to Ian to suggest a solution since it will no doubt
> involve some tcl plumbing (I'd be inclined to record 'hosts which are
> actually guests' somewhere and have the infra clean them up
> automatically after doing leak check and log collection).

Thinking "aloud":

The things we want to happen at the end of the job are:
  * leak check on the l1
 !* logs capture on the l1 (might involve "power cycle")
  * destruction of the l1 on l0
  * leak check on l0
 !* logs capture on l0

The items marked ! should happen even in case of failure.

This is going to be quite fiddly to do in a generic way.  But on the
other hand, doing it in a generic way would avoid writing an explicit
error handler in the l1 test case code, and replicating the "per-host
operations".

> I was thinking more along the lines of creating Osstest/PDU/guest.pm
> with the appropriate methods calling out to toolstack($l0)->foo, setting
> $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> power_cycle_host_setup to do it's thing.

Yes, that's what I meant.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Wed, 2015-06-17 at 11:00 +0000, Pang, LongtaoX wrote:
> > > 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp /var/run
> > /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> > > find: `/var/core': No such file or directory
> > 
> > /var/core is created by ts-host-install. I think the tail end of the
> > function sub in there which does that and populates /etc/sysctl.conf
> > and /etc/security/limits.d/coredumps.conf should be refactored probably
> > to be alongside the osstest-confirm-booted thing which IIRC you are
> > already going to refactor in the next version.
> > 
> I reviewed related code in 'ts-host-install'. But I am not very clear, the below code will be executed 
> in 'ts-debian-hvm-install' too? Or refactor 'osstest-confirm-booted' and the below action should be finished inside this shell script? 
> Since, it need '/var/core' directory in L1, please correct me.

I think you've already arranged for the osstest-confirm-booted stuff to
happen for the L1 host too, but that Ian J has requested it be done
differently such that it is not duplicated in two places.

The code to setup /var/core and the associated sysctl.conf and limits.d
changes should be done in the same manner as osstest-confirm-booted
eventually is based on Ian's suggestion, whatever that was.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Wednesday, June 17, 2015 7:49 PM
> To: Pang, LongtaoX
> Cc: Hu, Robert; Ian Jackson; xen-devel@lists.xen.org; wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> On Wed, 2015-06-17 at 11:00 +0000, Pang, LongtaoX wrote:
> > > > 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp
> /var/run
> > > /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> > > > find: `/var/core': No such file or directory
> > >
> > > /var/core is created by ts-host-install. I think the tail end of the
> > > function sub in there which does that and populates /etc/sysctl.conf
> > > and /etc/security/limits.d/coredumps.conf should be refactored probably
> > > to be alongside the osstest-confirm-booted thing which IIRC you are
> > > already going to refactor in the next version.
> > >
> > I reviewed related code in 'ts-host-install'. But I am not very clear, the below
> code will be executed
> > in 'ts-debian-hvm-install' too? Or refactor 'osstest-confirm-booted' and the
> below action should be finished inside this shell script?
> > Since, it need '/var/core' directory in L1, please correct me.
> 
> I think you've already arranged for the osstest-confirm-booted stuff to
> happen for the L1 host too, but that Ian J has requested it be done
> differently such that it is not duplicated in two places.
> 
Yes, I have arranged for osstest-confirm-booted stuff to happen for the L1 host via call
function in 'ts-nested-setup' script. So, the setup of /var/core and the associated sysctl.conf 
and limits.d changes should be done in 'ts-nested-setup' too? Or be done in 
'ts-debian-hvm-install' script? Which do you prefer? I prefer to 'ts-nested-setup'.

> The code to setup /var/core and the associated sysctl.conf and limits.d
> changes should be done in the same manner as osstest-confirm-booted
> eventually is based on Ian's suggestion, whatever that was.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Thu, 2015-06-18 at 09:16 +0000, Pang, LongtaoX wrote:
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Wednesday, June 17, 2015 7:49 PM
> > To: Pang, LongtaoX
> > Cc: Hu, Robert; Ian Jackson; xen-devel@lists.xen.org; wei.liu2@citrix.com
> > Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> > test job
> > 
> > On Wed, 2015-06-17 at 11:00 +0000, Pang, LongtaoX wrote:
> > > > > 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp
> > /var/run
> > > > /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> > > > > find: `/var/core': No such file or directory
> > > >
> > > > /var/core is created by ts-host-install. I think the tail end of the
> > > > function sub in there which does that and populates /etc/sysctl.conf
> > > > and /etc/security/limits.d/coredumps.conf should be refactored probably
> > > > to be alongside the osstest-confirm-booted thing which IIRC you are
> > > > already going to refactor in the next version.
> > > >
> > > I reviewed related code in 'ts-host-install'. But I am not very clear, the below
> > code will be executed
> > > in 'ts-debian-hvm-install' too? Or refactor 'osstest-confirm-booted' and the
> > below action should be finished inside this shell script?
> > > Since, it need '/var/core' directory in L1, please correct me.
> > 
> > I think you've already arranged for the osstest-confirm-booted stuff to
> > happen for the L1 host too, but that Ian J has requested it be done
> > differently such that it is not duplicated in two places.
> > 
> Yes, I have arranged for osstest-confirm-booted stuff to happen for the L1 host via call
> function in 'ts-nested-setup' script. So, the setup of /var/core and the associated sysctl.conf 
> and limits.d changes should be done in 'ts-nested-setup' too? Or be done in 
> 'ts-debian-hvm-install' script? Which do you prefer? I prefer to 'ts-nested-setup'.

Ian has already indicated in
<21880.17044.20906.699699@mariner.uk.xensource.com> that the
osstest-confirm-booted stuff should be broken out and not duplicated
into ts-nested-setup. Once you have followed that request then you will
also have an ideal place to do the core dump setup too.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Thursday, June 18, 2015 5:23 PM
> To: Pang, LongtaoX
> Cc: Hu, Robert; Ian Jackson; wei.liu2@citrix.com; xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [OSSTEST Nested PATCH v11 6/7] Compose the main
> recipe of nested test job
> 
> On Thu, 2015-06-18 at 09:16 +0000, Pang, LongtaoX wrote:
> >
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > Sent: Wednesday, June 17, 2015 7:49 PM
> > > To: Pang, LongtaoX
> > > Cc: Hu, Robert; Ian Jackson; xen-devel@lists.xen.org; wei.liu2@citrix.com
> > > Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of
> nested
> > > test job
> > >
> > > On Wed, 2015-06-17 at 11:00 +0000, Pang, LongtaoX wrote:
> > > > > > 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp
> > > /var/run
> > > > > /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> > > > > > find: `/var/core': No such file or directory
> > > > >
> > > > > /var/core is created by ts-host-install. I think the tail end of the
> > > > > function sub in there which does that and populates /etc/sysctl.conf
> > > > > and /etc/security/limits.d/coredumps.conf should be refactored probably
> > > > > to be alongside the osstest-confirm-booted thing which IIRC you are
> > > > > already going to refactor in the next version.
> > > > >
> > > > I reviewed related code in 'ts-host-install'. But I am not very clear, the below
> > > code will be executed
> > > > in 'ts-debian-hvm-install' too? Or refactor 'osstest-confirm-booted' and the
> > > below action should be finished inside this shell script?
> > > > Since, it need '/var/core' directory in L1, please correct me.
> > >
> > > I think you've already arranged for the osstest-confirm-booted stuff to
> > > happen for the L1 host too, but that Ian J has requested it be done
> > > differently such that it is not duplicated in two places.
> > >
> > Yes, I have arranged for osstest-confirm-booted stuff to happen for the L1 host
> via call
> > function in 'ts-nested-setup' script. So, the setup of /var/core and the associated
> sysctl.conf
> > and limits.d changes should be done in 'ts-nested-setup' too? Or be done in
> > 'ts-debian-hvm-install' script? Which do you prefer? I prefer to 'ts-nested-setup'.
> 
> Ian has already indicated in
> <21880.17044.20906.699699@mariner.uk.xensource.com> that the
> osstest-confirm-booted stuff should be broken out and not duplicated
> into ts-nested-setup. Once you have followed that request then you will
> also have an ideal place to do the core dump setup too.
> 
Thanks, I get it.
> Ian.
> 

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Friday, June 12, 2015 11:27 PM
> To: Pang, LongtaoX
> Cc: xen-devel@lists.xen.org; Ian.Campbell@citrix.com; wei.liu2@citrix.com; Hu,
> Robert
> Subject: RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main
> recipe of nested test job"):
> > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> ...
> > > leak-check compares the set of objects present at the `leak-check
> > > check' step with the set of objects present at the `basis' step, and
> > > the check fails if there are any new objects.  For this purpose,
> > > objects includes domains, corefiles, etc.
> > >
> > OK, so the recipe in sg-run-job should be like below, please correct me if
> something wrong.
> > proc need-hosts/test-nested {} {return host}
> > proc run-job/test-nested {} {
> 
> This is roughly right, but thinking about it, you want ts-logs-capture
> to run even if the previous steps fail.
> 
> I think it might be better to reuse (subvert?) the existing machinery
> in sg-run-job, by adding the l1 to need_xen_hosts.
> 
> Maybe something like
> 
>   proc add-xen-host-retrospectively {ident} {
>       global need_xen_hosts
>       ts-leak-check $ident + basis
>       lappend need_xen_hosts $ident
>   }
> 
> ?
> 
> And then call
> 
>   add-xen-host-retrospectively l1
> 
> at the appropriate point.
> 
> If you do this then the main run-job proc will automatically do the
> leak-check and the logs-capture for you.
> 
I have tried your suggestion, call 'add-xen-host- retrospectively l1' just after L1 has booted into XEN.
leak-check and logs-capture will be done automatically at final stage, but this happened after L1 guest destroyed 
and it will failed obviously(I have mentioned this in previous mail). 
So, may I implement these action via below recipe in sg-run-job? Since, this would be less code to 
be changed and we want to avoid to involve tcl plumbing in sg-run-job. Also, I think there is no harmful.
proc need-hosts/test-nested {} {return host}
proc run-job/test-nested {} {
    run-ts . = ts-debian-hvm-install + host l1
    run-ts . = ts-nested-setup + host l1
    run-ts . = ts-xen-install l1
    run-ts . = ts-host-reboot l1
    run-ts . = ts-leak-check basis l1
    run-ts . = ts-debian-hvm-install l1 l2
    run-ts . = ts-guest-stop l1 l2
    run-ts . = ts-leak-check check l1
    run-ts . = ts-logs-capture l1
    run-ts . = ts-guest-destroy + host l1
}
> 
> Thinking about this leads me to ask another question.  Suppose that a
> bug causes the l1 to lock up completely.  ts-logs-capture will attempt
> to hard reboot a locked-up host.  If it can't fetch any logs, it calls
>     target_reboot_hard($ho);
> 
> What will that do if $ho refers to the l1 ?  It relies on the power
> method.  Does your nested l1 "host" have a power method ?
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> So, may I implement these action via below recipe in sg-run-job? Since, this would be less code to 
> be changed and we want to avoid to involve tcl plumbing in sg-run-job. Also, I think there is no harmful.

I don't object to that approach.

But it isn't as simple as you suggest, unfortunately.  Because:

> proc need-hosts/test-nested {} {return host}
> proc run-job/test-nested {} {
>     run-ts . = ts-debian-hvm-install + host l1
>     run-ts . = ts-nested-setup + host l1
>     run-ts . = ts-xen-install l1
>     run-ts . = ts-host-reboot l1
>     run-ts . = ts-leak-check basis l1
>     run-ts . = ts-debian-hvm-install l1 l2
>     run-ts . = ts-guest-stop l1 l2
>     run-ts . = ts-leak-check check l1
>     run-ts . = ts-logs-capture l1
>     run-ts . = ts-guest-destroy + host l1

This will not run ts-logs-capture on l1, unless all the previous tests
passed.  That's obviously not what we want.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Ian Jackson writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
...
> But it isn't as simple as you suggest, unfortunately.  Because:
> 
> > proc need-hosts/test-nested {} {return host}
> > proc run-job/test-nested {} {
> >     run-ts . = ts-debian-hvm-install + host l1
> >     run-ts . = ts-nested-setup + host l1
> >     run-ts . = ts-xen-install l1
> >     run-ts . = ts-host-reboot l1
> >     run-ts . = ts-leak-check basis l1
> >     run-ts . = ts-debian-hvm-install l1 l2
> >     run-ts . = ts-guest-stop l1 l2
> >     run-ts . = ts-leak-check check l1
> >     run-ts . = ts-logs-capture l1
> >     run-ts . = ts-guest-destroy + host l1
> 
> This will not run ts-logs-capture on l1, unless all the previous tests
> passed.  That's obviously not what we want.

I think it would be best if I proposed some code to you.  I will get
back to you with a suggestion.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Pang, LongtaoX]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Wednesday, June 17, 2015 5:35 PM
> To: Pang, LongtaoX
> Cc: Hu, Robert; Ian Jackson; xen-devel@lists.xen.org; wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> test job
> 
> On Wed, 2015-06-17 at 08:54 +0000, Pang, LongtaoX wrote:
> 
> > After executing command ' ./standalone run-job --simulate -h dummy
> test-amd64-amd64-qemuu-nested | grep testid',
> > get below information:
> > root@OSSTEST:~/v11_pretest_3/osstest_v11# ./standalone run-job --simulate -h
> dummy test-amd64-amd64-qemuu-nested | grep testid
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 1 testid build-check(1) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 2 testid hosts-allocate ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 3 testid host-install(3) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 4 testid host-ping-check-native ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 5 testid xen-install ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 6 testid xen-boot ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 7 testid host-ping-check-xen ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 8 testid leak-check/basis(8) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 9 testid debian-hvm-install ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 10 testid nested-setup ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 11 testid xen-install/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 12 testid host-reboot/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 13 testid leak-check/basis/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 14 testid debian-hvm-install/nestedl1/nestedl2 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 15 testid guest-stop/nestedl1/nestedl2 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 16 testid guest-destroy ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 17 testid leak-check/check ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 18 testid leak-check/check/nestedl1 ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 19 testid capture-logs(19) ==========
> > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> ========== 20 testid capture-logs/nestedl1(20) ==========
> > root@OSSTEST:~/v11_pretest_3/osstest_v11#
> >
> > But, for testid of '18', I think it will failed to execute 'leak-check/check/nestedl1',
> since
> > 'nestedl1' has been destroyed via the action of 'run-ts . = ts-guest-destroy + host
> nestedl1'.
> > Please correct me if I am wrong.
> 
> I think you are correct, the logs capture will fail too.
> 
> I'll leave it to Ian to suggest a solution since it will no doubt
> involve some tcl plumbing (I'd be inclined to record 'hosts which are
> actually guests' somewhere and have the infra clean them up
> automatically after doing leak check and log collection).
> 
> > Another question, after execute testid of '13'(leak-check/basis/nestedl1), the
> testing will fail and exit with below message.
> > Something wrong with my test environment?
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xl list
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xenstore-ls -fp
> > 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp /var/run
> /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> > find: `/var/core': No such file or directory
> 
> /var/core is created by ts-host-install. I think the tail end of the
> function sub in there which does that and populates /etc/sysctl.conf
> and /etc/security/limits.d/coredumps.conf should be refactored probably
> to be alongside the osstest-confirm-booted thing which IIRC you are
> already going to refactor in the next version.
> 
> > > Perhaps arrange for an appropriate PowerMethod for "hosts which are
> > > actually guests"?
> > >
> > I think maybe we need to refactor 'power_cycle' function in TestSupport.pm. I
> have not try it, something like below?
> 
> I was thinking more along the lines of creating Osstest/PDU/guest.pm
> with the appropriate methods calling out to toolstack($l0)->foo, setting
> $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> power_cycle_host_setup to do it's thing.
> 
I have reviewed power_cycle_host_setup function, inside this function will call 
get_host_method_object, then we could get a $mo which will be assigned to 
$ho->{PowerMethobjs}, right?
Inside power_state function, it will call pdu_power_state which is defined in 
guest.pm, right?
So, I need to defined how to power off/on L1 inside pdu_power_state function? I think we
need to using 'xl destroy' and 'xl create' command to implement the power
method.
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Thu, 2015-06-25 at 08:21 +0000, Pang, LongtaoX wrote:
> 
> 
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: Wednesday, June 17, 2015 5:35 PM
> > To: Pang, LongtaoX
> > Cc: Hu, Robert; Ian Jackson; xen-devel@lists.xen.org; wei.liu2@citrix.com
> > Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested
> > test job
> > 
> > On Wed, 2015-06-17 at 08:54 +0000, Pang, LongtaoX wrote:
> > 
> > > After executing command ' ./standalone run-job --simulate -h dummy
> > test-amd64-amd64-qemuu-nested | grep testid',
> > > get below information:
> > > root@OSSTEST:~/v11_pretest_3/osstest_v11# ./standalone run-job --simulate -h
> > dummy test-amd64-amd64-qemuu-nested | grep testid
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 1 testid build-check(1) ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 2 testid hosts-allocate ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 3 testid host-install(3) ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 4 testid host-ping-check-native ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 5 testid xen-install ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 6 testid xen-boot ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 7 testid host-ping-check-xen ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 8 testid leak-check/basis(8) ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 9 testid debian-hvm-install ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 10 testid nested-setup ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 11 testid xen-install/nestedl1 ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 12 testid host-reboot/nestedl1 ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 13 testid leak-check/basis/nestedl1 ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 14 testid debian-hvm-install/nestedl1/nestedl2 ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 15 testid guest-stop/nestedl1/nestedl2 ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 16 testid guest-destroy ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 17 testid leak-check/check ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 18 testid leak-check/check/nestedl1 ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 19 testid capture-logs(19) ==========
> > > 2015-06-17 07:33:35 Z standalone.test-amd64-amd64-qemuu-nested
> > ========== 20 testid capture-logs/nestedl1(20) ==========
> > > root@OSSTEST:~/v11_pretest_3/osstest_v11#
> > >
> > > But, for testid of '18', I think it will failed to execute 'leak-check/check/nestedl1',
> > since
> > > 'nestedl1' has been destroyed via the action of 'run-ts . = ts-guest-destroy + host
> > nestedl1'.
> > > Please correct me if I am wrong.
> > 
> > I think you are correct, the logs capture will fail too.
> > 
> > I'll leave it to Ian to suggest a solution since it will no doubt
> > involve some tcl plumbing (I'd be inclined to record 'hosts which are
> > actually guests' somewhere and have the infra clean them up
> > automatically after doing leak check and log collection).
> > 
> > > Another question, after execute testid of '13'(leak-check/basis/nestedl1), the
> > testing will fail and exit with below message.
> > > Something wrong with my test environment?
> > > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xl list
> > > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> > > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 ps -wwef
> > > 2015-06-17 08:10:25 Z executing ssh ... root@192.168.199.25 xenstore-ls -fp
> > > 2015-06-17 08:10:26 Z executing ssh ... root@192.168.199.25 find /tmp /var/run
> > /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0'
> > > find: `/var/core': No such file or directory
> > 
> > /var/core is created by ts-host-install. I think the tail end of the
> > function sub in there which does that and populates /etc/sysctl.conf
> > and /etc/security/limits.d/coredumps.conf should be refactored probably
> > to be alongside the osstest-confirm-booted thing which IIRC you are
> > already going to refactor in the next version.
> > 
> > > > Perhaps arrange for an appropriate PowerMethod for "hosts which are
> > > > actually guests"?
> > > >
> > > I think maybe we need to refactor 'power_cycle' function in TestSupport.pm. I
> > have not try it, something like below?
> > 
> > I was thinking more along the lines of creating Osstest/PDU/guest.pm
> > with the appropriate methods calling out to toolstack($l0)->foo, setting
> > $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> > power_cycle_host_setup to do it's thing.
> > 
> I have reviewed power_cycle_host_setup function, inside this function will call 
> get_host_method_object, then we could get a $mo which will be assigned to 
> $ho->{PowerMethobjs}, right?

Yes. Note that PowerMethobjs is an array rather than a single scalar,
since you can have a sequence of $mo's which are needed for a given
host.

> Inside power_state function, it will call pdu_power_state which is defined in 
> guest.pm, right?

It will call $mo->pdu_power_state for each $mo in PowerMethobjs, yes.

> So, I need to defined how to power off/on L1 inside pdu_power_state function?

In a new Osstest::PDU::guest module, yes.

>  I think we
> need to using 'xl destroy' and 'xl create' command to implement the power
> method.

I think so, yes.

In one of your patches you rely on the affect of "xl block-attach"
persisting after an "xl reboot", while it won't with "xl destroy"
followed by "xl create". IOW in that patch you may want to also update
the config file too.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
...
> > I think you are correct, the logs capture will fail too.
> > 
> > I'll leave it to Ian to suggest a solution since it will no doubt
> > involve some tcl plumbing (I'd be inclined to record 'hosts which are
> > actually guests' somewhere and have the infra clean them up
> > automatically after doing leak check and log collection).

Sorry I haven't done this yet, it's still on my radar.

> > I was thinking more along the lines of creating Osstest/PDU/guest.pm
> > with the appropriate methods calling out to toolstack($l0)->foo, setting
> > $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> > power_cycle_host_setup to do it's thing.
> 
> I have reviewed power_cycle_host_setup function, inside this
> function will call get_host_method_object, then we could get a $mo
> which will be assigned to $ho->{PowerMethobjs}, right?  Inside
> power_state function, it will call pdu_power_state which is defined
> in guest.pm, right?

Yes.

> So, I need to defined how to power off/on L1 inside pdu_power_state
> function? I think we need to using 'xl destroy' and 'xl create'
> command to implement the power method.

Indeed.  You'll need to use the appropriate toolstack object, in case
it's libvirt or something.  toolstack($ho) where $ho is the L0.

Ian.

[OSSTEST RFC PATCH 0/4] Nested job execution infrastructure

[Ian Jackson]

I wrote:
> I think it would be best if I proposed some code to you.  I will get
> back to you with a suggestion.

Here is the result.  Please let me know what you think.

NB that this is an RFC and I have NOT EXECUTED IT.  It is easy in Tcl
to make daft mistakes like missing `global' or misspelling variable
names, so I apologise in advance for the bugs.


The first two patches are, I think, uncontroversial:

  1/4 Tcl: Provide lunappend
  2/4 sg-run-job: Declare Tcl (for the benefit of Emacs)

The next one makes an assumption about how much you want to reuse of
the standard host setup machinery.  I think you want to go back at
least to ts-xen-install ?  In which case this is probably
approximately correct.

  3/4 sg-run-job: Break out per-host-prep and per-host-finish

The final patch provides the functionality you will need for your proc
run-job/test-nested:

  4/4 sg-run-job: Provide infrastructure for layers of nesting

You would use this by writing someething like this:

  proc run-job/test-nested {} {
      run-ts . = ts-debian-hvm-install + host l1
      run-ts . = ts-nested-setup + host l1
      nested-layer-descend l1
      run-ts . = ts-debian-hvm-install l1 l2
      ...

And there is then no need to do anything special at the end of the
test: the nested layering part will leak check and log capture the l1,
tear it down, and then do the same to the l0.


If you like the look of this approach, please feel free to fold these
patches into your own series at the appropriate point (retaining my
From: and S-o-b would be conventional).

Before posting them other than as an RFC, you will have to test them,
I'm afraid.  If it doesn't work and you can't figure it out please ask
me and I will try to reply quickly.

When you have tested them, you can add your own Tested-by to the
patch tags next to by Signed-off-by.


I hope this is helpful.

Thanks,
Ian.

[OSSTEST PATCH 1/4] Tcl: Provide lunappend

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 tcl/osstestlib.tcl |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tcl/osstestlib.tcl b/tcl/osstestlib.tcl
index 61a6a09..b5a52d3 100644
--- a/tcl/osstestlib.tcl
+++ b/tcl/osstestlib.tcl
@@ -75,6 +75,13 @@ proc lshift {listvar} {
     return $head
 }
 
+proc lunappend {listvar} {
+    upvar 1 $listvar list
+    set tail [lindex $list end]
+    set list [lrange $list 0 end-1]
+    return $tail
+}
+
 proc var-or-default {varname {default {}}} {
     upvar 1 $varname var
     if {[info exists var]} { return $var }
-- 
1.7.10.4

[OSSTEST PATCH 2/4] sg-run-job: Declare Tcl (for the benefit of Emacs)

[Ian Jackson]

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-run-job |    1 +
 1 file changed, 1 insertion(+)

diff --git a/sg-run-job b/sg-run-job
index d53fd83..33a836b 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -1,4 +1,5 @@
 #!/usr/bin/tclsh8.4
+# -*- Tcl -*-
 
 # This is part of "osstest", an automated testing framework for Xen.
 # Copyright (C) 2009-2013 Citrix Inc.
-- 
1.7.10.4

[OSSTEST PATCH 3/4] sg-run-job: Break out per-host-prep and per-host-finish

[Ian Jackson]

No functional change.

We now call the per-host-ts finish steps unconditionally, rather than
only if !$need_build_host, per-host-ts is (complicated) no-op if
$need_build_host, since in that case $need_xen_hosts is {}.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-run-job |   31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/sg-run-job b/sg-run-job
index 33a836b..312b0d7 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -24,6 +24,20 @@ source ./tcl/osstestlib.tcl
 readconfig
 source-method JobDB
 
+proc per-host-prep {} {
+    per-host-ts .       host-ping-check-native/@ ts-host-ping-check
+    per-host-ts .       xen-install/@     ts-xen-install
+    per-host-ts .       xen-boot/@        ts-host-reboot
+
+    per-host-ts .       host-ping-check-xen/@ ts-host-ping-check
+    per-host-ts .       =(*)             {ts-leak-check basis}
+}
+
+proc per-host-finish {} {
+    per-host-ts .       =                {ts-leak-check check}
+    per-host-ts !broken capture-logs/@(*) ts-logs-capture
+}
+
 proc run-job {job} {
     global jobinfo builds flight ok need_xen_hosts anyfailed
 
@@ -52,22 +66,15 @@ proc run-job {job} {
     if {$ok} { setstatus running                                          }
 
     per-host-ts broken  host-install/@(*) ts-host-install-twice
-    per-host-ts .       host-ping-check-native/@ ts-host-ping-check
-    per-host-ts .       xen-install/@     ts-xen-install
-    per-host-ts .       xen-boot/@        ts-host-reboot
-    per-host-ts .       host-ping-check-xen/@ ts-host-ping-check
 
-    per-host-ts .       =(*)             {ts-leak-check basis}
+    per-hosts-prep
 
     if {$ok} { catching-otherwise fail      run-job/$jobinfo(recipe)      }
-    per-host-ts .       =                {ts-leak-check check}
 
-    if {!$need_build_host} {
-        per-host-ts !broken capture-logs/@(*) ts-logs-capture
-    } else {
-        if {$anyfailed} {
-            run-ts  !broken capture-logs      ts-logs-capture + host
-        }
+    per-host-finish
+
+    if {$need_build_host && $anyfailed} {
+	run-ts  !broken capture-logs      ts-logs-capture + host
     }
 
     if {$ok} { setstatus pass                                             }
-- 
1.7.10.4

[OSSTEST PATCH 4/4] sg-run-job: Provide infrastructure for layers of nesting

[Ian Jackson]

Provides nested-layer-descend, which can be called in an individual
test job at the appropriate point (after the L1 has been set up).

The inner host is a guest of the outer host; powering it off means
destroying it.  Putting the poweroff at this point in the loop, rather
than in per-host-finish, avoids powering off physical servers.  The
use of `.'  rather than `!.' for iffail means we do not power off
after failures (as we might want to preserve the state for debugging
etc).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-run-job |   21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/sg-run-job b/sg-run-job
index 312b0d7..c0fbc78 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -40,6 +40,7 @@ proc per-host-finish {} {
 
 proc run-job {job} {
     global jobinfo builds flight ok need_xen_hosts anyfailed
+    global nested_layers_hosts
 
     set ok 1
     set anyfailed 0
@@ -53,6 +54,7 @@ proc run-job {job} {
         set need_xen_hosts $nh
         set need_build_host 0
     }
+    set nested_layers_hosts {}
 
     catching-otherwise blocked            check-not-blocked
     if {!$ok} return
@@ -71,7 +73,15 @@ proc run-job {job} {
 
     if {$ok} { catching-otherwise fail      run-job/$jobinfo(recipe)      }
 
-    per-host-finish
+    while 1 {
+	per-host-finish
+
+	if {![llength nested_layers_hosts]} break
+
+	per-host-ts . = final-poweroff {ts-host-powercycle --power=0}
+
+        set need_xen_hosts [lunappend nested_layers_hosts]
+    }
 
     if {$need_build_host && $anyfailed} {
 	run-ts  !broken capture-logs      ts-logs-capture + host
@@ -246,6 +256,15 @@ proc per-host-ts {iffail ident script args} {
     }
 }
 
+proc nested-layer-descend {nested_hosts} {
+    # We save need_xen_hosts on a stack in nested_layers_hosts
+    # It gets popped again during the cleanup part of run-job
+    global nested_layers_hosts need_xen_hosts
+    lappend nested_layers_hosts $need_xen_hosts
+    set need_xen_hosts $nested_hosts
+    per-host-prep
+}
+
 #---------- test recipes ----------
 
 proc need-hosts/test-debian-nomigr {} { return host }
-- 
1.7.10.4

Re: [OSSTEST RFC PATCH 0/4] Nested job execution infrastructure

[Ian Jackson]

Ian Jackson writes ("[OSSTEST RFC PATCH 0/4] Nested job execution infrastructure"):
> Here is the result.  Please let me know what you think.

Oh, I should say: I have put this on xenbits, for if you prefer a `git
fetch':

   http://xenbits.xen.org/gitweb/?p=people/iwj/osstest.git;a=summary
   git://xenbits.xen.org/people/iwj/osstest.git
      wip.nested-sgrun-infra-poc.v1

Ian.

Re: [OSSTEST RFC PATCH 0/4] Nested job execution infrastructure

[Robert Hu]

On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> I wrote:
> > I think it would be best if I proposed some code to you.  I will get
> > back to you with a suggestion.
> 
> Here is the result.  Please let me know what you think.
> 
> NB that this is an RFC and I have NOT EXECUTED IT.  It is easy in Tcl
> to make daft mistakes like missing `global' or misspelling variable
> names, so I apologise in advance for the bugs.
> 
> 
> The first two patches are, I think, uncontroversial:
> 
>   1/4 Tcl: Provide lunappend
>   2/4 sg-run-job: Declare Tcl (for the benefit of Emacs)
> 
> The next one makes an assumption about how much you want to reuse of
> the standard host setup machinery.  I think you want to go back at
> least to ts-xen-install ?  In which case this is probably
> approximately correct.
> 
>   3/4 sg-run-job: Break out per-host-prep and per-host-finish
> 
> The final patch provides the functionality you will need for your proc
> run-job/test-nested:
> 
>   4/4 sg-run-job: Provide infrastructure for layers of nesting
> 
> You would use this by writing someething like this:
> 
>   proc run-job/test-nested {} {
>       run-ts . = ts-debian-hvm-install + host l1
>       run-ts . = ts-nested-setup + host l1
>       nested-layer-descend l1
>       run-ts . = ts-debian-hvm-install l1 l2
>       ...
> 
> And there is then no need to do anything special at the end of the
> test: the nested layering part will leak check and log capture the l1,
> tear it down, and then do the same to the l0.
> 
> 
> If you like the look of this approach, please feel free to fold these
> patches into your own series at the appropriate point (retaining my
> From: and S-o-b would be conventional).
Sure. Thanks Ian. 'S-o-b' is signed-off-by, right?
I'm afraid I'm busy on other stuff at present. I'll try them after
stuffs at hand are done; perhaps 2 weeks later. Sorry about this.
> 
> Before posting them other than as an RFC, you will have to test them,
> I'm afraid.  If it doesn't work and you can't figure it out please ask
> me and I will try to reply quickly.
> 
> When you have tested them, you can add your own Tested-by to the
> patch tags next to by Signed-off-by.
> 
> 
> I hope this is helpful.
> 
> Thanks,
> Ian.

Re: [OSSTEST RFC PATCH 0/4] Nested job execution infrastructure

[Ian Jackson]

Robert Hu writes ("Re: [OSSTEST RFC PATCH 0/4] Nested job execution infrastructure"):
> On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> > If you like the look of this approach, please feel free to fold these
> > patches into your own series at the appropriate point (retaining my
> > From: and S-o-b would be conventional).
> 
> Sure. Thanks Ian. 'S-o-b' is signed-off-by, right?

Yes.

> I'm afraid I'm busy on other stuff at present. I'll try them after
> stuffs at hand are done; perhaps 2 weeks later. Sorry about this.

No problem at all.

Thanks,
Ian.

Re: [OSSTEST RFC PATCH 0/4] Nested job execution infrastructure

[Robert Hu]

On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> I wrote:
> > I think it would be best if I proposed some code to you.  I will get
> > back to you with a suggestion.
> 
> Here is the result.  Please let me know what you think.
> 
> NB that this is an RFC and I have NOT EXECUTED IT.  It is easy in Tcl
> to make daft mistakes like missing `global' or misspelling variable
> names, so I apologise in advance for the bugs.
Hi Ian,
Just had some time to integrate your patch. Here is some issue I met:
Once some ts fails, seems the sg-run-job cannot return.
> 
> 
> The first two patches are, I think, uncontroversial:
> 
>   1/4 Tcl: Provide lunappend
>   2/4 sg-run-job: Declare Tcl (for the benefit of Emacs)
> 
> The next one makes an assumption about how much you want to reuse of
> the standard host setup machinery.  I think you want to go back at
> least to ts-xen-install ?  In which case this is probably
> approximately correct.
> 
>   3/4 sg-run-job: Break out per-host-prep and per-host-finish
> 
> The final patch provides the functionality you will need for your proc
> run-job/test-nested:
> 
>   4/4 sg-run-job: Provide infrastructure for layers of nesting
> 
> You would use this by writing someething like this:
> 
>   proc run-job/test-nested {} {
>       run-ts . = ts-debian-hvm-install + host l1
>       run-ts . = ts-nested-setup + host l1
>       nested-layer-descend l1
>       run-ts . = ts-debian-hvm-install l1 l2
>       ...
> 
> And there is then no need to do anything special at the end of the
> test: the nested layering part will leak check and log capture the l1,
> tear it down, and then do the same to the l0.
> 
> 
> If you like the look of this approach, please feel free to fold these
> patches into your own series at the appropriate point (retaining my
> From: and S-o-b would be conventional).
> 
> Before posting them other than as an RFC, you will have to test them,
> I'm afraid.  If it doesn't work and you can't figure it out please ask
> me and I will try to reply quickly.
> 
> When you have tested them, you can add your own Tested-by to the
> patch tags next to by Signed-off-by.
> 
> 
> I hope this is helpful.
> 
> Thanks,
> Ian.

Re: [OSSTEST PATCH 3/4] sg-run-job: Break out per-host-prep and per-host-finish

[Robert Hu]

On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> No functional change.
> 
> We now call the per-host-ts finish steps unconditionally, rather than
> only if !$need_build_host, per-host-ts is (complicated) no-op if
> $need_build_host, since in that case $need_xen_hosts is {}.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  sg-run-job |   31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/sg-run-job b/sg-run-job
> index 33a836b..312b0d7 100755
> --- a/sg-run-job
> +++ b/sg-run-job
> @@ -24,6 +24,20 @@ source ./tcl/osstestlib.tcl
>  readconfig
>  source-method JobDB
>  
> +proc per-host-prep {} {
> +    per-host-ts .       host-ping-check-native/@ ts-host-ping-check
> +    per-host-ts .       xen-install/@     ts-xen-install
> +    per-host-ts .       xen-boot/@        ts-host-reboot
> +
> +    per-host-ts .       host-ping-check-xen/@ ts-host-ping-check
> +    per-host-ts .       =(*)             {ts-leak-check basis}
> +}
> +
> +proc per-host-finish {} {
> +    per-host-ts .       =                {ts-leak-check check}
> +    per-host-ts !broken capture-logs/@(*) ts-logs-capture
> +}
> +
>  proc run-job {job} {
>      global jobinfo builds flight ok need_xen_hosts anyfailed
>  
> @@ -52,22 +66,15 @@ proc run-job {job} {
>      if {$ok} { setstatus running                                          }
>  
>      per-host-ts broken  host-install/@(*) ts-host-install-twice
> -    per-host-ts .       host-ping-check-native/@ ts-host-ping-check
> -    per-host-ts .       xen-install/@     ts-xen-install
> -    per-host-ts .       xen-boot/@        ts-host-reboot
> -    per-host-ts .       host-ping-check-xen/@ ts-host-ping-check
>  
> -    per-host-ts .       =(*)             {ts-leak-check basis}
> +    per-hosts-prep
a trivial typo, I think 'per-host-prep' shall be.
>  
>      if {$ok} { catching-otherwise fail      run-job/$jobinfo(recipe)      }
> -    per-host-ts .       =                {ts-leak-check check}
>  
> -    if {!$need_build_host} {
> -        per-host-ts !broken capture-logs/@(*) ts-logs-capture
> -    } else {
> -        if {$anyfailed} {
> -            run-ts  !broken capture-logs      ts-logs-capture + host
> -        }
> +    per-host-finish
> +
> +    if {$need_build_host && $anyfailed} {
> +	run-ts  !broken capture-logs      ts-logs-capture + host
>      }
>  
>      if {$ok} { setstatus pass                                             }

Re: [OSSTEST PATCH 4/4] sg-run-job: Provide infrastructure for layers of nesting

[Robert Hu]

On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> Provides nested-layer-descend, which can be called in an individual
> test job at the appropriate point (after the L1 has been set up).
> 
> The inner host is a guest of the outer host; powering it off means
> destroying it.  Putting the poweroff at this point in the loop, rather
> than in per-host-finish, avoids powering off physical servers.  The
> use of `.'  rather than `!.' for iffail means we do not power off
> after failures (as we might want to preserve the state for debugging
> etc).
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  sg-run-job |   21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/sg-run-job b/sg-run-job
> index 312b0d7..c0fbc78 100755
> --- a/sg-run-job
> +++ b/sg-run-job
> @@ -40,6 +40,7 @@ proc per-host-finish {} {
>  
>  proc run-job {job} {
>      global jobinfo builds flight ok need_xen_hosts anyfailed
> +    global nested_layers_hosts
>  
>      set ok 1
>      set anyfailed 0
> @@ -53,6 +54,7 @@ proc run-job {job} {
>          set need_xen_hosts $nh
>          set need_build_host 0
>      }
> +    set nested_layers_hosts {}
>  
>      catching-otherwise blocked            check-not-blocked
>      if {!$ok} return
> @@ -71,7 +73,15 @@ proc run-job {job} {
>  
>      if {$ok} { catching-otherwise fail      run-job/$jobinfo(recipe)      }
>  
> -    per-host-finish
> +    while 1 {
> +	per-host-finish
> +
> +	if {![llength nested_layers_hosts]} break
The can-not-return issue roots here, [llength nested_layers_hosts]
returns 1, while initially you've set it to blank list. Strange.
> +
> +	per-host-ts . = final-poweroff {ts-host-powercycle --power=0}
> +
> +        set need_xen_hosts [lunappend nested_layers_hosts]
> +    }
>  
>      if {$need_build_host && $anyfailed} {
>  	run-ts  !broken capture-logs      ts-logs-capture + host
> @@ -246,6 +256,15 @@ proc per-host-ts {iffail ident script args} {
>      }
>  }
>  
> +proc nested-layer-descend {nested_hosts} {
> +    # We save need_xen_hosts on a stack in nested_layers_hosts
> +    # It gets popped again during the cleanup part of run-job
> +    global nested_layers_hosts need_xen_hosts
> +    lappend nested_layers_hosts $need_xen_hosts
> +    set need_xen_hosts $nested_hosts
> +    per-host-prep
> +}
> +
>  #---------- test recipes ----------
>  
>  proc need-hosts/test-debian-nomigr {} { return host }

Re: [OSSTEST PATCH 4/4] sg-run-job: Provide infrastructure for layers of nesting

[Ian Jackson]

Robert Hu writes ("Re: [OSSTEST PATCH 4/4] sg-run-job: Provide infrastructure for layers of nesting"):
> On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> > +	if {![llength nested_layers_hosts]} break
>
> The can-not-return issue roots here, [llength nested_layers_hosts]
> returns 1, while initially you've set it to blank list. Strange.

That line should read

    +	if {![llength $nested_layers_hosts]} break
    
Sorry!

Ian.

Re: [OSSTEST PATCH 3/4] sg-run-job: Break out per-host-prep and per-host-finish

[Ian Jackson]

Robert Hu writes ("Re: [OSSTEST PATCH 3/4] sg-run-job: Break out per-host-prep and per-host-finish"):
> On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> > -    per-host-ts .       =(*)             {ts-leak-check basis}
> > +    per-hosts-prep
>
> a trivial typo, I think 'per-host-prep' shall be.

Yes.

Sorry that I have sent you buggy code.  It seems that my warning

   NB that this is an RFC and I have NOT EXECUTED IT.  It is easy in
   Tcl to make daft mistakes like missing `global' or misspelling
   variable names, so I apologise in advance for the bugs.

was fully justified.

Ian.

Re: [OSSTEST PATCH 3/4] sg-run-job: Break out per-host-prep and per-host-finish

[Robert Hu]

On Tue, 2015-07-28 at 16:15 +0100, Ian Jackson wrote:
> Robert Hu writes ("Re: [OSSTEST PATCH 3/4] sg-run-job: Break out per-host-prep and per-host-finish"):
> > On Tue, 2015-06-30 at 17:36 +0100, Ian Jackson wrote:
> > > -    per-host-ts .       =(*)             {ts-leak-check basis}
> > > +    per-hosts-prep
> >
> > a trivial typo, I think 'per-host-prep' shall be.
> 
> Yes.
> 
> Sorry that I have sent you buggy code.  It seems that my warning
No, no apologies necessary at all. Thanks for your help on making
osstest job scheduler accommodated to nested test cases.
> 
>    NB that this is an RFC and I have NOT EXECUTED IT.  It is easy in
>    Tcl to make daft mistakes like missing `global' or misspelling
>    variable names, so I apologise in advance for the bugs.
> 
> was fully justified.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Hu, Robert]

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Thursday, June 25, 2015 6:22 PM
> To: Pang, LongtaoX
> Cc: Ian Campbell; Hu, Robert; xen-devel@lists.xen.org; wei.liu2@citrix.com
> Subject: RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of
> nested test job
> 
> Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the
> main recipe of nested test job"):
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> ...
> > > I think you are correct, the logs capture will fail too.
> > >
> > > I'll leave it to Ian to suggest a solution since it will no doubt
> > > involve some tcl plumbing (I'd be inclined to record 'hosts which are
> > > actually guests' somewhere and have the infra clean them up
> > > automatically after doing leak check and log collection).
> 
> Sorry I haven't done this yet, it's still on my radar.
> 
> > > I was thinking more along the lines of creating Osstest/PDU/guest.pm
> > > with the appropriate methods calling out to toolstack($l0)->foo, setting
> > > $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> > > power_cycle_host_setup to do it's thing.
> >
> > I have reviewed power_cycle_host_setup function, inside this
> > function will call get_host_method_object, then we could get a $mo
> > which will be assigned to $ho->{PowerMethobjs}, right?  Inside
> > power_state function, it will call pdu_power_state which is defined
> > in guest.pm, right?
> 
> Yes.
I'm not quite sure about how @$methobjs is obtained.
sub power_cycle_host_setup ($) {
    my ($ho) = @_;
    my $methobjs = [ ];
    foreach my $meth (split /\;\s*/, $ho->{Power}) {
	push @$methobjs, get_host_method_object($ho,'PDU',$meth);
    }
    $ho->{PowerMethobjs} = $methobjs;
}
Seems it is derived from $ho->{Power} while $ho->{Power} is defined by either:
1. selecthost()
	$ho->{Power}= get_host_property($ho,'power-method');
2. default_methods()
	$ho->{Power} ||= "statedb $ho->{Asset}";
  Seems no statedb for standalone mode.

So how shall I assign $ho->{Power} method? by statically through config file?
or some generic way automatically identify $host is some guest, therefore
assign 'guest' to $ho->{Power}?
Would you give me some idea? thanks.
> 
> > So, I need to defined how to power off/on L1 inside pdu_power_state
> > function? I think we need to using 'xl destroy' and 'xl create'
> > command to implement the power method.
> 
> Indeed.  You'll need to use the appropriate toolstack object, in case
> it's libvirt or something.  toolstack($ho) where $ho is the L0.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Tue, 2015-08-18 at 06:46 +0000, Hu, Robert wrote:

Sorry for the delay, I've been away.

> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Thursday, June 25, 2015 6:22 PM
> > To: Pang, LongtaoX
> > Cc: Ian Campbell; Hu, Robert; xen-devel@lists.xen.org; 
> > wei.liu2@citrix.com
> > Subject: RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of
> > nested test job
> > 
> > Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the
> > main recipe of nested test job"):
> > > > -----Original Message-----
> > > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > ...
> > > > I think you are correct, the logs capture will fail too.
> > > > 
> > > > I'll leave it to Ian to suggest a solution since it will no doubt
> > > > involve some tcl plumbing (I'd be inclined to record 'hosts which 
> > > > are
> > > > actually guests' somewhere and have the infra clean them up
> > > > automatically after doing leak check and log collection).
> > 
> > Sorry I haven't done this yet, it's still on my radar.
> > 
> > > > I was thinking more along the lines of creating 
> > > > Osstest/PDU/guest.pm
> > > > with the appropriate methods calling out to toolstack($l0)->foo, 
> > > > setting
> > > > $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> > > > power_cycle_host_setup to do it's thing.
> > > 
> > > I have reviewed power_cycle_host_setup function, inside this
> > > function will call get_host_method_object, then we could get a $mo
> > > which will be assigned to $ho->{PowerMethobjs}, right?  Inside
> > > power_state function, it will call pdu_power_state which is defined
> > > in guest.pm, right?
> > 
> > Yes.
> I'm not quite sure about how @$methobjs is obtained.
> sub power_cycle_host_setup ($) {
>     my ($ho) = @_;
>     my $methobjs = [ ];
>     foreach my $meth (split /\;\s*/, $ho->{Power}) {
> 	push @$methobjs, get_host_method_object($ho,'PDU',$meth);
>     }
>     $ho->{PowerMethobjs} = $methobjs;
> }
> Seems it is derived from $ho->{Power} while $ho->{Power} is defined by 
> either:
> 1. selecthost()
> 	$ho->{Power}= get_host_property($ho,'power-method');
> 2. default_methods()
> 	$ho->{Power} ||= "statedb $ho->{Asset}";
>   Seems no statedb for standalone mode.

Indeed, statedb is actually peculiar to the old Cambridge instance of
osstest, it's not used in the production colo. In fact it is also not used
in Cambridge any more, every host has a power-method host property. In
standalone mode the default {Power} is "manual" I think, that's the one
which prompts you to press a key.

The {Power} property is essentially a ";" separated list, each list entry
is "<module> <arg1> <arg2> <...>". <module> is Osstest/PDU/<module>.pm and
<arg1> <arg2> <...> are passed to the constructor. These objects are
constructed by get_host_method_object.

Then on power operation the appropriate method each object is called in
turn (so you can call multiple modules if needed by the host).

> So how shall I assign $ho->{Power} method? by statically through config
> file?
> or some generic way automatically identify $host is some guest, therefore
> assign 'guest' to $ho->{Power}?
> Would you give me some idea? thanks.

I'm not sure. Something in selecthost would need to know somehow that the
host being requested was actually an L1 guest.

Ian, any ideas?

Ian

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Hi Robert.  Sorry I haven't had a chance to look at your whole v11
series properly yet.  But I will reply here:

Ian Campbell writes ("Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> Then on power operation the appropriate method each object is called in
> turn (so you can call multiple modules if needed by the host).

Ian's explanation is right.

> > So how shall I assign $ho->{Power} method? by statically through config
> > file?
> > or some generic way automatically identify $host is some guest, therefore
> > assign 'guest' to $ho->{Power}?
> > Would you give me some idea? thanks.
> 
> I'm not sure. Something in selecthost would need to know somehow that the
> host being requested was actually an L1 guest.

I think this would be correct.

> Ian, any ideas?

I was expecting there to be a runvar which would for an L1 guest give
the ident of its host.  ts-nested-setup would set this runvar and
selecthost would look at it.

There are various possible exact syntaxes but I think the best one is
probably that the runvar which ordinarily gives the name of a real
host instead contains the containing host name and the L1 guest name.

So if you do
  ./ts-nested-setup l0_host l1
then you get a runvar `l1' with value `l0_host:l1'.

selecthost would see the colon, and set $ho->{Host} to `l0_host', so
that get_target_property's recursion into containing hosts works
properly.

selecthost would also be able to see that $ho->{Host} being set meant
it was a nested guest, and that therefore power operations should be
done by running toolstack operations on the host.  (Rather than
looking up a host property.)

It also needs to arrange that for an L1 (well, anything with
$ho->{Host} aka the _nestedhost runvar), the MAC address is read (into
$ho) from the runvar which is created by the guest creation, rather
than being looked up as a host property.  Then the functions for
finding the IP address by asking the DHCP server have the right input.

The $ho->{DiskDevice} setting in selecthost is wrong too.

This is all probably most easily achieved by having the part of
selecthost which deals with L1 guests seed $ho->{Properties} with
appropriate information.

You want to make "----- calculation of the host's properties -----"
conditional, and do something entirely different (ie, populate
$ho->{Properties} and perhaps some of $ho->{Flags}) when $ho->{Host}
is set.

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Campbell]

On Tue, 2015-09-01 at 15:41 +0100, Ian Jackson wrote:
> Hi Robert.  Sorry I haven't had a chance to look at your whole v11
> series properly yet.  But I will reply here:
> 
> Ian Campbell writes ("Re: [OSSTEST Nested PATCH v11 6/7] Compose the main 
> recipe of nested test job"):
> > Then on power operation the appropriate method each object is called in
> > turn (so you can call multiple modules if needed by the host).
> 
> Ian's explanation is right.
> 
> > > So how shall I assign $ho->{Power} method? by statically through 
> > > config
> > > file?
> > > or some generic way automatically identify $host is some guest, 
> > > therefore
> > > assign 'guest' to $ho->{Power}?
> > > Would you give me some idea? thanks.
> > 
> > I'm not sure. Something in selecthost would need to know somehow that 
> > the
> > host being requested was actually an L1 guest.
> 
> I think this would be correct.
> 
> > Ian, any ideas?
> 
> I was expecting there to be a runvar which would for an L1 guest give
> the ident of its host.  ts-nested-setup would set this runvar and
> selecthost would look at it.
> 
> There are various possible exact syntaxes but I think the best one is
> probably that the runvar which ordinarily gives the name of a real
> host instead contains the containing host name and the L1 guest name.
> 
> So if you do
>   ./ts-nested-setup l0_host l1
> then you get a runvar `l1' with value `l0_host:l1'.
> 
> selecthost would see the colon, and set $ho->{Host} to `l0_host', so
> that get_target_property's recursion into containing hosts works
> properly.
> 
> selecthost would also be able to see that $ho->{Host} being set meant
> it was a nested guest, and that therefore power operations should be
> done by running toolstack operations on the host.  (Rather than
> looking up a host property.)
> 
> It also needs to arrange that for an L1 (well, anything with
> $ho->{Host} aka the _nestedhost runvar),

ITYM:         aka a host runvar containing a colon),

Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Hu, Robert]

Thanks Ian.
So strange, seems this mail was sent ' Tuesday, September 1, 2015 10:42 PM ', but I 
have just received it.
I'm fully occupied by some release test, so have to carefully read your comments 1 ~2
weeks later. Sorry about this.

Best Regards,
Robert Ho

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Tuesday, September 1, 2015 10:42 PM
> To: Ian Campbell
> Cc: Hu, Robert; xen-devel@lists.xen.org; wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of
> nested test job
> 
> Hi Robert.  Sorry I haven't had a chance to look at your whole v11
> series properly yet.  But I will reply here:
> 
> Ian Campbell writes ("Re: [OSSTEST Nested PATCH v11 6/7] Compose the
> main recipe of nested test job"):
> > Then on power operation the appropriate method each object is called in
> > turn (so you can call multiple modules if needed by the host).
> 
> Ian's explanation is right.
> 
> > > So how shall I assign $ho->{Power} method? by statically through config
> > > file?
> > > or some generic way automatically identify $host is some guest,
> therefore
> > > assign 'guest' to $ho->{Power}?
> > > Would you give me some idea? thanks.
> >
> > I'm not sure. Something in selecthost would need to know somehow that
> the
> > host being requested was actually an L1 guest.
> 
> I think this would be correct.
> 
> > Ian, any ideas?
> 
> I was expecting there to be a runvar which would for an L1 guest give
> the ident of its host.  ts-nested-setup would set this runvar and
> selecthost would look at it.
> 
> There are various possible exact syntaxes but I think the best one is
> probably that the runvar which ordinarily gives the name of a real
> host instead contains the containing host name and the L1 guest name.
> 
> So if you do
>   ./ts-nested-setup l0_host l1
> then you get a runvar `l1' with value `l0_host:l1'.
> 
> selecthost would see the colon, and set $ho->{Host} to `l0_host', so
> that get_target_property's recursion into containing hosts works
> properly.
> 
> selecthost would also be able to see that $ho->{Host} being set meant
> it was a nested guest, and that therefore power operations should be
> done by running toolstack operations on the host.  (Rather than
> looking up a host property.)
> 
> It also needs to arrange that for an L1 (well, anything with
> $ho->{Host} aka the _nestedhost runvar), the MAC address is read (into
> $ho) from the runvar which is created by the guest creation, rather
> than being looked up as a host property.  Then the functions for
> finding the IP address by asking the DHCP server have the right input.
> 
> The $ho->{DiskDevice} setting in selecthost is wrong too.
> 
> This is all probably most easily achieved by having the part of
> selecthost which deals with L1 guests seed $ho->{Properties} with
> appropriate information.
> 
> You want to make "----- calculation of the host's properties -----"
> conditional, and do something entirely different (ie, populate
> $ho->{Properties} and perhaps some of $ho->{Flags}) when $ho->{Host}
> is set.
> 
> Ian.

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Hu, Robert]

Thanks Ian.
So strange, seems this mail was sent ' Tuesday, September 1, 2015 10:42 PM ', but I 
have just received it.
I'm fully occupied by some release test, so have to carefully read your comments 1 ~2
weeks later. Sorry about this.

Best Regards,
Robert Ho

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: Tuesday, September 1, 2015 9:52 PM
> To: Hu, Robert; Ian Jackson
> Cc: xen-devel@lists.xen.org; wei.liu2@citrix.com
> Subject: Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of
> nested test job
> 
> On Tue, 2015-08-18 at 06:46 +0000, Hu, Robert wrote:
> 
> Sorry for the delay, I've been away.
> 
> > > -----Original Message-----
> > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > > Sent: Thursday, June 25, 2015 6:22 PM
> > > To: Pang, LongtaoX
> > > Cc: Ian Campbell; Hu, Robert; xen-devel@lists.xen.org;
> > > wei.liu2@citrix.com
> > > Subject: RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe
> of
> > > nested test job
> > >
> > > Pang, LongtaoX writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose
> the
> > > main recipe of nested test job"):
> > > > > -----Original Message-----
> > > > > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > > ...
> > > > > I think you are correct, the logs capture will fail too.
> > > > >
> > > > > I'll leave it to Ian to suggest a solution since it will no doubt
> > > > > involve some tcl plumbing (I'd be inclined to record 'hosts which
> > > > > are
> > > > > actually guests' somewhere and have the infra clean them up
> > > > > automatically after doing leak check and log collection).
> > >
> > > Sorry I haven't done this yet, it's still on my radar.
> > >
> > > > > I was thinking more along the lines of creating
> > > > > Osstest/PDU/guest.pm
> > > > > with the appropriate methods calling out to toolstack($l0)->foo,
> > > > > setting
> > > > > $ho->{Power} = 'guest $l1guestname' somewhere and allowing
> > > > > power_cycle_host_setup to do it's thing.
> > > >
> > > > I have reviewed power_cycle_host_setup function, inside this
> > > > function will call get_host_method_object, then we could get a $mo
> > > > which will be assigned to $ho->{PowerMethobjs}, right?  Inside
> > > > power_state function, it will call pdu_power_state which is defined
> > > > in guest.pm, right?
> > >
> > > Yes.
> > I'm not quite sure about how @$methobjs is obtained.
> > sub power_cycle_host_setup ($) {
> >     my ($ho) = @_;
> >     my $methobjs = [ ];
> >     foreach my $meth (split /\;\s*/, $ho->{Power}) {
> > 	push @$methobjs, get_host_method_object($ho,'PDU',$meth);
> >     }
> >     $ho->{PowerMethobjs} = $methobjs;
> > }
> > Seems it is derived from $ho->{Power} while $ho->{Power} is defined by
> > either:
> > 1. selecthost()
> > 	$ho->{Power}= get_host_property($ho,'power-method');
> > 2. default_methods()
> > 	$ho->{Power} ||= "statedb $ho->{Asset}";
> >   Seems no statedb for standalone mode.
> 
> Indeed, statedb is actually peculiar to the old Cambridge instance of
> osstest, it's not used in the production colo. In fact it is also not used
> in Cambridge any more, every host has a power-method host property. In
> standalone mode the default {Power} is "manual" I think, that's the one
> which prompts you to press a key.
> 
> The {Power} property is essentially a ";" separated list, each list entry
> is "<module> <arg1> <arg2> <...>". <module> is Osstest/PDU/<module>.pm
> and
> <arg1> <arg2> <...> are passed to the constructor. These objects are
> constructed by get_host_method_object.
> 
> Then on power operation the appropriate method each object is called in
> turn (so you can call multiple modules if needed by the host).
> 
> > So how shall I assign $ho->{Power} method? by statically through config
> > file?
> > or some generic way automatically identify $host is some guest, therefore
> > assign 'guest' to $ho->{Power}?
> > Would you give me some idea? thanks.
> 
> I'm not sure. Something in selecthost would need to know somehow that the
> host being requested was actually an L1 guest.
> 
> Ian, any ideas?
> 
> Ian

Re: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job

[Ian Jackson]

Hu, Robert writes ("RE: [OSSTEST Nested PATCH v11 6/7] Compose the main recipe of nested test job"):
> So strange, seems this mail was sent ' Tuesday, September 1, 2015
> 10:42 PM ', but I have just received it.

How annoying.

>  I'm fully occupied by some
> release test, so have to carefully read your comments 1 ~2 weeks
> later. Sorry about this.

That's quite OK.  I have been slow too.  I want to reassure you again
that I want this feature in osstest.  I think we are making reasonable
progress.

Ian.