Re: [osstest test] 65001: regressions - trouble: broken/fail/pass

Re: [osstest test] 65001: regressions - trouble: broken/fail/pass

[Ian Campbell]

On Mon, 2015-11-23 at 10:03 +0000, osstest service owner wrote:
> flight 65001 osstest real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/65001/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-amd64-qemuu-nested 15 leak-check/basis/l1(15)  fail REGR. vs. 64659

http://logs.test-lab.xenproject.org/osstest/logs/65001/test-amd64-amd64-qemuu-nested/15.ts-leak-check.log

    xl: error while loading shared libraries: libnl-route-3.so.200: cannot open shared object file: No such file or directory

This is in the L1 guest, which was installed by ts-xen-install as if it
were a host. That script contains:

    if ($ho->{Suite} =~ m/jessie/) {
        target_install_packages($ho, 'libnl-route-3-200');
    }

There is no suite runvar on either the L0 or the L1, however the L0 run of
ts-xen-install did seem to get this correct and install libxl-route-3-200.

This is confusing because $ho-{Suite} comes from selecthost:

    if (defined $job) {
        $ho->{Suite} = get_runvar_default("${ident}_suite",$job,
                                          $c{DebianSuite});
    }

which is before the short-circuit for nested hosts.

I've no idea why $job would be undefined, in
http://logs.test-lab.xenproject.org/osstest/logs/65001/test-amd64-amd64-qemuu-nested/15.ts-leak-check.log 
I can see OSSTEST_JOB is set and exported and tsreadconfig() hasn't
complained. 

Any ideas?

Ian.

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

[PATCH OSSTEST] selecthost: Correctly set ->{Suite} for a nested host in selecthost()

[Ian Campbell]

Since $child comes initially from selectguest() (and is then tailored
to look more like a host) it never has a ->{Suite} set.

Since $c{DebianSuite} and $c{DebianGuestSuite} differ and it is
unclear that we would want to apply $c{DebianGuestSuite} as a default
in this context fix this up in selecthost() (using the parent's suite
as the default) rather than causing selectguest() to always set
->{Suite}.

Tested in some adhoc invocations of the relevant test scripts.

This was exposed by the switch to DebianSuite==Jessie. ts-xen-install
for a nested L1 failed to conditionally install the Jessie libarary
libnl-route-3-200.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 Osstest/TestSupport.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index d1f7d36..0b61fda 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -876,6 +876,11 @@ sub selecthost ($) {
 	$child->{Info} = [ "in", $parent->{Name}, @{ $parent->{Info} } ];
 	$child->{NestingLevel} = $parent->{NestingLevel}+1;
 
+	if (defined $job) {
+	    $child->{Suite} = get_runvar_default("${name}_suite",$job,
+						 $parent->{Suite});
+	}
+
 	$child->{Power} = 'guest';
 	power_cycle_host_setup($child);
 
-- 
2.6.1

[PATCH OSSTEST v2] Add $gho->{Suite} field to guest objects from {prepare, select}guest()

[Ian Campbell]

Currently those places which want this open code a lookup of the
{ident}_suite runvar with a fallback to the configuration file.

However selecthost was missing such a lookup in the case where it is
constructing a nested L1 host (which begins from the selectguest
template), which lead to ts-xen-install on Jessie missing the
installation of libnl-route-3-200.

Fix this by having prepareguest() store a {ident}_suite runvar (taking
care to handle the case where one is already set by e.g. make-flight)
and have selectguest() initialise $gho->Suite from it.

ts-debian-install, ts-debian-di-install and ts-debian-hvm-install now
simply use $gho->{Suite} instead of open coding the lookup.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Totally different approach, replacing "selecthost: Correctly set
    ->{Suite} for a nested host in selecthost()". Previous version
    had ts-debian-hvm-install using DebianGuestSuite and
    ts-xen-install using DebianSuite.
---
 Osstest/TestSupport.pm | 4 ++++
 ts-debian-di-install   | 2 +-
 ts-debian-hvm-install  | 2 +-
 ts-debian-install      | 2 +-
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index d1f7d36..51f9210 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1453,6 +1453,7 @@ sub selectguest ($$) {
         CfgPath => $r{"${gn}_cfgpath"},
         Tftp => $ho->{Tftp},
 	Host => $ho,
+	Suite => $r{"${gn}_suite"},
     };
     foreach my $opt (guest_var_commalist($gho,'options')) {
         $gho->{Options}{$opt}++;
@@ -1695,6 +1696,9 @@ sub prepareguest ($$$$$$) {
     store_runvar("${gn}_tcpcheckport", $tcpcheckport);
     store_runvar("${gn}_boot_timeout", $boot_timeout);
 
+    store_runvar("${gn}_suite", $c{GuestDebianSuite})
+	unless $r{"${gn}_suite"};
+
     my $gho= selectguest($gn, $ho);
     store_runvar("${gn}_domname", $gho->{Name});
 
diff --git a/ts-debian-di-install b/ts-debian-di-install
index 64b5d44..d566a0d 100755
--- a/ts-debian-di-install
+++ b/ts-debian-di-install
@@ -190,7 +190,7 @@ END
 
     if ( $method eq "netboot" )
     {
-	my $suite= $r{"$gho->{Guest}_suite"};
+	my $suite= $gho->{Suite}};
 	logm("$method $suite/$arch");
 
 	$method_cfg = setup_netboot($tmpdir, $arch, $suite);
diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 2c7580c..71cb23f 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -199,7 +199,7 @@ sub prep () {
                        $disk_mb + 1,
                        200);
 
-    $gsuite = guest_var($gho,'suite',$c{GuestDebianSuite});
+    $gsuite = $gho->{Suite};
     $kernel = iso_path('kernel', 'vmlinuz');
     $ramdisk = iso_path('ramdisk', 'initrd.gz');
 
diff --git a/ts-debian-install b/ts-debian-install
index 0dfe40c..4db9a3b 100755
--- a/ts-debian-install
+++ b/ts-debian-install
@@ -47,7 +47,7 @@ sub prep () {
 sub ginstall () {
     my $arch= $r{"$gho->{Guest}_arch"};
     my $archarg= defined($arch) ? "--arch $arch" : '';
-    my $gsuite= guest_var($gho,'suite',$c{GuestDebianSuite});
+    my $gsuite= $gho->{Suite};
 
     my $kernpath = guest_var($gho,'kernel_path',$r{xen_kernel_path});
     my $initrd = guest_var($gho,'initrd_path',$r{xen_initrd_path});
-- 
2.6.1

Re: [PATCH OSSTEST v2] Add $gho->{Suite} field to guest objects from {prepare, select}guest()

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2] Add $gho->{Suite} field to guest objects from {prepare,select}guest()"):
> Currently those places which want this open code a lookup of the
> {ident}_suite runvar with a fallback to the configuration file.
> 
> However selecthost was missing such a lookup in the case where it is
> constructing a nested L1 host (which begins from the selectguest
> template), which lead to ts-xen-install on Jessie missing the
> installation of libnl-route-3-200.
> 
> Fix this by having prepareguest() store a {ident}_suite runvar (taking
> care to handle the case where one is already set by e.g. make-flight)
> and have selectguest() initialise $gho->Suite from it.

Having thought about this, ISTM that this will cause strangenesses
such as runvars called `redhat_suite' with value `jessie' (or
whatever).

Perhaps ts-debian*install could call some new function
debian_guest_suite in Debian.pm ?

Ian.

Re: [PATCH OSSTEST v2] Add $gho->{Suite} field to guest objects from {prepare, select}guest()

[Ian Campbell]

On Mon, 2015-11-23 at 16:12 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v2] Add $gho->{Suite} field to guest
> objects from {prepare,select}guest()"):
> > Currently those places which want this open code a lookup of the
> > {ident}_suite runvar with a fallback to the configuration file.
> > 
> > However selecthost was missing such a lookup in the case where it is
> > constructing a nested L1 host (which begins from the selectguest
> > template), which lead to ts-xen-install on Jessie missing the
> > installation of libnl-route-3-200.
> > 
> > Fix this by having prepareguest() store a {ident}_suite runvar (taking
> > care to handle the case where one is already set by e.g. make-flight)
> > and have selectguest() initialise $gho->Suite from it.
> 
> Having thought about this, ISTM that this will cause strangenesses
> such as runvars called `redhat_suite' with value `jessie' (or
> whatever).
> 
> Perhaps ts-debian*install could call some new function
> debian_guest_suite in Debian.pm ?

Sure. Should it return the suite (to be put into $gho->{Suite} by the
caller) or initialise $gho->{Suite} itself? I lean towards the latter.

Ian.

Re: [PATCH OSSTEST v2] Add $gho->{Suite} field to guest objects from {prepare, select}guest()

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST v2] Add $gho->{Suite} field to guest objects from {prepare,select}guest()"):
> On Mon, 2015-11-23 at 16:12 +0000, Ian Jackson wrote:
> > Perhaps ts-debian*install could call some new function
> > debian_guest_suite in Debian.pm ?
> 
> Sure. Should it return the suite (to be put into $gho->{Suite} by the
> caller) or initialise $gho->{Suite} itself? I lean towards the latter.

It could do both so you can write
  $gsuite = debian_guest_set_suite($gho)
or whatever ?

Ian.

Re: [PATCH OSSTEST v2] Add $gho->{Suite} field to guest objects from {prepare, select}guest()

[Ian Campbell]

On Mon, 2015-11-23 at 16:48 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH OSSTEST v2] Add $gho->{Suite} field to
> guest objects from {prepare,select}guest()"):
> > On Mon, 2015-11-23 at 16:12 +0000, Ian Jackson wrote:
> > > Perhaps ts-debian*install could call some new function
> > > debian_guest_suite in Debian.pm ?
> > 
> > Sure. Should it return the suite (to be put into $gho->{Suite} by the
> > caller) or initialise $gho->{Suite} itself? I lean towards the latter.
> 
> It could do both so you can write
>   $gsuite = debian_guest_set_suite($gho)
> or whatever ?

Good plan.

Ian.

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

[PATCH v3] Set {ident}_suite runvar when install a Debian guest.

[Ian Campbell]

Currently those places which want this open code a lookup of the
{ident}_suite runvar with a fallback to the configuration file.

However selecthost was missing such a lookup in the case where it is
constructing a nested L1 host (which begins from the selectguest
template), which lead to ts-xen-install on Jessie missing the
installation of libnl-route-3-200.

Fix this by providing debian_guest_suite($gho) which as well as
initialising $gho->{Suite} stores an {ident}_suite runvar (taking care
to handle the case where one is already set by e.g. make-flight). For
convenience debian_guest_suite() also returns the suite name.

ts-debian-install, ts-debian-di-install and ts-debian-hvm-install now
use debian_guest_suite instead of open coding the lookup.

The final piece of the puzzle is to have selectguest() pickup the
{ident}_suite runvar (if it is set) and initialise $gho->{Suite} from
it.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
I've kicked off an adhoc run testing:
+    test-amd64-amd64-qemuu-nested);; # nested
+    test-amd64-amd64-xl-qemuu-debianhvm-amd64);; # regular HVM
+    test-amd64-amd64-xl-qcow2);; # DI install
+    test-amd64-amd64-xl);; # Normal PV
with this change.

v2: Totally different approach, replacing "selecthost: Correctly set
    ->{Suite} for a nested host in selecthost()". Previous version
    had ts-debian-hvm-install using DebianGuestSuite and
    ts-xen-install using DebianSuite.
v3: Another different approach, replacing "Add $gho->{Suite} field to
    guest objects from {prepare,select}guest()".
---
 Osstest/Debian.pm      | 15 +++++++++++++++
 Osstest/TestSupport.pm |  1 +
 ts-debian-di-install   |  2 +-
 ts-debian-hvm-install  |  2 +-
 ts-debian-install      |  2 +-
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 317f663..c9b8e17 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -35,6 +35,7 @@ BEGIN {
     @ISA         = qw(Exporter);
     @EXPORT      = qw(debian_boot_setup
                       debian_overlays
+                      debian_guest_suite
                       %preseed_cmds
                       preseed_base
                       preseed_create
@@ -1335,4 +1336,18 @@ sub preseed_hook_cmds () {
     return $preseed;
 }
 
+sub debian_guest_suite ($) {
+    my ($gho) = @_;
+
+    return $gho->{Suite} if $gho->{Suite};
+
+    $gho->{Suite} = guest_var($gho,'suite',undef);
+
+    if ( ! $gho->{Suite} ) {
+	$gho->{Suite} = $c{GuestDebianSuite};
+	store_runvar("$gho->{Guest}_suite", $gho->{Suite});
+    }
+    return $gho->{Suite};
+}
+
 1;
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index d1f7d36..61e885a 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1453,6 +1453,7 @@ sub selectguest ($$) {
         CfgPath => $r{"${gn}_cfgpath"},
         Tftp => $ho->{Tftp},
 	Host => $ho,
+	Suite => $r{"${gn}_suite"},
     };
     foreach my $opt (guest_var_commalist($gho,'options')) {
         $gho->{Options}{$opt}++;
diff --git a/ts-debian-di-install b/ts-debian-di-install
index 64b5d44..a9b3b20 100755
--- a/ts-debian-di-install
+++ b/ts-debian-di-install
@@ -190,7 +190,7 @@ END
 
     if ( $method eq "netboot" )
     {
-	my $suite= $r{"$gho->{Guest}_suite"};
+	my $suite= debian_guest_suite($gho);
 	logm("$method $suite/$arch");
 
 	$method_cfg = setup_netboot($tmpdir, $arch, $suite);
diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 2c7580c..96190a1 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -199,7 +199,7 @@ sub prep () {
                        $disk_mb + 1,
                        200);
 
-    $gsuite = guest_var($gho,'suite',$c{GuestDebianSuite});
+    $gsuite = debian_guest_suite($gho);
     $kernel = iso_path('kernel', 'vmlinuz');
     $ramdisk = iso_path('ramdisk', 'initrd.gz');
 
diff --git a/ts-debian-install b/ts-debian-install
index 0dfe40c..c8002d9 100755
--- a/ts-debian-install
+++ b/ts-debian-install
@@ -47,7 +47,7 @@ sub prep () {
 sub ginstall () {
     my $arch= $r{"$gho->{Guest}_arch"};
     my $archarg= defined($arch) ? "--arch $arch" : '';
-    my $gsuite= guest_var($gho,'suite',$c{GuestDebianSuite});
+    my $gsuite= debian_guest_suite($gho);
 
     my $kernpath = guest_var($gho,'kernel_path',$r{xen_kernel_path});
     my $initrd = guest_var($gho,'initrd_path',$r{xen_initrd_path});
-- 
2.6.1

Re: [PATCH v3] Set {ident}_suite runvar when install a Debian guest.

[Ian Jackson]

Ian Campbell writes ("[PATCH v3] Set {ident}_suite runvar when install a Debian guest."):
> Currently those places which want this open code a lookup of the
> {ident}_suite runvar with a fallback to the configuration file.
> 
> However selecthost was missing such a lookup in the case where it is
> constructing a nested L1 host (which begins from the selectguest
> template), which lead to ts-xen-install on Jessie missing the
> installation of libnl-route-3-200.
> 
> Fix this by providing debian_guest_suite($gho) which as well as
> initialising $gho->{Suite} stores an {ident}_suite runvar (taking care
> to handle the case where one is already set by e.g. make-flight). For
> convenience debian_guest_suite() also returns the suite name.
> 
> ts-debian-install, ts-debian-di-install and ts-debian-hvm-install now
> use debian_guest_suite instead of open coding the lookup.
> 
> The final piece of the puzzle is to have selectguest() pickup the
> {ident}_suite runvar (if it is set) and initialise $gho->{Suite} from
> it.

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

> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> ---
> I've kicked off an adhoc run testing:
> +    test-amd64-amd64-qemuu-nested);; # nested
> +    test-amd64-amd64-xl-qemuu-debianhvm-amd64);; # regular HVM
> +    test-amd64-amd64-xl-qcow2);; # DI install
> +    test-amd64-amd64-xl);; # Normal PV
> with this change.

This looks like you wrote a case statement in sh.  Did you know about
OSSTEST_JOBS_ONLY (see cs-job-create) ?


FFR, I have some style tips:

> +sub debian_guest_suite ($) {
> +    my ($gho) = @_;
> +
> +    return $gho->{Suite} if $gho->{Suite};
> +
> +    $gho->{Suite} = guest_var($gho,'suite',undef);

If you do this instead

  +    $gho->{Suite} //= guest_var($gho,'suite',undef);

the previous line (`return ... if') is not needed.

> +    if ( ! $gho->{Suite} ) {

This is stylistically odd IMO.  Seems to have been written by a
hypervisor maintainer :-).

> +	$gho->{Suite} = $c{GuestDebianSuite};
> +	store_runvar("$gho->{Guest}_suite", $gho->{Suite});
> +    }
> +    return $gho->{Suite};
> +}

But none of that really matters.

Ian.

Re: [PATCH v3] Set {ident}_suite runvar when install a Debian guest.

[Ian Campbell]

On Tue, 2015-11-24 at 11:17 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH v3] Set {ident}_suite runvar when install a
> Debian guest."):
> > Currently those places which want this open code a lookup of the
> > {ident}_suite runvar with a fallback to the configuration file.
> > 
> > However selecthost was missing such a lookup in the case where it is
> > constructing a nested L1 host (which begins from the selectguest
> > template), which lead to ts-xen-install on Jessie missing the
> > installation of libnl-route-3-200.
> > 
> > Fix this by providing debian_guest_suite($gho) which as well as
> > initialising $gho->{Suite} stores an {ident}_suite runvar (taking care
> > to handle the case where one is already set by e.g. make-flight). For
> > convenience debian_guest_suite() also returns the suite name.
> > 
> > ts-debian-install, ts-debian-di-install and ts-debian-hvm-install now
> > use debian_guest_suite instead of open coding the lookup.
> > 
> > The final piece of the puzzle is to have selectguest() pickup the
> > {ident}_suite runvar (if it is set) and initialise $gho->{Suite} from
> > it.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > I've kicked off an adhoc run testing:
> > +    test-amd64-amd64-qemuu-nested);; # nested
> > +    test-amd64-amd64-xl-qemuu-debianhvm-amd64);; # regular HVM
> > +    test-amd64-amd64-xl-qcow2);; # DI install
> > +    test-amd64-amd64-xl);; # Normal PV

FYI this last one failed, I'll investigate and no doubt there will be a v4.
I'll fix the style stuff as I go.

> > with this change.
> 
> This looks like you wrote a case statement in sh.  Did you know about
> OSSTEST_JOBS_ONLY (see cs-job-create) ?

Indeed, I hacked it into make-flights filter hooks. I'll investigate
OSSTEST_JOBS_ONLY for next time.

> FFR, I have some style tips:
> 
> > +sub debian_guest_suite ($) {
> > +    my ($gho) = @_;
> > +
> > +    return $gho->{Suite} if $gho->{Suite};
> > +
> > +    $gho->{Suite} = guest_var($gho,'suite',undef);
> 
> If you do this instead
> 
>   +    $gho->{Suite} //= guest_var($gho,'suite',undef);
> 
> the previous line (`return ... if') is not needed.

Done. (I suppose this will elide the call to guest_var if ->{Suite} is
already defined, like an || would).

> > +    if ( ! $gho->{Suite} ) {
> 
> This is stylistically odd IMO.  Seems to have been written by a
> hypervisor maintainer :-).

I think you are referring to the too many spaces and would normally do
	if (!$gho->{Suite}) {
or was there something else?

Ian.


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

Re: [PATCH v3] Set {ident}_suite runvar when install a Debian guest.

[Ian Campbell]

On Tue, 2015-11-24 at 12:41 +0000, Ian Campbell wrote:
> On Tue, 2015-11-24 at 11:17 +0000, Ian Jackson wrote:
> > Ian Campbell writes ("[PATCH v3] Set {ident}_suite runvar when install
> > a
> > Debian guest."):
> > > Currently those places which want this open code a lookup of the
> > > {ident}_suite runvar with a fallback to the configuration file.
> > > 
> > > However selecthost was missing such a lookup in the case where it is
> > > constructing a nested L1 host (which begins from the selectguest
> > > template), which lead to ts-xen-install on Jessie missing the
> > > installation of libnl-route-3-200.
> > > 
> > > Fix this by providing debian_guest_suite($gho) which as well as
> > > initialising $gho->{Suite} stores an {ident}_suite runvar (taking
> > > care
> > > to handle the case where one is already set by e.g. make-flight). For
> > > convenience debian_guest_suite() also returns the suite name.
> > > 
> > > ts-debian-install, ts-debian-di-install and ts-debian-hvm-install now
> > > use debian_guest_suite instead of open coding the lookup.
> > > 
> > > The final piece of the puzzle is to have selectguest() pickup the
> > > {ident}_suite runvar (if it is set) and initialise $gho->{Suite} from
> > > it.
> > 
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> > 
> > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > > ---
> > > I've kicked off an adhoc run testing:
> > > +    test-amd64-amd64-qemuu-nested);; # nested
> > > +    test-amd64-amd64-xl-qemuu-debianhvm-amd64);; # regular HVM
> > > +    test-amd64-amd64-xl-qcow2);; # DI install
> > > +    test-amd64-amd64-xl);; # Normal PV
> 
> FYI this last one failed, I'll investigate and no doubt there will be a v4.
> I'll fix the style stuff as I go.

This turned out to be a missing "use Osstest::Debian" in ts-debian-install, 
so I shall retain the ack. I'll run another test before sending v4.

Ian.

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

Re: [PATCH v3] Set {ident}_suite runvar when install a Debian guest.

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH v3] Set {ident}_suite runvar when install a Debian guest."):
> On Tue, 2015-11-24 at 11:17 +0000, Ian Jackson wrote:
> > If you do this instead
> >   +    $gho->{Suite} //= guest_var($gho,'suite',undef);
> > the previous line (`return ... if') is not needed.
> 
> Done. (I suppose this will elide the call to guest_var if ->{Suite} is
> already defined, like an || would).

Indeed.  // is like || but looks at definedness rather than
trueishness.

> > > +    if ( ! $gho->{Suite} ) {
> > 
> > This is stylistically odd IMO.  Seems to have been written by a
> > hypervisor maintainer :-).
> 
> I think you are referring to the too many spaces and would normally do
> 	if (!$gho->{Suite}) {
> or was there something else?

Yes, just that.

(Also saw your other mail, thanks.)

Thanks,
Ian.

[PATCH OSSTEST v4] Set {ident}_suite runvar when install a Debian guest.

[Ian Campbell]

Currently those places which want this open code a lookup of the
{ident}_suite runvar with a fallback to the configuration file.

However selecthost was missing such a lookup in the case where it is
constructing a nested L1 host (which begins from the selectguest
template), which lead to ts-xen-install on Jessie missing the
installation of libnl-route-3-200.

Fix this by providing debian_guest_suite($gho) which as well as
initialising $gho->{Suite} stores an {ident}_suite runvar (taking care
to handle the case where one is already set by e.g. make-flight). For
convenience debian_guest_suite() also returns the suite name.

ts-debian-install, ts-debian-di-install and ts-debian-hvm-install now
use debian_guest_suite instead of open coding the lookup.

The final piece of the puzzle is to have selectguest() pickup the
{ident}_suite runvar (if it is set) and initialise $gho->{Suite} from
it.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
I've run an adhoc test with:
OSSTEST_JOBS_ONLY=
OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64
OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64-pvops
OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-qemuu-nested # nested
OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qemuu-debianhvm-amd64 # regular HVM
OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qcow2 # DI install
OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl # Normal PV
export OSSTEST_JOBS_ONLY
( ;-) )

The three HVM ones have passed and the PV one is well past the point where any
of this would make a difference (it's at 17.ts-guest-localmigrate)

v2: Totally different approach, replacing "selecthost: Correctly set
    ->{Suite} for a nested host in selecthost()". Previous version
    had ts-debian-hvm-install using DebianGuestSuite and
    ts-xen-install using DebianSuite.
v3: Another different approach, replacing "Add $gho->{Suite} field to
    guest objects from {prepare,select}guest()".
v4: Use //= guest_var, and remove spaces inside if expr.
    "use Osstest::Debian" in ts-debian-install (how it avoided that
    requirement until now I'm not sure!)
---
 Osstest/Debian.pm      | 14 ++++++++++++++
 Osstest/TestSupport.pm |  1 +
 ts-debian-di-install   |  2 +-
 ts-debian-hvm-install  |  2 +-
 ts-debian-install      |  3 ++-
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 317f663..cee0d88 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -35,6 +35,7 @@ BEGIN {
     @ISA         = qw(Exporter);
     @EXPORT      = qw(debian_boot_setup
                       debian_overlays
+                      debian_guest_suite
                       %preseed_cmds
                       preseed_base
                       preseed_create
@@ -1335,4 +1336,17 @@ sub preseed_hook_cmds () {
     return $preseed;
 }
 
+sub debian_guest_suite ($) {
+    my ($gho) = @_;
+
+    $gho->{Suite} //= guest_var($gho,'suite',undef);
+
+    if (!$gho->{Suite}) {
+	$gho->{Suite} = $c{GuestDebianSuite};
+	store_runvar("$gho->{Guest}_suite", $gho->{Suite});
+    }
+
+    return $gho->{Suite};
+}
+
 1;
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index d1f7d36..61e885a 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -1453,6 +1453,7 @@ sub selectguest ($$) {
         CfgPath => $r{"${gn}_cfgpath"},
         Tftp => $ho->{Tftp},
 	Host => $ho,
+	Suite => $r{"${gn}_suite"},
     };
     foreach my $opt (guest_var_commalist($gho,'options')) {
         $gho->{Options}{$opt}++;
diff --git a/ts-debian-di-install b/ts-debian-di-install
index 64b5d44..a9b3b20 100755
--- a/ts-debian-di-install
+++ b/ts-debian-di-install
@@ -190,7 +190,7 @@ END
 
     if ( $method eq "netboot" )
     {
-	my $suite= $r{"$gho->{Guest}_suite"};
+	my $suite= debian_guest_suite($gho);
 	logm("$method $suite/$arch");
 
 	$method_cfg = setup_netboot($tmpdir, $arch, $suite);
diff --git a/ts-debian-hvm-install b/ts-debian-hvm-install
index 2c7580c..96190a1 100755
--- a/ts-debian-hvm-install
+++ b/ts-debian-hvm-install
@@ -199,7 +199,7 @@ sub prep () {
                        $disk_mb + 1,
                        200);
 
-    $gsuite = guest_var($gho,'suite',$c{GuestDebianSuite});
+    $gsuite = debian_guest_suite($gho);
     $kernel = iso_path('kernel', 'vmlinuz');
     $ramdisk = iso_path('ramdisk', 'initrd.gz');
 
diff --git a/ts-debian-install b/ts-debian-install
index 0dfe40c..f42edbf 100755
--- a/ts-debian-install
+++ b/ts-debian-install
@@ -19,6 +19,7 @@ use strict qw(vars);
 use DBI;
 use Osstest;
 use Osstest::TestSupport;
+use Osstest::Debian;
 
 tsreadconfig();
 
@@ -47,7 +48,7 @@ sub prep () {
 sub ginstall () {
     my $arch= $r{"$gho->{Guest}_arch"};
     my $archarg= defined($arch) ? "--arch $arch" : '';
-    my $gsuite= guest_var($gho,'suite',$c{GuestDebianSuite});
+    my $gsuite= debian_guest_suite($gho);
 
     my $kernpath = guest_var($gho,'kernel_path',$r{xen_kernel_path});
     my $initrd = guest_var($gho,'initrd_path',$r{xen_initrd_path});
-- 
2.6.1

Re: [PATCH OSSTEST v4] Set {ident}_suite runvar when install a Debian guest.

[Ian Campbell]

On Tue, 2015-11-24 at 15:14 +0000, Ian Campbell wrote:
> ---
> I've run an adhoc test with:
> OSSTEST_JOBS_ONLY=
> OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64
> OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64-pvops
> OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-qemuu-nested # nested
> OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qemuu-debianhvm-amd64 # regular HVM
> OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qcow2 # DI install
> OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl # Normal PV
> export OSSTEST_JOBS_ONLY
> ( ;-) )
> 
> The three HVM ones have passed and the PV one is well past the point where any
> of this would make a difference (it's at 17.ts-guest-localmigrate)

I did notice one oddity, which I think is unrelated to this series which is
that test-amd64-amd64-qemuu-nested/18.ts-leak-check.log contains:

    2015-11-24 14:49:41 Z [host l1] L1 host l1: guest l1 (in scape-moth) 5a:36:0e:bd:00:03 10.80.238.243 
    2015-11-24 14:49:41 Z executing ssh ...     root@10.80.238.243     xl list 
    2015-11-24 14:49:41 Z executing ssh ...     root@10.80.238.243     ps -wwef 
    2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     ps -wwef 
    2015-11-24 14:49:42 Z LEAKED [process 2000 /usr/sbin/ntpd] process: ntp       2000     1  0 14:30 ?        00:00:00 /usr/sbin/ntpd -p /var/run/ntpd.pid -g -c /var/lib/ntp/ntp.conf.dhcp -u 107:114 
    2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     xenstore-ls -fp 
    2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     losetup -a 
    2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     find /tmp /var/run /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls -printf '\0' 
    2015-11-24 14:49:42 Z FAILURE: 1 leaked object(s) 

While e.g. http://logs.test-lab.xenproject.org/osstest/logs/65050/test-amd6
4-amd64-qemuu-nested-intel/info.html did not suffer from this.

In leak-basis-l1.guest.osstest in my flight I see:
    root      1531     1  0 14:30 ?        00:00:00 /bin/sh /etc/init.d/ntp start
compared with 65050 where I see 
    ntp       4154     1  0 07:47 ?        00:00:00 /usr/sbin/ntpd -p /var/run/ntpd.pid -g -u 103:107

So this seems like a race of some sort.

L1 serial console ends:
         Starting LSB: exim Mail Transport Agent...^M
         Starting LSB: Apache2 web server...^M
         Starting LSB: Confirm fully booted...^M
         Starting LSB: Start NTP daemon...^M
         Starting LSB: Start/stop xenstored and xenconsoled...^M
         Starting D-Bus System Message Bus...^M
[  OK  ] Started D-Bus System Message Bus.^M
         Starting System Logging Service...^M
         Starting Permit User Sessions...^M
         Starting ACPI event daemon...^M
[  OK  ] Started ACPI event daemon.^M
[  OK  ] Started /etc/rc.local Compatibility.^M
[  OK  ] Started LSB: Confirm fully booted.^M
[  OK  ] Started Permit User Sessions.^M
         Starting Getty on tty1...^M
[  OK  ] Started Getty on tty1.^M
         Starting Serial Getty on hvc0...^M
[  OK  ] Started Serial Getty on hvc0.^M
[  OK  ] Reached target Login Prompts.^M
[  OK  ] Started Login Service.^M
[  OK  ] Started System Logging Service.^M
[  OK  ] Started LSB: exim Mail Transport Agent.^M
[  OK  ] Started LSB: Apache2 web server.^M
[  OK  ] Started LSB: Start/stop xenstored and xenconsoled.^M

(I had to strip a bunch of colour/escaping, applying the FANCYTTY=0 to
these guests would be useful, so would timestamps).

Ian.

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

Re: [PATCH OSSTEST v4] Set {ident}_suite runvar when install a Debian guest.

[Ian Campbell]

On Tue, 2015-11-24 at 15:26 +0000, Ian Campbell wrote:
> On Tue, 2015-11-24 at 15:14 +0000, Ian Campbell wrote:
> > ---
> > I've run an adhoc test with:
> > OSSTEST_JOBS_ONLY=
> > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64
> > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64-pvops
> > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-qemuu-nested #
> > nested
> > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qemuu-
> > debianhvm-amd64 # regular HVM
> > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qcow2 # DI
> > install
> > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl # Normal PV
> > export OSSTEST_JOBS_ONLY
> > ( ;-) )
> > 
> > The three HVM ones have passed and the PV one is well past the point
> > where any
> > of this would make a difference (it's at 17.ts-guest-localmigrate)
> 
> I did notice one oddity, which I think is unrelated to this series which is
> that test-amd64-amd64-qemuu-nested/18.ts-leak-check.log contains:

The flight finished and the logs are being sync'd to http://xenbits.xen.org
/people/ianc/tmp/38333/ now.

> 
>     2015-11-24 14:49:41 Z [host l1] L1 host l1: guest l1 (in scape-moth)
> 5a:36:0e:bd:00:03 10.80.238.243 
>     2015-11-24 14:49:41 Z executing ssh ...     root@10.80.238.243     xl
> list 
>     2015-11-24 14:49:41 Z executing ssh ...     root@10.80.238.243     ps
> -wwef 
>     2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     ps
> -wwef 
>     2015-11-24 14:49:42 Z LEAKED [process 2000 /usr/sbin/ntpd] process:
> ntp       2000     1  0 14:30 ?        00:00:00 /usr/sbin/ntpd -p
> /var/run/ntpd.pid -g -c /var/lib/ntp/ntp.conf.dhcp -u 107:114 
>     2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     xe
> nstore-ls -fp 
>     2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     lo
> setup -a 
>     2015-11-24 14:49:42 Z executing ssh ...     root@10.80.238.243     fi
> nd /tmp /var/run /var/tmp /var/lib/xen /var/core ! -type d -print0 -ls
> -printf '\0' 
>     2015-11-24 14:49:42 Z FAILURE: 1 leaked object(s) 
> 
> While e.g. http://logs.test-lab.xenproject.org/osstest/logs/65050/test-am
> d6
> 4-amd64-qemuu-nested-intel/info.html did not suffer from this.
> 
> In leak-basis-l1.guest.osstest in my flight I see:
>     root      1531     1  0 14:30 ?        00:00:00 /bin/sh
> /etc/init.d/ntp start
> compared with 65050 where I see 
>     ntp       4154     1  0 07:47 ?        00:00:00 /usr/sbin/ntpd -p
> /var/run/ntpd.pid -g -u 103:107
> 
> So this seems like a race of some sort.
> 
> L1 serial console ends:
>          Starting LSB: exim Mail Transport Agent...^M
>          Starting LSB: Apache2 web server...^M
>          Starting LSB: Confirm fully booted...^M
>          Starting LSB: Start NTP daemon...^M
>          Starting LSB: Start/stop xenstored and xenconsoled...^M
>          Starting D-Bus System Message Bus...^M
> [  OK  ] Started D-Bus System Message Bus.^M
>          Starting System Logging Service...^M
>          Starting Permit User Sessions...^M
>          Starting ACPI event daemon...^M
> [  OK  ] Started ACPI event daemon.^M
> [  OK  ] Started /etc/rc.local Compatibility.^M
> [  OK  ] Started LSB: Confirm fully booted.^M
> [  OK  ] Started Permit User Sessions.^M
>          Starting Getty on tty1...^M
> [  OK  ] Started Getty on tty1.^M
>          Starting Serial Getty on hvc0...^M
> [  OK  ] Started Serial Getty on hvc0.^M
> [  OK  ] Reached target Login Prompts.^M
> [  OK  ] Started Login Service.^M
> [  OK  ] Started System Logging Service.^M
> [  OK  ] Started LSB: exim Mail Transport Agent.^M
> [  OK  ] Started LSB: Apache2 web server.^M
> [  OK  ] Started LSB: Start/stop xenstored and xenconsoled.^M
> 
> (I had to strip a bunch of colour/escaping, applying the FANCYTTY=0 to
> these guests would be useful, so would timestamps).
> 
> Ian.

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

Re: [PATCH OSSTEST v4] Set {ident}_suite runvar when install a Debian guest.

[Ian Campbell]

On Tue, 2015-11-24 at 15:28 +0000, Ian Campbell wrote:
> On Tue, 2015-11-24 at 15:26 +0000, Ian Campbell wrote:
> > On Tue, 2015-11-24 at 15:14 +0000, Ian Campbell wrote:
> > > ---
> > > I've run an adhoc test with:
> > > OSSTEST_JOBS_ONLY=
> > > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64
> > > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:build-amd64-pvops
> > > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-qemuu-nested #
> > > nested
> > > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qemuu-
> > > debianhvm-amd64 # regular HVM
> > > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl-qcow2 # DI
> > > install
> > > OSSTEST_JOBS_ONLY=$OSSTEST_JOBS_ONLY:test-amd64-amd64-xl # Normal PV
> > > export OSSTEST_JOBS_ONLY
> > > ( ;-) )
> > > 
> > > The three HVM ones have passed and the PV one is well past the point
> > > where any
> > > of this would make a difference (it's at 17.ts-guest-localmigrate)
> > 
> > I did notice one oddity,

So, I was running with DebianSuite==jessie and I noticed in http://xenbits.
xen.org/people/ianc/tmp/38333/test-amd64-amd64-qemuu-nested/scape-
moth_l1.guest.osstest---var-log-daemon.log that the L1 seems to be running
systemd, yet preseed_create contains:

    # Systemd doesn't honor osstest-confirm-booted service, which
    # breaks ts-leak-check.  Fall back to SysV init for now.
    if ( $ho->{Suite} =~ /jessie/ ) {
       preseed_hook_command($ho, 'late_command', $sfx, <<END)
in-target apt-get install -y sysvinit-core
END
    }

Which it seems didn't take effect.

I think this is because ts-debian-hvm-install does not use preseed_create,
but instead uses preseed_base.

I think this snippet probably wants to move to preseed_base, I'll
investigate and send a patch.

Ian.

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

Re: [PATCH OSSTEST v4] Set {ident}_suite runvar when install a Debian guest.

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST v4] Set {ident}_suite runvar when install a Debian guest."):
> I think this is because ts-debian-hvm-install does not use preseed_create,
> but instead uses preseed_base.

That sounds very likely.

> I think this snippet probably wants to move to preseed_base, I'll
> investigate and send a patch.

Thanks,
Ian.