Fixes for distros-debian-* flights on armhf
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.
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
@@ -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
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.
PATCH 2/7 ts-debian-di-install: Use exit/poweroff in preference to exit/always_halt
In reply to Ian Campbellalways_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
@@ -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
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_haltresults 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
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
@@ -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
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
Signed-off-by Ian Campbell <[email protected]>
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.
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
Signed-off-by Ian Campbell <[email protected]>This package seems to be called
pv-grub-menu', notpv-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.
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.
PATCH 4/7 ts-debian-di-install: Use the suite in the default hostname
In reply to Ian CampbellThis 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
@@ -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
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.
PATCH 5/7 Debian: ARM: only apply no bootloader workaround if xopts{PvMenuLst}
In reply to Ian CampbellThis 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
@@ -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
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,
PATCH 6/7 Debian: ARM has no bootloader (for Xen) even in Stretch.
In reply to Ian CampbellRealistically 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
@@ -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
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.
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
@@ -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
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
Signed-off-by Ian Campbell <[email protected]>
Can this please be conditional on the suite ?
Thanks,
Ian.
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
Signed-off-by Ian Campbell <[email protected]>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.
Ian Jackson
Acked-byRe: [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
Signed-off-by Ian Campbell <[email protected]>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.