* Re: [osstest test] 56922: regressions - FAIL
[not found] <osstest-56922-mainreport@xen.org>
@ 2015-05-22 9:57 ` Ian Campbell
2015-05-22 11:32 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-22 9:57 UTC (permalink / raw)
To: Pang, LongtaoX, Hu, Robert; +Cc: ian.jackson, xen-devel
Hi Robert and Longtao,
On Fri, 2015-05-22 at 02:08 +0000, osstest service user wrote:
> flight 56922 osstest real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/56922/
>
> Regressions :-(
This flight was testing:
477a9aa grub: remove patch to disable submenu from 20_linux_xen overla
cc0a60a Move the code for setting memory size into prep()
3d1ff6d Edit some APIs in TestSupport.pm for nested test
46bba78 Refactor installation of overlays
3cc3ef3 Changes to support '/boot' leading paths of kernel, xen, in gr
9419ffe Parsing grub which has 'submenu' primitive
Looking at
http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/info.html
and the serial log at
http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/serial-italia1.log
It seems it has picked the wrong entry to boot. It has picked entry 21
which seems to be 'Debian GNU/Linux, with Xen 4.6-unstable and Linux
3.14.43+' whereas I think it ought to have picked entry 22 which is
'Debian GNU/Linux, with Xen 4.6-unstable (XSM enabled) and Linux 3.14.43
+'
I think the problem is most likely in the new code which copes with the
submenu primitive.
Ian.
>
> Tests which did not succeed and are blocking,
> including tests which could not be run:
> test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 56857
> test-amd64-i386-xl-xsm 6 xen-boot fail REGR. vs. 56857
> test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 56857
> test-amd64-amd64-xl-xsm 6 xen-boot fail REGR. vs. 56857
> test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 56857
> test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 56857
> test-amd64-i386-freebsd10-amd64 14 guest-saverestore.2 fail REGR. vs. 56857
> test-armhf-armhf-xl-multivcpu 6 xen-boot fail REGR. vs. 56857
>
> Regressions which are regarded as allowable (not blocking):
> test-amd64-i386-libvirt-xsm 6 xen-boot fail REGR. vs. 56857
> test-amd64-amd64-libvirt-xsm 6 xen-boot fail REGR. vs. 56857
> test-armhf-armhf-libvirt 11 guest-start fail REGR. vs. 56857
> test-armhf-armhf-xl-xsm 11 guest-start fail blocked in 56857
> test-amd64-amd64-rumpuserxen-amd64 15 rumpuserxen-demo-xenstorels/xenstorels.repeat fail like 56857
> test-amd64-i386-libvirt 11 guest-start fail like 56857
> test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 56857
> test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 56857
> test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 56857
> test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 56857
> test-amd64-amd64-libvirt 11 guest-start fail like 56857
>
> Tests which did not succeed, but are not blocking:
> test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass
> test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass
> test-armhf-armhf-libvirt-xsm 11 guest-start fail never pass
> test-armhf-armhf-xl-sedf 12 migrate-support-check fail never pass
> test-armhf-armhf-xl-cubietruck 12 migrate-support-check fail never pass
> test-armhf-armhf-xl-sedf-pin 12 migrate-support-check fail never pass
> test-armhf-armhf-xl 12 migrate-support-check fail never pass
> test-armhf-armhf-xl-arndale 12 migrate-support-check fail never pass
> test-armhf-armhf-xl-credit2 12 migrate-support-check fail never pass
>
> version targeted for testing:
> osstest 477a9aa4fa8ff5eb3b0a2e286585665007bb9795
> baseline version:
> osstest ecf4cb76b15396c85fed5a2eb0a6a60e381225c5
>
> jobs:
> build-amd64-xsm pass
> build-armhf-xsm pass
> build-i386-xsm pass
> build-amd64 pass
> build-armhf pass
> build-i386 pass
> build-amd64-libvirt pass
> build-armhf-libvirt pass
> build-i386-libvirt pass
> build-amd64-pvops pass
> build-armhf-pvops pass
> build-i386-pvops pass
> build-amd64-rumpuserxen pass
> build-i386-rumpuserxen pass
> test-amd64-amd64-xl pass
> test-armhf-armhf-xl pass
> test-amd64-i386-xl pass
> test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm fail
> test-amd64-i386-xl-qemut-debianhvm-amd64-xsm fail
> test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm fail
> test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm fail
> test-amd64-amd64-libvirt-xsm fail
> test-armhf-armhf-libvirt-xsm fail
> test-amd64-i386-libvirt-xsm fail
> test-amd64-amd64-xl-xsm fail
> test-armhf-armhf-xl-xsm fail
> test-amd64-i386-xl-xsm fail
> test-amd64-amd64-xl-pvh-amd fail
> test-amd64-i386-qemut-rhel6hvm-amd pass
> test-amd64-i386-qemuu-rhel6hvm-amd pass
> test-amd64-amd64-xl-qemut-debianhvm-amd64 pass
> test-amd64-i386-xl-qemut-debianhvm-amd64 pass
> test-amd64-amd64-xl-qemuu-debianhvm-amd64 pass
> test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
> test-amd64-i386-freebsd10-amd64 fail
> test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
> test-amd64-i386-xl-qemuu-ovmf-amd64 pass
> test-amd64-amd64-rumpuserxen-amd64 fail
> test-amd64-amd64-xl-qemut-win7-amd64 fail
> test-amd64-i386-xl-qemut-win7-amd64 fail
> test-amd64-amd64-xl-qemuu-win7-amd64 fail
> test-amd64-i386-xl-qemuu-win7-amd64 fail
> test-armhf-armhf-xl-arndale pass
> test-amd64-amd64-xl-credit2 pass
> test-armhf-armhf-xl-credit2 pass
> test-armhf-armhf-xl-cubietruck pass
> test-amd64-i386-freebsd10-i386 pass
> test-amd64-i386-rumpuserxen-i386 pass
> test-amd64-amd64-xl-pvh-intel fail
> test-amd64-i386-qemut-rhel6hvm-intel pass
> test-amd64-i386-qemuu-rhel6hvm-intel pass
> test-amd64-amd64-libvirt fail
> test-armhf-armhf-libvirt fail
> test-amd64-i386-libvirt fail
> test-amd64-amd64-xl-multivcpu pass
> test-armhf-armhf-xl-multivcpu fail
> test-amd64-amd64-pair pass
> test-amd64-i386-pair pass
> test-amd64-amd64-xl-sedf-pin pass
> test-armhf-armhf-xl-sedf-pin pass
> test-amd64-amd64-xl-sedf pass
> test-armhf-armhf-xl-sedf pass
> test-amd64-i386-xl-qemut-winxpsp3-vcpus1 pass
> test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
> test-amd64-amd64-xl-qemut-winxpsp3 pass
> test-amd64-i386-xl-qemut-winxpsp3 pass
> test-amd64-amd64-xl-qemuu-winxpsp3 pass
> test-amd64-i386-xl-qemuu-winxpsp3 pass
>
>
> ------------------------------------------------------------
> sg-report-flight on osstest.test-lab.xenproject.org
> logs: /home/logs/logs
> images: /home/logs/images
>
> Logs, config files, etc. are available at
> http://logs.test-lab.xenproject.org/osstest/logs
>
> Test harness code can be found at
> http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
>
>
> Not pushing.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 9:57 ` [osstest test] 56922: regressions - FAIL Ian Campbell
@ 2015-05-22 11:32 ` Ian Campbell
2015-05-22 12:30 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-22 11:32 UTC (permalink / raw)
To: Pang, LongtaoX; +Cc: Hu, Robert, ian.jackson, xen-devel
On Fri, 2015-05-22 at 10:57 +0100, Ian Campbell wrote:
> Hi Robert and Longtao,
>
> On Fri, 2015-05-22 at 02:08 +0000, osstest service user wrote:
> > flight 56922 osstest real [real]
> > http://logs.test-lab.xenproject.org/osstest/logs/56922/
> >
> > Regressions :-(
>
> This flight was testing:
>
> 477a9aa grub: remove patch to disable submenu from 20_linux_xen overla
> cc0a60a Move the code for setting memory size into prep()
> 3d1ff6d Edit some APIs in TestSupport.pm for nested test
> 46bba78 Refactor installation of overlays
> 3cc3ef3 Changes to support '/boot' leading paths of kernel, xen, in gr
> 9419ffe Parsing grub which has 'submenu' primitive
>
> Looking at
> http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/info.html
> and the serial log at
> http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/serial-italia1.log
>
> It seems it has picked the wrong entry to boot. It has picked entry 21
> which seems to be 'Debian GNU/Linux, with Xen 4.6-unstable and Linux
> 3.14.43+' whereas I think it ought to have picked entry 22 which is
> 'Debian GNU/Linux, with Xen 4.6-unstable (XSM enabled) and Linux 3.14.43
> +'
>
> I think the problem is most likely in the new code which copes with the
> submenu primitive.
I think grub2 considers submenu entries to be entries, and therefore
when determining which entry to use we should include those in the
offset.
I think you didn't see this in your testing because without XSM the
entry we want is the first item in the first submenu, so there is an off
by one in the entry we ask for, which turns out to correspond to the
first submenu itself, and then the grub timeout means that it then
selects the first entry in that submenu, so it works.
I'm currently testing the patch below, if it works then I intend to fold
it into "Parsing grub which has 'submenu' primitive" and will re-push
the result.
Ian.
diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 282175b..0b731c3 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -451,6 +451,7 @@ sub setupboot_grub2 ($$$$) {
}
if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
$submenu={ StartLine =>$.};
+ $count++;
}
if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
die unless $entry;
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 11:32 ` Ian Campbell
@ 2015-05-22 12:30 ` Ian Campbell
2015-05-22 12:47 ` Robert Hu
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-22 12:30 UTC (permalink / raw)
To: Pang, LongtaoX; +Cc: Hu, Robert, ian.jackson, xen-devel
On Fri, 2015-05-22 at 12:32 +0100, Ian Campbell wrote:
> I'm currently testing the patch below, if it works then I intend to fold
> it into "Parsing grub which has 'submenu' primitive" and will re-push
> the result.
It didn't seem to work. I'm trying to repro on a machine I can recover
more easily, but perhaps you could also take a look?
> Ian.
>
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 282175b..0b731c3 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -451,6 +451,7 @@ sub setupboot_grub2 ($$$$) {
> }
> if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> $submenu={ StartLine =>$.};
> + $count++;
> }
> if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
> die unless $entry;
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 12:30 ` Ian Campbell
@ 2015-05-22 12:47 ` Robert Hu
2015-05-22 12:58 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Robert Hu @ 2015-05-22 12:47 UTC (permalink / raw)
To: Ian Campbell; +Cc: Hu, Robert, Pang, LongtaoX, ian.jackson, xen-devel
On Fri, 2015-05-22 at 13:30 +0100, Ian Campbell wrote:
> On Fri, 2015-05-22 at 12:32 +0100, Ian Campbell wrote:
> > I'm currently testing the patch below, if it works then I intend to fold
> > it into "Parsing grub which has 'submenu' primitive" and will re-push
> > the result.
>
> It didn't seem to work. I'm trying to repro on a machine I can recover
> more easily, but perhaps you could also take a look?
Yeah. Thanks Ian.
I'm to look into it this weekend.
>
> > Ian.
> >
> > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> > index 282175b..0b731c3 100644
> > --- a/Osstest/Debian.pm
> > +++ b/Osstest/Debian.pm
> > @@ -451,6 +451,7 @@ sub setupboot_grub2 ($$$$) {
> > }
> > if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
> > $submenu={ StartLine =>$.};
> > + $count++;
> > }
> > if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
> > die unless $entry;
> >
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 12:47 ` Robert Hu
@ 2015-05-22 12:58 ` Ian Campbell
2015-05-22 13:42 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-22 12:58 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, xen-devel
On Fri, 2015-05-22 at 20:47 +0800, Robert Hu wrote:
> On Fri, 2015-05-22 at 13:30 +0100, Ian Campbell wrote:
> > On Fri, 2015-05-22 at 12:32 +0100, Ian Campbell wrote:
> > > I'm currently testing the patch below, if it works then I intend to fold
> > > it into "Parsing grub which has 'submenu' primitive" and will re-push
> > > the result.
> >
> > It didn't seem to work. I'm trying to repro on a machine I can recover
> > more easily, but perhaps you could also take a look?
> Yeah. Thanks Ian.
> I'm to look into it this weekend.
>From https://help.ubuntu.com/community/Grub2/Submenus it seems like
perhaps the syntax is "N>M" where N is the submenu and M is the offset
within that submenu.
I've not confirmed this though.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 12:58 ` Ian Campbell
@ 2015-05-22 13:42 ` Ian Campbell
2015-05-22 14:21 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-22 13:42 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, xen-devel
On Fri, 2015-05-22 at 13:58 +0100, Ian Campbell wrote:
> On Fri, 2015-05-22 at 20:47 +0800, Robert Hu wrote:
> > On Fri, 2015-05-22 at 13:30 +0100, Ian Campbell wrote:
> > > On Fri, 2015-05-22 at 12:32 +0100, Ian Campbell wrote:
> > > > I'm currently testing the patch below, if it works then I intend to fold
> > > > it into "Parsing grub which has 'submenu' primitive" and will re-push
> > > > the result.
> > >
> > > It didn't seem to work. I'm trying to repro on a machine I can recover
> > > more easily, but perhaps you could also take a look?
> > Yeah. Thanks Ian.
> > I'm to look into it this weekend.
>
> From https://help.ubuntu.com/community/Grub2/Submenus it seems like
> perhaps the syntax is "N>M" where N is the submenu and M is the offset
> within that submenu.
>
> I've not confirmed this though.
I have now, 8>1 correctly booted: submenu "Xen 4.6-unstable" {
menuentry 'Debian GNU/Linux, with Xen 4.6-unstable and Linux 3.14.36+' --class debian --class gnu-linux --class gnu --class os --class xen {
>From my particular grub.cfg. For real usage setupboot_grub2 will
obviously need to become cleverer to count things correctly.
BTW, I just noticed that the comment about setupboot_grub2 is still in
place:
# Note on running OSSTest on Squeeze with old Xen kernel: check out
# Debian bug #633127 "/etc/grub/20_linux does not recognise some old
# Xen kernels"
# Currently setupboot_grub2 relies on Grub menu not having submenu.
# Check Debian bug #690538.
sub setupboot_grub2 ($$$$) {
The last bit is no longer true with the patch, so it should be
removed...
I've dropped the following from pretest for now:
Parsing grub which has 'submenu' primitive
Changes to support '/boot' leading paths of kernel, xen, in grub
grub: remove patch to disable submenu from 20_linux_xen overlay
(the /boot one had contextual dependencies on the prior patch).
In pretest now is just:
Refactor installation of overlays
Edit some APIs in TestSupport.pm for nested test
Move the code for setting memory size into prep()
I've pushed that to a new branch nestedhvm-v10-pretest-reduced in my
xenbits repo too so you can base future resends on that.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 13:42 ` Ian Campbell
@ 2015-05-22 14:21 ` Ian Campbell
2015-05-23 3:35 ` Robert Hu
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Ian Campbell @ 2015-05-22 14:21 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, xen-devel
On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> From my particular grub.cfg. For real usage setupboot_grub2 will
> obviously need to become cleverer to count things correctly.
I've not tested extensively but the following incremental patch seems to
do the right thing, at least by inspection of the resulting grub.cfg.
Needs more testing (e.g. I haven't tried non-XSM yet) and review from
Ian I think, since there may be a more idiomatically Perl way to
manipulate the @offsets array (in particular shrinking it).
Ian.
diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 282175b..b5148fd 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -393,8 +393,6 @@ sub setupboot_grub1 ($$$$) {
# Note on running OSSTest on Squeeze with old Xen kernel: check out
# Debian bug #633127 "/etc/grub/20_linux does not recognise some old
# Xen kernels"
-# Currently setupboot_grub2 relies on Grub menu not having submenu.
-# Check Debian bug #690538.
sub setupboot_grub2 ($$$$) {
my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
my $bl= { };
@@ -405,7 +403,7 @@ 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>) {
@@ -417,6 +415,8 @@ sub setupboot_grub2 ($$$$) {
"$submenu->{StartLine}. ".
"Our want kern is $want_kernver");
$submenu=undef;
+ $#offsets = $#offsets-1;
+ $offsets[$#offsets]++;
next;
}
my (@missing) =
@@ -446,11 +446,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 =>$.};
+ $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
+ $offsets[$#offsets+1] = 0;
}
if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
die unless $entry;
@@ -511,7 +512,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;
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 14:21 ` Ian Campbell
@ 2015-05-23 3:35 ` Robert Hu
2015-05-23 6:46 ` Robert Hu
2015-05-23 8:31 ` Robert Hu
2015-05-23 13:34 ` Robert Hu
2 siblings, 1 reply; 19+ messages in thread
From: Robert Hu @ 2015-05-23 3:35 UTC (permalink / raw)
To: Ian Campbell; +Cc: robert.hu, Pang, LongtaoX, ian.jackson, xen-devel
On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > From my particular grub.cfg. For real usage setupboot_grub2 will
> > obviously need to become cleverer to count things correctly.
>
> I've not tested extensively but the following incremental patch seems to
> do the right thing, at least by inspection of the resulting grub.cfg.
>
> Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> Ian I think, since there may be a more idiomatically Perl way to
> manipulate the @offsets array (in particular shrinking it).
Thanks Ian. You are so quick. I wrote that piece of code almost a year
ago; was just about to warm up.
We're to test your fix in our environment as well on non-XSM.
> Ian.
>
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 282175b..b5148fd 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -393,8 +393,6 @@ sub setupboot_grub1 ($$$$) {
> # Note on running OSSTest on Squeeze with old Xen kernel: check out
> # Debian bug #633127 "/etc/grub/20_linux does not recognise some old
> # Xen kernels"
> -# Currently setupboot_grub2 relies on Grub menu not having submenu.
> -# Check Debian bug #690538.
> sub setupboot_grub2 ($$$$) {
> my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
> my $bl= { };
> @@ -405,7 +403,7 @@ 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>) {
> @@ -417,6 +415,8 @@ sub setupboot_grub2 ($$$$) {
> "$submenu->{StartLine}. ".
> "Our want kern is $want_kernver");
> $submenu=undef;
> + $#offsets = $#offsets-1;
> + $offsets[$#offsets]++;
> next;
> }
> my (@missing) =
> @@ -446,11 +446,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 =>$.};
> + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> + $offsets[$#offsets+1] = 0;
> }
> if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
> die unless $entry;
> @@ -511,7 +512,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;
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-23 3:35 ` Robert Hu
@ 2015-05-23 6:46 ` Robert Hu
2015-05-23 6:58 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Robert Hu @ 2015-05-23 6:46 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, Ian Campbell, xen-devel
On Sat, 2015-05-23 at 11:35 +0800, Robert Hu wrote:
> On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> > On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > > From my particular grub.cfg. For real usage setupboot_grub2 will
> > > obviously need to become cleverer to count things correctly.
> >
> > I've not tested extensively but the following incremental patch seems to
> > do the right thing, at least by inspection of the resulting grub.cfg.
> >
> > Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> > Ian I think, since there may be a more idiomatically Perl way to
> > manipulate the @offsets array (in particular shrinking it).
> Thanks Ian. You are so quick. I wrote that piece of code almost a year
> ago; was just about to warm up.
> We're to test your fix in our environment as well on non-XSM.
Just furbished up that piece of code. Now I recall the memory. This
piece of code was developed before 'XSM' things introduced; though now
after XSM things merged. May I know what is 'xsm' stuff?
Can you send me the 'grub.cfg' of those 'xsm' cases? so that I can take
a look.
> > Ian.
> >
> > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> > index 282175b..b5148fd 100644
> > --- a/Osstest/Debian.pm
> > +++ b/Osstest/Debian.pm
> > @@ -393,8 +393,6 @@ sub setupboot_grub1 ($$$$) {
> > # Note on running OSSTest on Squeeze with old Xen kernel: check out
> > # Debian bug #633127 "/etc/grub/20_linux does not recognise some old
> > # Xen kernels"
> > -# Currently setupboot_grub2 relies on Grub menu not having submenu.
> > -# Check Debian bug #690538.
> > sub setupboot_grub2 ($$$$) {
> > my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
> > my $bl= { };
> > @@ -405,7 +403,7 @@ 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>) {
> > @@ -417,6 +415,8 @@ sub setupboot_grub2 ($$$$) {
> > "$submenu->{StartLine}. ".
> > "Our want kern is $want_kernver");
> > $submenu=undef;
> > + $#offsets = $#offsets-1;
> > + $offsets[$#offsets]++;
> > next;
> > }
> > my (@missing) =
> > @@ -446,11 +446,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 =>$.};
> > + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> > + $offsets[$#offsets+1] = 0;
> > }
> > if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
> > die unless $entry;
> > @@ -511,7 +512,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;
> >
> >
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-23 6:46 ` Robert Hu
@ 2015-05-23 6:58 ` Ian Campbell
2015-05-23 7:52 ` Robert Hu
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-23 6:58 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, xen-devel
On Sat, 2015-05-23 at 14:46 +0800, Robert Hu wrote:
> On Sat, 2015-05-23 at 11:35 +0800, Robert Hu wrote:
> > On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> > > On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > > > From my particular grub.cfg. For real usage setupboot_grub2 will
> > > > obviously need to become cleverer to count things correctly.
> > >
> > > I've not tested extensively but the following incremental patch seems to
> > > do the right thing, at least by inspection of the resulting grub.cfg.
> > >
> > > Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> > > Ian I think, since there may be a more idiomatically Perl way to
> > > manipulate the @offsets array (in particular shrinking it).
> > Thanks Ian. You are so quick. I wrote that piece of code almost a year
> > ago; was just about to warm up.
> > We're to test your fix in our environment as well on non-XSM.
>
> Just furbished up that piece of code. Now I recall the memory. This
> piece of code was developed before 'XSM' things introduced; though now
> after XSM things merged. May I know what is 'xsm' stuff?
XSM is Xen Security Modules (sort of selinux for Xen). It involves some
special additional entries in grub to provide the policy.
> Can you send me the 'grub.cfg' of those 'xsm' cases? so that I can take
> a look.
All of the logs for the flight can be found at:
http://logs.test-lab.xenproject.org/osstest/logs/56922/
Clicking the heading of one of the failing tests would take you to e.g.:
http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/info.html
and in there is:
http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/italia1--grub.cfg.1
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-23 6:58 ` Ian Campbell
@ 2015-05-23 7:52 ` Robert Hu
2015-05-23 8:25 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Robert Hu @ 2015-05-23 7:52 UTC (permalink / raw)
To: Ian Campbell; +Cc: robert.hu, Pang, LongtaoX, ian.jackson, xen-devel
On Sat, 2015-05-23 at 07:58 +0100, Ian Campbell wrote:
> On Sat, 2015-05-23 at 14:46 +0800, Robert Hu wrote:
> > On Sat, 2015-05-23 at 11:35 +0800, Robert Hu wrote:
> > > On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> > > > On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > > > > From my particular grub.cfg. For real usage setupboot_grub2 will
> > > > > obviously need to become cleverer to count things correctly.
> > > >
> > > > I've not tested extensively but the following incremental patch seems to
> > > > do the right thing, at least by inspection of the resulting grub.cfg.
> > > >
> > > > Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> > > > Ian I think, since there may be a more idiomatically Perl way to
> > > > manipulate the @offsets array (in particular shrinking it).
> > > Thanks Ian. You are so quick. I wrote that piece of code almost a year
> > > ago; was just about to warm up.
> > > We're to test your fix in our environment as well on non-XSM.
> >
> > Just furbished up that piece of code. Now I recall the memory. This
> > piece of code was developed before 'XSM' things introduced; though now
> > after XSM things merged. May I know what is 'xsm' stuff?
>
> XSM is Xen Security Modules (sort of selinux for Xen). It involves some
> special additional entries in grub to provide the policy.
>
> > Can you send me the 'grub.cfg' of those 'xsm' cases? so that I can take
> > a look.
>
> All of the logs for the flight can be found at:
>
> http://logs.test-lab.xenproject.org/osstest/logs/56922/
>
> Clicking the heading of one of the failing tests would take you to e.g.:
>
> http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/info.html
>From the log I see
'boot check: grub2, found Debian GNU/Linux, with Xen 4.6-unstable (XSM
enabled) and Linux 3.14.43+'
Seems it finds what ought to be.
>
> and in there is:
>
> http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/italia1--grub.cfg.1
>
> Ian.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-23 7:52 ` Robert Hu
@ 2015-05-23 8:25 ` Ian Campbell
2015-05-23 8:28 ` Robert Hu
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-23 8:25 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, xen-devel
On Sat, 2015-05-23 at 15:52 +0800, Robert Hu wrote:
> On Sat, 2015-05-23 at 07:58 +0100, Ian Campbell wrote:
> > On Sat, 2015-05-23 at 14:46 +0800, Robert Hu wrote:
> > > On Sat, 2015-05-23 at 11:35 +0800, Robert Hu wrote:
> > > > On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> > > > > On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > > > > > From my particular grub.cfg. For real usage setupboot_grub2 will
> > > > > > obviously need to become cleverer to count things correctly.
> > > > >
> > > > > I've not tested extensively but the following incremental patch seems to
> > > > > do the right thing, at least by inspection of the resulting grub.cfg.
> > > > >
> > > > > Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> > > > > Ian I think, since there may be a more idiomatically Perl way to
> > > > > manipulate the @offsets array (in particular shrinking it).
> > > > Thanks Ian. You are so quick. I wrote that piece of code almost a year
> > > > ago; was just about to warm up.
> > > > We're to test your fix in our environment as well on non-XSM.
> > >
> > > Just furbished up that piece of code. Now I recall the memory. This
> > > piece of code was developed before 'XSM' things introduced; though now
> > > after XSM things merged. May I know what is 'xsm' stuff?
> >
> > XSM is Xen Security Modules (sort of selinux for Xen). It involves some
> > special additional entries in grub to provide the policy.
> >
> > > Can you send me the 'grub.cfg' of those 'xsm' cases? so that I can take
> > > a look.
> >
> > All of the logs for the flight can be found at:
> >
> > http://logs.test-lab.xenproject.org/osstest/logs/56922/
> >
> > Clicking the heading of one of the failing tests would take you to e.g.:
> >
> > http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/info.html
> From the log I see
> 'boot check: grub2, found Debian GNU/Linux, with Xen 4.6-unstable (XSM
> enabled) and Linux 3.14.43+'
> Seems it finds what ought to be.
Yes, but the corresponding thing which it puts into GRUB_DEFAULT (which
becomes "default" in grub.cfg) is "21" which is not correct, it needs to
be something like "8>1" to correctly refer to that entry.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-23 8:25 ` Ian Campbell
@ 2015-05-23 8:28 ` Robert Hu
0 siblings, 0 replies; 19+ messages in thread
From: Robert Hu @ 2015-05-23 8:28 UTC (permalink / raw)
To: Ian Campbell; +Cc: robert.hu, Pang, LongtaoX, ian.jackson, xen-devel
On Sat, 2015-05-23 at 09:25 +0100, Ian Campbell wrote:
> On Sat, 2015-05-23 at 15:52 +0800, Robert Hu wrote:
> > On Sat, 2015-05-23 at 07:58 +0100, Ian Campbell wrote:
> > > On Sat, 2015-05-23 at 14:46 +0800, Robert Hu wrote:
> > > > On Sat, 2015-05-23 at 11:35 +0800, Robert Hu wrote:
> > > > > On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> > > > > > On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > > > > > > From my particular grub.cfg. For real usage setupboot_grub2 will
> > > > > > > obviously need to become cleverer to count things correctly.
> > > > > >
> > > > > > I've not tested extensively but the following incremental patch seems to
> > > > > > do the right thing, at least by inspection of the resulting grub.cfg.
> > > > > >
> > > > > > Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> > > > > > Ian I think, since there may be a more idiomatically Perl way to
> > > > > > manipulate the @offsets array (in particular shrinking it).
> > > > > Thanks Ian. You are so quick. I wrote that piece of code almost a year
> > > > > ago; was just about to warm up.
> > > > > We're to test your fix in our environment as well on non-XSM.
> > > >
> > > > Just furbished up that piece of code. Now I recall the memory. This
> > > > piece of code was developed before 'XSM' things introduced; though now
> > > > after XSM things merged. May I know what is 'xsm' stuff?
> > >
> > > XSM is Xen Security Modules (sort of selinux for Xen). It involves some
> > > special additional entries in grub to provide the policy.
> > >
> > > > Can you send me the 'grub.cfg' of those 'xsm' cases? so that I can take
> > > > a look.
> > >
> > > All of the logs for the flight can be found at:
> > >
> > > http://logs.test-lab.xenproject.org/osstest/logs/56922/
> > >
> > > Clicking the heading of one of the failing tests would take you to e.g.:
> > >
> > > http://logs.test-lab.xenproject.org/osstest/logs/56922/test-amd64-amd64-xl-xsm/info.html
> > From the log I see
> > 'boot check: grub2, found Debian GNU/Linux, with Xen 4.6-unstable (XSM
> > enabled) and Linux 3.14.43+'
> > Seems it finds what ought to be.
>
> Yes, but the corresponding thing which it puts into GRUB_DEFAULT (which
> becomes "default" in grub.cfg) is "21" which is not correct, it needs to
> be something like "8>1" to correctly refer to that entry.
Ah, I see.
>
> Ian.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 14:21 ` Ian Campbell
2015-05-23 3:35 ` Robert Hu
@ 2015-05-23 8:31 ` Robert Hu
2015-05-23 8:53 ` Robert Hu
2015-05-23 13:34 ` Robert Hu
2 siblings, 1 reply; 19+ messages in thread
From: Robert Hu @ 2015-05-23 8:31 UTC (permalink / raw)
To: Ian Campbell; +Cc: robert.hu, Pang, LongtaoX, ian.jackson, xen-devel
On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > From my particular grub.cfg. For real usage setupboot_grub2 will
> > obviously need to become cleverer to count things correctly.
>
> I've not tested extensively but the following incremental patch seems to
> do the right thing, at least by inspection of the resulting grub.cfg.
>
> Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> Ian I think, since there may be a more idiomatically Perl way to
> manipulate the @offsets array (in particular shrinking it).
>
> Ian.
>
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 282175b..b5148fd 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -393,8 +393,6 @@ sub setupboot_grub1 ($$$$) {
> # Note on running OSSTest on Squeeze with old Xen kernel: check out
> # Debian bug #633127 "/etc/grub/20_linux does not recognise some old
> # Xen kernels"
> -# Currently setupboot_grub2 relies on Grub menu not having submenu.
> -# Check Debian bug #690538.
> sub setupboot_grub2 ($$$$) {
> my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
> my $bl= { };
> @@ -405,7 +403,7 @@ 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>) {
> @@ -417,6 +415,8 @@ sub setupboot_grub2 ($$$$) {
> "$submenu->{StartLine}. ".
> "Our want kern is $want_kernver");
> $submenu=undef;
> + $#offsets = $#offsets-1;
> + $offsets[$#offsets]++;
> next;
> }
> my (@missing) =
> @@ -446,11 +446,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 =>$.};
> + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> + $offsets[$#offsets+1] = 0;
I see your points now.
Seems you even considers nested submenu in the future.:)
Just 1 question (I'm not a Perl expert): after "$offsets[$#offsets+1] =
0;", $#offsets increases automatically/implicitly, right?
> }
> if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
> die unless $entry;
> @@ -511,7 +512,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;
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-23 8:31 ` Robert Hu
@ 2015-05-23 8:53 ` Robert Hu
2015-05-24 13:53 ` Ian Campbell
0 siblings, 1 reply; 19+ messages in thread
From: Robert Hu @ 2015-05-23 8:53 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, Ian Campbell, xen-devel
On Sat, 2015-05-23 at 16:31 +0800, Robert Hu wrote:
> On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> > On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > > From my particular grub.cfg. For real usage setupboot_grub2 will
> > > obviously need to become cleverer to count things correctly.
> >
> > I've not tested extensively but the following incremental patch seems to
> > do the right thing, at least by inspection of the resulting grub.cfg.
> >
> > Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> > Ian I think, since there may be a more idiomatically Perl way to
> > manipulate the @offsets array (in particular shrinking it).
Carefully read your code and I think they can handle submenu (even
submenu in a submenu) correctly. Thanks for correcting my previous
implementation.
Just my 2 cents below inline.
> >
> > Ian.
> >
> > diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> > index 282175b..b5148fd 100644
> > --- a/Osstest/Debian.pm
> > +++ b/Osstest/Debian.pm
> > @@ -393,8 +393,6 @@ sub setupboot_grub1 ($$$$) {
> > # Note on running OSSTest on Squeeze with old Xen kernel: check out
> > # Debian bug #633127 "/etc/grub/20_linux does not recognise some old
> > # Xen kernels"
> > -# Currently setupboot_grub2 relies on Grub menu not having submenu.
> > -# Check Debian bug #690538.
> > sub setupboot_grub2 ($$$$) {
> > my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
> > my $bl= { };
> > @@ -405,7 +403,7 @@ 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>) {
> > @@ -417,6 +415,8 @@ sub setupboot_grub2 ($$$$) {
> > "$submenu->{StartLine}. ".
> > "Our want kern is $want_kernver");
> > $submenu=undef;
> > + $#offsets = $#offsets-1;
> > + $offsets[$#offsets]++;
may consider 'pop/push' operations on @offsets array? I worry about if
'$#offsets-1' can always shrink array size correctly. pop/push anyway is
some official way to do this.
> > next;
> > }
> > my (@missing) =
> > @@ -446,11 +446,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 =>$.};
> > + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
Here MenuEntryPath element for $submenu is actually debug purpose only,
I think, may be can remove it. correct me if I'm wrong.
> > + $offsets[$#offsets+1] = 0;
> I see your points now.
> Seems you even considers nested submenu in the future.:)
> Just 1 question (I'm not a Perl expert): after "$offsets[$#offsets+1] =
> 0;", $#offsets increases automatically/implicitly, right?
> > }
> > if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
> > die unless $entry;
> > @@ -511,7 +512,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;
> >
> >
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-22 14:21 ` Ian Campbell
2015-05-23 3:35 ` Robert Hu
2015-05-23 8:31 ` Robert Hu
@ 2015-05-23 13:34 ` Robert Hu
2 siblings, 0 replies; 19+ messages in thread
From: Robert Hu @ 2015-05-23 13:34 UTC (permalink / raw)
To: Ian Campbell; +Cc: robert.hu, Pang, LongtaoX, ian.jackson, xen-devel
On Fri, 2015-05-22 at 15:21 +0100, Ian Campbell wrote:
> On Fri, 2015-05-22 at 14:42 +0100, Ian Campbell wrote:
> > From my particular grub.cfg. For real usage setupboot_grub2 will
> > obviously need to become cleverer to count things correctly.
>
> I've not tested extensively but the following incremental patch seems to
> do the right thing, at least by inspection of the resulting grub.cfg.
>
> Needs more testing (e.g. I haven't tried non-XSM yet) and review from
> Ian I think, since there may be a more idiomatically Perl way to
> manipulate the @offsets array (in particular shrinking it).
I've done unit test with this piece of code, by making a sub comprised
of this piece of code and constructing various conditions: xsm, non-xsm,
xenhopt defined/undefined, all works as expected.
Of course, we'll further test this in real job-running cases.
>
> Ian.
>
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 282175b..b5148fd 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -393,8 +393,6 @@ sub setupboot_grub1 ($$$$) {
> # Note on running OSSTest on Squeeze with old Xen kernel: check out
> # Debian bug #633127 "/etc/grub/20_linux does not recognise some old
> # Xen kernels"
> -# Currently setupboot_grub2 relies on Grub menu not having submenu.
> -# Check Debian bug #690538.
> sub setupboot_grub2 ($$$$) {
> my ($ho,$want_kernver,$want_xsm,$xenhopt,$xenkopt) = @_;
> my $bl= { };
> @@ -405,7 +403,7 @@ 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>) {
> @@ -417,6 +415,8 @@ sub setupboot_grub2 ($$$$) {
> "$submenu->{StartLine}. ".
> "Our want kern is $want_kernver");
> $submenu=undef;
> + $#offsets = $#offsets-1;
> + $offsets[$#offsets]++;
> next;
> }
> my (@missing) =
> @@ -446,11 +446,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 =>$.};
> + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> + $offsets[$#offsets+1] = 0;
> }
> if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
> die unless $entry;
> @@ -511,7 +512,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;
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-23 8:53 ` Robert Hu
@ 2015-05-24 13:53 ` Ian Campbell
2015-05-25 2:25 ` Robert Hu
0 siblings, 1 reply; 19+ messages in thread
From: Ian Campbell @ 2015-05-24 13:53 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, xen-devel
On Sat, 2015-05-23 at 16:53 +0800, Robert Hu wrote:
> > > + $#offsets = $#offsets-1;
> > > + $offsets[$#offsets]++;
> may consider 'pop/push' operations on @offsets array? I worry about if
> '$#offsets-1' can always shrink array size correctly. pop/push anyway is
> some official way to do this.
Assigning to $#offsets is defined to change the length of the array. But
I think you are correct that pop/push would be a more idiomatic way to
do this.
It is a public holiday on Monday, but I'll take a look on Tuesday,
unless you fancy doing it in the meantime.
> @@ -446,11 +446,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 =>$.};
> > > + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> Here MenuEntryPath element for $submenu is actually debug purpose only,
> I think, may be can remove it. correct me if I'm wrong.
It is just for debug, I think it would be OK to leave it though.
Ian.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-24 13:53 ` Ian Campbell
@ 2015-05-25 2:25 ` Robert Hu
2015-05-25 3:04 ` Robert Hu
0 siblings, 1 reply; 19+ messages in thread
From: Robert Hu @ 2015-05-25 2:25 UTC (permalink / raw)
To: Ian Campbell; +Cc: robert.hu, Pang, LongtaoX, ian.jackson, xen-devel
On Sun, 2015-05-24 at 14:53 +0100, Ian Campbell wrote:
> On Sat, 2015-05-23 at 16:53 +0800, Robert Hu wrote:
> > > > + $#offsets = $#offsets-1;
> > > > + $offsets[$#offsets]++;
> > may consider 'pop/push' operations on @offsets array? I worry about if
> > '$#offsets-1' can always shrink array size correctly. pop/push anyway is
> > some official way to do this.
>
> Assigning to $#offsets is defined to change the length of the array. But
> I think you are correct that pop/push would be a more idiomatic way to
> do this.
>
> It is a public holiday on Monday, but I'll take a look on Tuesday,
> unless you fancy doing it in the meantime.
Sure I will do this.
>
> > @@ -446,11 +446,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 =>$.};
> > > > + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> > Here MenuEntryPath element for $submenu is actually debug purpose only,
> > I think, may be can remove it. correct me if I'm wrong.
>
> It is just for debug, I think it would be OK to leave it though.
>
> Ian.
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [osstest test] 56922: regressions - FAIL
2015-05-25 2:25 ` Robert Hu
@ 2015-05-25 3:04 ` Robert Hu
0 siblings, 0 replies; 19+ messages in thread
From: Robert Hu @ 2015-05-25 3:04 UTC (permalink / raw)
To: robert.hu; +Cc: Pang, LongtaoX, ian.jackson, Ian Campbell, xen-devel
On Mon, 2015-05-25 at 10:25 +0800, Robert Hu wrote:
> On Sun, 2015-05-24 at 14:53 +0100, Ian Campbell wrote:
> > On Sat, 2015-05-23 at 16:53 +0800, Robert Hu wrote:
> > > > > + $#offsets = $#offsets-1;
> > > > > + $offsets[$#offsets]++;
> > > may consider 'pop/push' operations on @offsets array? I worry about if
> > > '$#offsets-1' can always shrink array size correctly. pop/push anyway is
> > > some official way to do this.
> >
> > Assigning to $#offsets is defined to change the length of the array. But
> > I think you are correct that pop/push would be a more idiomatic way to
> > do this.
> >
> > It is a public holiday on Monday, but I'll take a look on Tuesday,
> > unless you fancy doing it in the meantime.
> Sure I will do this.
Done the test with pop/push alternative. Same effect as your original
patch.
Here is the trivial change on your patch.
diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 607c9ce..68101e0 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -415,7 +415,7 @@ sub setupboot_grub2 ($$$$) {
"$submenu->{StartLine}. ".
"Our want kern is $want_kernver");
$submenu=undef;
- $#offsets = $#offsets-1;
+ pop @offsets;
$offsets[$#offsets]++;
next;
}
@@ -451,7 +451,7 @@ sub setupboot_grub2 ($$$$) {
}
if (m/^submenu\s+[\'\"](.*)[\'\"].*\{\s*$/) {
$submenu={ StartLine =>$., MenuEntryPath => join ">",
@offsets };
- $offsets[$#offsets+1] = 0;
+ push @offsets,(0);
}
if (m/^\s*multiboot\s*(?:\/boot)?\/(xen\S+)/) {
die unless $entry;
> >
> > > @@ -446,11 +446,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 =>$.};
> > > > > + $submenu={ StartLine =>$., MenuEntryPath => join ">", @offsets };
> > > Here MenuEntryPath element for $submenu is actually debug purpose only,
> > > I think, may be can remove it. correct me if I'm wrong.
> >
> > It is just for debug, I think it would be OK to leave it though.
> >
> > Ian.
> >
>
>
^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-05-25 3:04 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <osstest-56922-mainreport@xen.org>
2015-05-22 9:57 ` [osstest test] 56922: regressions - FAIL Ian Campbell
2015-05-22 11:32 ` Ian Campbell
2015-05-22 12:30 ` Ian Campbell
2015-05-22 12:47 ` Robert Hu
2015-05-22 12:58 ` Ian Campbell
2015-05-22 13:42 ` Ian Campbell
2015-05-22 14:21 ` Ian Campbell
2015-05-23 3:35 ` Robert Hu
2015-05-23 6:46 ` Robert Hu
2015-05-23 6:58 ` Ian Campbell
2015-05-23 7:52 ` Robert Hu
2015-05-23 8:25 ` Ian Campbell
2015-05-23 8:28 ` Robert Hu
2015-05-23 8:31 ` Robert Hu
2015-05-23 8:53 ` Robert Hu
2015-05-24 13:53 ` Ian Campbell
2015-05-25 2:25 ` Robert Hu
2015-05-25 3:04 ` Robert Hu
2015-05-23 13:34 ` Robert Hu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.