Skip to contentSkip to conversation

Fixes for distros-debian-* flights on armhf

PATCH Superseded Ian Campbell 2 participants 19 messages Aug 6, 2015 – Aug 13, 2015

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover Fixes for distros-debian-* flights on armhf

The armhf tests in distros-debian-* have always failed since they did not shutdown at the end of installation. I've now investigated that and the fix is patch #2 in this series.

The remainder of the series consists of various fixes, extensions to existing quirks and workarounds, mainly to the use of the pv-menu-list package on ARM when actually starting the guest (which was previously blocked by the install failure).

In addition to this a xen patch ("libxl: use correct command line for arm guests.") is also needed. That should be applied for 4.6 pretty soon (probably right after I've sent this mail...)

This will fix the armhf tests in distros-debian-jessie flights (and later) but won't fix the armhf -raw/qcow/vhd tests in other flights since they also suffer from lack of Xen support in Wheezy's kernel. Once we switch osstest to using Jessie by default then together with this series those things ought to spring into life.

Ian.

#1 lore raw

PATCH 1/7 ts-logs-capture: Collect /var/log/xen/bootloader.*.log

This is the pygrub debug log.

---
 ts-logs-capture | 1 +
 1 file changed, 1 insertion(+)
ts-logs-capture +1 -0
index 0081372..b99b1db 100755
@@ -143,6 +143,7 @@ sub fetch_logs_host () {
                   /var/log/xen/xend-debug.log*
                   /var/log/xen/xen-hotplug.log*
                   /var/log/xen/domain-builder-ng.log*
+                  /var/log/xen/bootloader.*.log
                   /var/log/xen/qemu-dm*
                   /var/log/xen/xl*.log
                   /var/log/xen/osstest*
Signature · 1 line
2.1.4
#2 lore raw

PATCH 1/7 ts-logs-capture: Collect /var/log/xen/bootloader.*.log

Ian Campbell writes ("[PATCH OSSTEST 1/7] ts-logs-capture: Collect /var/log/xen/bootloader.*.log"):

Quoted 1 line

This is the pygrub debug log.

#3 lore raw

PATCH 2/7 ts-debian-di-install: Use exit/poweroff in preference to exit/always_halt

In reply to Ian Campbell

always_halt results in d-i calling "halt", which does not necessarily poweroff the host (it seems to for x86/PV Xen guests, but does not for ARM). Using exit/poweroff calls "poweroff" which is equivalent to "halt -p", doing so results in ARM guests powering off as desired.

---
 ts-debian-di-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ts-debian-di-install +1 -1
index 34b8e1e..612e0fa 100755
@@ -210,7 +210,7 @@ END
     }
 
     my @cmdline = ();
-    push @cmdline, "debian-installer/exit/always_halt=true";
+    push @cmdline, "debian-installer/exit/poweroff=true";
     push @cmdline, "domain=$c{TestHostDomain}";
     push @cmdline, "console=hvc0";
     push @cmdline, di_installcmdline_core($gho, $ps_url);
Signature · 1 line
2.1.4
Acked-by on v1 2/7
#4 lore raw

Re: [PATCH OSSTEST 2/7] ts-debian-di-install: Use exit/poweroff in preference to exit/always_halt

Ian Campbell writes ("[PATCH OSSTEST 2/7] ts-debian-di-install: Use exit/poweroff in preference to exit/always_halt"):

Quoted 4 lines

always_halt results in d-i calling "halt", which does not necessarily poweroff the host (it seems to for x86/PV Xen guests, but does not for ARM). Using exit/poweroff calls "poweroff" which is equivalent to "halt -p", doing so results in ARM guests powering off as desired.

WTF

#5 lore raw

PATCH 3/7 ts-debian-di-install: Install pv-menu-list in ARM guests, always.

In reply to Ian Campbell
---
 ts-debian-di-install | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
ts-debian-di-install +10 -1
index 612e0fa..0e778e3 100755
@@ -190,9 +190,18 @@ END
 
 	$method_cfg = setup_netboot($tmpdir, $arch, $suite);
 
+	# We need the pv-menu-list package:
+	# - On x86 when running pvgrub, since it only speaks grub1
+	#   menu.lst syntax and by grub2 is now the only grub in
+	#   Debian.
+	# - On ARM, which uses pygrub, because grub2 is not installed
+	#   on ARM by default (except for, maybe, UEFI guests in the
+	#   future, but not today).
+	my $pvmenulst = ($bl eq "pvgrub" || $arch =~ /arm/);
+
 	$ps_url = preseed_create_guest($gho, $arch, '',
 				       Suite=>$suite,
-				       PvMenuLst=>($bl eq "pvgrub"));
+				       PvMenuLst=>$pvmenulst);
 
 	$extra_disk = "";
     }
Signature · 1 line
2.1.4
#6 lore raw

Re: [PATCH OSSTEST 3/7] ts-debian-di-install: Install pv-menu-list in ARM guests, always.

Ian Campbell writes ("[PATCH OSSTEST 3/7] ts-debian-di-install: Install pv-menu-list in ARM guests, always."):

Quoted 1 line

This package seems to be called pv-grub-menu', not pv-menu-list'.

ts-debian-di-install END · line 196

Quoted 8 lines
ts-debian-di-install +4 -0
@@ -190,9 +190,18 @@ END
 
 	$method_cfg = setup_netboot($tmpdir, $arch, $suite);
 
+	# We need the pv-menu-list package:
+	# - On x86 when running pvgrub, since it only speaks grub1
+	#   menu.lst syntax and by grub2 is now the only grub in
+	#   Debian.

This comment seems mangled. "and by grub2" ? Also in "it only speaks grub1", it would be clearer if "it" said "pvgrub".

ts-debian-di-install END · line 200

Quoted 4 lines
ts-debian-di-install +4 -0
+	# - On ARM, which uses pygrub, because grub2 is not installed
+	#   on ARM by default (except for, maybe, UEFI guests in the
+	#   future, but not today).
+	my $pvmenulst = ($bl eq "pvgrub" || $arch =~ /arm/);

"pvgrub" here is pvgrub1, not pv grub2. This could perhaps be clearer...

Thanks,
Ian.

#7 lore raw

Re: [PATCH OSSTEST 3/7] ts-debian-di-install: Install pv-menu-list in ARM guests, always.

Quoted 5 lines · Ian Jackson

Ian Campbell writes ("[PATCH OSSTEST 3/7] ts-debian-di-install: Install pv-menu-list in ARM guests, always."):

Quoted 1 line

This package seems to be called pv-grub-menu', not pv-menu-list'.

I get that wrong more often than not it seems...

Quoted 5 lines
Quoted 5 lines
ts-debian-di-install +0 -0
@@ -190,9 +190,18 @@ END
 
 	$method_cfg = setup_netboot($tmpdir, $arch, $suite);
 
Quoted 7 lines

ts-debian-di-install END · line 196

Quoted 4 lines
ts-debian-di-install +4 -0
+	# We need the pv-menu-list package:
+	# - On x86 when running pvgrub, since it only speaks grub1
+	#   menu.lst syntax and by grub2 is now the only grub in
+	#   Debian.

This comment seems mangled. "and by grub2" ? Also in "it only speaks grub1", it would be clearer if "it" said "pvgrub".

Sure.

Quoted 7 lines

ts-debian-di-install END · line 200

Quoted 4 lines
ts-debian-di-install +4 -0
+	# - On ARM, which uses pygrub, because grub2 is not installed
+	#   on ARM by default (except for, maybe, UEFI guests in the
+	#   future, but not today).
+	my $pvmenulst = ($bl eq "pvgrub" || $arch =~ /arm/);

"pvgrub" here is pvgrub1, not pv grub2. This could perhaps be clearer...

Yes, although pvgrub is what we use in production now. I could go and change it everywhere if you like, or we could defer to the point where we add pv grub 2 testing?

Quoted 3 lines

Thanks,
Ian.

#8 lore raw

Re: [PATCH OSSTEST 3/7] ts-debian-di-install: Install pv-menu-list in ARM guests, always.

Ian Campbell writes ("Re: [PATCH OSSTEST 3/7] ts-debian-di-install: Install pv-menu-list in ARM guests, always."):

Quoted 7 lines · Ian Jackson
Quoted 2 lines

"pvgrub" here is pvgrub1, not pv grub2. This could perhaps be clearer...

Yes, although pvgrub is what we use in production now. I could go and change it everywhere if you like, or we could defer to the point where we add pv grub 2 testing?

Maybe we could just have a comment somewhere.

Ian.

#9 lore raw

PATCH 4/7 ts-debian-di-install: Use the suite in the default hostname

In reply to Ian Campbell

This is more useful in standalone mode than having everything be "debian".

---
 ts-debian-di-install | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
ts-debian-di-install +1 -1
index 0e778e3..95a9215 100755
@@ -66,7 +66,7 @@ our $ho= selecthost($whhost);
 our $ram_mb=    512;
 our $disk_mb= 10000;
 
-our $guesthost= "$gn.guest.osstest";
+our $guesthost= ($r{"${gn}_suite"}//$gn).".guest.osstest";
 our $gho;
 
 sub prep () {
Signature · 1 line
2.1.4
#10 lore raw

Re: [PATCH OSSTEST 4/7] ts-debian-di-install: Use the suite in the default hostname

Ian Campbell writes ("[PATCH OSSTEST 4/7] ts-debian-di-install: Use the suite in the default hostname"):

Quoted 2 lines

This is more useful in standalone mode than having everything be "debian".

...

ts-debian-di-install our $ho= selecthost($whhost); · line 69

Quoted 2 lines
ts-debian-di-install +1 -1
-our $guesthost= "$gn.guest.osstest";
+our $guesthost= ($r{"${gn}_suite"}//$gn).".guest.osstest";

So if GN_suite is set, we use SUITE.guest.osstest, otherwise we use GN.guest.osstest.

But what if there are multiple GN with the same, specified, SUITE ?

I think $guesthost (which ends up in lv names, vm names, etc.) needs to have GN in it either way.

Ian.

#11 lore raw

PATCH 5/7 Debian: ARM: only apply no bootloader workaround if xopts{PvMenuLst}

In reply to Ian Campbell

This workaround is only necessary because of how pv-menu-list works, so we should only apply both or neither of them.

This results in a long line and I'm about to add a second workaround to this block, so switch to a regular if block instead of postfixing on the one command. Move the comment inside that block in preparation for other workarounds as well.

---
 Osstest/Debian.pm | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
Osstest/Debian.pm +9 -5
index 7ce5d67..ff7975e 100644
@@ -913,15 +913,19 @@ d-i     grub-installer/bootdev          string /dev/xvda
 
 END
 
-    # Debian doesn't currently know what bootloader to install in a
-    # Xen guest on ARM. We install pv-grub-menu above which actually
-    # does what we need, but the installer doesn't treat that as a
-    # "bootloader".
     logm("\$arch is $arch, \$suite is $suite");
-    $preseed_file.= (<<END) if $arch =~ /^arm/ && $suite =~ /wheezy|jessie|sid/;
+    if ($xopts{PvMenuLst} && $arch =~ /^arm/ &&
+	$suite =~ /wheezy|jessie|sid/ ) {
+
+	# Debian doesn't currently know what bootloader to install in
+	# a Xen guest on ARM. We install pv-grub-menu above which
+	# actually does what we need, but the installer doesn't treat
+	# that as a "bootloader".
+	$preseed_file.= (<<END);
 d-i     nobootloader/confirmation_common boolean true
 
 END
+    }
 
     $preseed_file .= preseed_hook_cmds();
 
Signature · 1 line
2.1.4
Acked-by on v1 5/7
#12 lore raw

Re: [PATCH OSSTEST 5/7] Debian: ARM: only apply no bootloader workaround if xopts{PvMenuLst}

Ian Campbell writes ("[PATCH OSSTEST 5/7] Debian: ARM: only apply no bootloader workaround if xopts{PvMenuLst}"):

Quoted 7 lines

This workaround is only necessary because of how pv-menu-list works, so we should only apply both or neither of them.

This results in a long line and I'm about to add a second workaround to this block, so switch to a regular if block instead of postfixing on the one command. Move the comment inside that block in preparation for other workarounds as well.

...

Osstest/Debian.pm d-i grub-installer/bootdev string /dev/xvda · line 918

Quoted 2 lines
Osstest/Debian.pm +2 -0
+    if ($xopts{PvMenuLst} && $arch =~ /^arm/ &&
+	$suite =~ /wheezy|jessie|sid/ ) {

If you're going to wrap this I would mildly prefer /all/ the &&s to generate a newline, so it is clearer that it's a 3-factor expression.

But, regardless,

#13 lore raw

PATCH 6/7 Debian: ARM has no bootloader (for Xen) even in Stretch.

In reply to Ian Campbell

Realistically this isn't going to change until we have either u-boot or UEFI in an arm32 guest.

---
 Osstest/Debian.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Osstest/Debian.pm +1 -1
index ff7975e..7a0ead4 100644
@@ -915,7 +915,7 @@ END
 
     logm("\$arch is $arch, \$suite is $suite");
     if ($xopts{PvMenuLst} && $arch =~ /^arm/ &&
-	$suite =~ /wheezy|jessie|sid/ ) {
+	$suite =~ /wheezy|jessie|stretch|sid/ ) {
 
 	# Debian doesn't currently know what bootloader to install in
 	# a Xen guest on ARM. We install pv-grub-menu above which
Signature · 1 line
2.1.4
Acked-by on v1 6/7
#14 lore raw

Re: [PATCH OSSTEST 6/7] Debian: ARM has no bootloader (for Xen) even in Stretch.

Ian Campbell writes ("[PATCH OSSTEST 6/7] Debian: ARM has no bootloader (for Xen) even in Stretch."):

Quoted 2 lines

Realistically this isn't going to change until we have either u-boot or UEFI in an arm32 guest.

#15 lore raw

PATCH 7/7 Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled

In reply to Ian Campbell
---
 Osstest/Debian.pm | 8 ++++++++
 1 file changed, 8 insertions(+)
Osstest/Debian.pm +8 -0
index 7a0ead4..07a71e2 100644
@@ -923,7 +923,15 @@ END
 	# that as a "bootloader".
 	$preseed_file.= (<<END);
 d-i     nobootloader/confirmation_common boolean true
+END
 
+        # Debian Bug #771949 means that update-menu-list always
+        # generates a full absolute path to the kernel + initrd, while
+        # by default the partition layout on ARM has a separate /boot.
+        preseed_hook_command($ho, 'late_command', $sfx, <<END);
+#!/bin/sh
+set -ex
+ln -s . /target/boot/boot
 END
     }
 
Signature · 1 line
2.1.4
#16 lore raw

Re: [PATCH OSSTEST 7/7] Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled

Ian Campbell writes ("[PATCH OSSTEST 7/7] Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled"):

Quoted 1 line

Can this please be conditional on the suite ?

Thanks,
Ian.

#17 lore raw

Re: [PATCH OSSTEST 7/7] Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled

Quoted 5 lines · Ian Jackson

Ian Campbell writes ("[PATCH OSSTEST 7/7] Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled"):

Quoted 1 line

Can this please be conditional on the suite ?

It is, it is within the:

    if ($xopts{PvMenuLst} &&
	$arch =~ /^arm/ &&
	$suite =~ /wheezy|jessie|stretch|sid/ ) {

block.

If/when this workaround and.or the no-bootloader workaround go away I would expect them to both go away together. If not the condition can be adjusted.

Ian.

Acked-by
#18 lore raw

Re: [PATCH OSSTEST 7/7] Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled

Ian Campbell writes ("Re: [PATCH OSSTEST 7/7] Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled"):

Quoted 7 lines · Ian Jackson
Quoted 5 lines

Ian Campbell writes ("[PATCH OSSTEST 7/7] Debian: Create /boot/boot -> . symlink on ARM when PvMenuLst enabled"):

Quoted 1 line

Can this please be conditional on the suite ?

Quoted 8 lines

It is, it is within the:

    if ($xopts{PvMenuLst} &&
	$arch =~ /^arm/ &&
	$suite =~ /wheezy|jessie|stretch|sid/ ) {

block.

If/when this workaround and.or the no-bootloader workaround go away I would expect them to both go away together. If not the condition can be adjusted.

Ah, OK.