[PATCH OSSTEST 0/2] Uboot patches for XSM test case

[PATCH OSSTEST 0/2] Uboot patches for XSM test case

[Wei Liu]

Wei Liu (2):
  uboot: use "readlink -f"
  uboot: make flask loading address host property

 Osstest/Debian.pm | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH OSSTEST 1/2] uboot: use "readlink -f"

[Wei Liu]

If the path is not a symlink, readlink by default returns empty string.
Use "-f" to always return canonical path. This fixes the problem that
xenpolicy file not getting loaded (because it is not a symlink).

Also change another spot that calls readlink to get xen binary path in
case in the future we decide to not use symlink.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 Osstest/Debian.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 6784024..b829878 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -162,7 +162,7 @@ sub setupboot_uboot ($$$$) {
 	    $flask_commands = <<END;
 
 setenv flask_policy_addr_r 0x1200000
-flaskpolicy=`readlink /boot/$flaskpolicy`
+flaskpolicy=`readlink -f /boot/$flaskpolicy`
 ext2load scsi 0 \\\${flask_policy_addr_r} \$flaskpolicy
 fdt mknod /chosen module\@2
 fdt set /chosen/module\@2 compatible "xen,xsm-policy" "xen,multiboot-module"
@@ -227,7 +227,7 @@ fi
 cp -n /boot/boot.xen /boot/boot.xen.bak
 cp -n /boot/boot.scr.xen /boot/boot.scr.xen.bak
 
-xen=`readlink /boot/$xen`
+xen=`readlink -f /boot/$xen`
 
 cat >/boot/boot.xen <<EOF
 ${load_dtb}
-- 
1.9.1

Re: [PATCH OSSTEST 1/2] uboot: use "readlink -f"

[Ian Campbell]

On Tue, 2015-03-24 at 11:45 +0000, Wei Liu wrote:
> If the path is not a symlink, readlink by default returns empty string.
> Use "-f" to always return canonical path. This fixes the problem that
> xenpolicy file not getting loaded (because it is not a symlink).
> 
> Also change another spot that calls readlink to get xen binary path in
> case in the future we decide to not use symlink.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH OSSTEST 1/2] uboot: use "readlink -f"

[Ian Campbell]

On Tue, 2015-03-24 at 11:45 +0000, Wei Liu wrote:
> If the path is not a symlink, readlink by default returns empty string.
> Use "-f" to always return canonical path. This fixes the problem that
> xenpolicy file not getting loaded (because it is not a symlink).
> 
> Also change another spot that calls readlink to get xen binary path in
> case in the future we decide to not use symlink.

Unfortunately this has unexpectedly made things worse. See:
http://www.chiark.greenend.org.uk/~xensrcts/logs/36757/

The issue is that readlink -f returns the full absolute path, so given 
    $ ls /boot/xen*
    xen -> xen-X.Y
    xen-X.Y
Then:
    $ readlink /boot/xen
    xen-X.Y
    $ readlink -f /boot/xen
    /boot/xen-X.Y

This breaks because on these systems /boot is a separate partition, so
from the bootloaders point of view we need to be accessing /xen-X.Y
not /boot/xen-X.Y.

I don't know of an easy way to get the path of a file relative to the
filesystem root which contains it. So I would propose to drop the second
hunk and the second paragraph of the commit message and to make the
first hunk simply "flaskpolicy=$flaskpolicy" (rather than unescaping the
usages, this will make it easier to change in the future).

I think that resubmission will end up being to the new colo at this
point.

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  Osstest/Debian.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index 6784024..b829878 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -162,7 +162,7 @@ sub setupboot_uboot ($$$$) {
>  	    $flask_commands = <<END;
>  
>  setenv flask_policy_addr_r 0x1200000
> -flaskpolicy=`readlink /boot/$flaskpolicy`
> +flaskpolicy=`readlink -f /boot/$flaskpolicy`
>  ext2load scsi 0 \\\${flask_policy_addr_r} \$flaskpolicy
>  fdt mknod /chosen module\@2
>  fdt set /chosen/module\@2 compatible "xen,xsm-policy" "xen,multiboot-module"
> @@ -227,7 +227,7 @@ fi
>  cp -n /boot/boot.xen /boot/boot.xen.bak
>  cp -n /boot/boot.scr.xen /boot/boot.scr.xen.bak
>  
> -xen=`readlink /boot/$xen`
> +xen=`readlink -f /boot/$xen`
>  
>  cat >/boot/boot.xen <<EOF
>  ${load_dtb}

[PATCH OSSTEST v2 1/2] uboot: do not use readlink on xsm policy

[Ian Campbell]

The policy is not a symlink, so readlink will return nothing.

We cannot use readlink -f because that will return an absolute path
and we need a path relative to the filesystem root (in this case
/boot).

Keep flaskpolicy=$flaskpolicy as a shell variable rather than
unescaping the uses (so they are interpreted by Perl) to easy any
future changes.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
Replaces: uboot: use "readlink -f"
---
 Osstest/Debian.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 6784024..3a0993b 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -162,7 +162,7 @@ sub setupboot_uboot ($$$$) {
 	    $flask_commands = <<END;
 
 setenv flask_policy_addr_r 0x1200000
-flaskpolicy=`readlink /boot/$flaskpolicy`
+flaskpolicy=$flaskpolicy
 ext2load scsi 0 \\\${flask_policy_addr_r} \$flaskpolicy
 fdt mknod /chosen module\@2
 fdt set /chosen/module\@2 compatible "xen,xsm-policy" "xen,multiboot-module"
-- 
2.1.4

Re: [PATCH OSSTEST v2 1/2] uboot: do not use readlink on xsm policy

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v2 1/2] uboot: do not use readlink on xsm policy"):
> The policy is not a symlink, so readlink will return nothing.
> 
> We cannot use readlink -f because that will return an absolute path
> and we need a path relative to the filesystem root (in this case
> /boot).
> 
> Keep flaskpolicy=$flaskpolicy as a shell variable rather than
> unescaping the uses (so they are interpreted by Perl) to easy any
> future changes.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

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

[PATCH OSSTEST v2 2/2] uboot: make flask loading address host property

[Ian Campbell]

From: Wei Liu <wei.liu2@citrix.com>

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: [ijc] Rebase over change to preceding patch
---
 Osstest/Debian.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 3a0993b..3ca319c 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -158,10 +158,16 @@ sub setupboot_uboot ($$$$) {
 	    # Use the flaskpolicy from tools build job because we might
 	    # want to test cross releases policy compatibility.
 	    my $flaskpolicy = get_runvar('flaskpolicy',$r{buildjob});
+	    my $flask_policy_addr_r =
+		get_host_property($ho, 'UBootSetFlaskAddrR', undef);
+	    my $set_flask_addr_r =
+		$flask_policy_addr_r ?
+		"setenv flask_policy_addr_r $flask_policy_addr_r" : "";
+
 	    $xenhopt .= " flask=enforcing";
 	    $flask_commands = <<END;
 
-setenv flask_policy_addr_r 0x1200000
+${set_flask_addr_r}
 flaskpolicy=$flaskpolicy
 ext2load scsi 0 \\\${flask_policy_addr_r} \$flaskpolicy
 fdt mknod /chosen module\@2
-- 
2.1.4

Re: [PATCH OSSTEST 1/2] uboot: use "readlink -f"

[Atom2]

Am 27.03.15 um 10:28 schrieb Ian Campbell:
> Unfortunately this has unexpectedly made things worse. See:
> http://www.chiark.greenend.org.uk/~xensrcts/logs/36757/
>
> The issue is that readlink -f returns the full absolute path, so given
>      $ ls /boot/xen*
>      xen -> xen-X.Y
>      xen-X.Y
> Then:
>      $ readlink /boot/xen
>      xen-X.Y
>      $ readlink -f /boot/xen
>      /boot/xen-X.Y
>
> This breaks because on these systems /boot is a separate partition, so
> from the bootloaders point of view we need to be accessing /xen-X.Y
> not /boot/xen-X.Y.
>
> I don't know of an easy way to get the path of a file relative to the
> filesystem root which contains it. So I would propose to drop the second
> hunk and the second paragraph of the commit message and to make the
> first hunk simply "flaskpolicy=$flaskpolicy" (rather than unescaping the
> usages, this will make it easier to change in the future).
>
> I think that resubmission will end up being to the new colo at this
> point.
>
Hi Ian,
wouldn't the following snippet of code actually achieve what you want:

mylink="$(readlink -f /boot/xen)"
relpath="${mylink#$(stat -c "%m" "$mylink")}"

Regards Atom2

P.S. I know I still owe you the 64bit stat counter changes.
I haven't forgotten about it, but have not been able to dig into it yet.

Re: [PATCH OSSTEST 1/2] uboot: use "readlink -f"

[Ian Campbell]

On Fri, 2015-03-27 at 17:31 +0100, Atom2 wrote:
> Am 27.03.15 um 10:28 schrieb Ian Campbell:
> > Unfortunately this has unexpectedly made things worse. See:
> > http://www.chiark.greenend.org.uk/~xensrcts/logs/36757/
> >
> > The issue is that readlink -f returns the full absolute path, so given
> >      $ ls /boot/xen*
> >      xen -> xen-X.Y
> >      xen-X.Y
> > Then:
> >      $ readlink /boot/xen
> >      xen-X.Y
> >      $ readlink -f /boot/xen
> >      /boot/xen-X.Y
> >
> > This breaks because on these systems /boot is a separate partition, so
> > from the bootloaders point of view we need to be accessing /xen-X.Y
> > not /boot/xen-X.Y.
> >
> > I don't know of an easy way to get the path of a file relative to the
> > filesystem root which contains it. So I would propose to drop the second
> > hunk and the second paragraph of the commit message and to make the
> > first hunk simply "flaskpolicy=$flaskpolicy" (rather than unescaping the
> > usages, this will make it easier to change in the future).
> >
> > I think that resubmission will end up being to the new colo at this
> > point.
> >
> Hi Ian,
> wouldn't the following snippet of code actually achieve what you want:
> 
> mylink="$(readlink -f /boot/xen)"
> relpath="${mylink#$(stat -c "%m" "$mylink")}"

Modulo the quoting of the shell-within-Perl we are dealing with here I
think it might, yes. Thanks.


> P.S. I know I still owe you the 64bit stat counter changes.
> I haven't forgotten about it, but have not been able to dig into it yet.

No worries.

[PATCH OSSTEST 2/2] uboot: make flask loading address host property

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Please set host property 'UBootSetFlaskAddrR' to 0x1200000.
---
 Osstest/Debian.pm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index b829878..9f7705e 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -158,10 +158,16 @@ sub setupboot_uboot ($$$$) {
 	    # Use the flaskpolicy from tools build job because we might
 	    # want to test cross releases policy compatibility.
 	    my $flaskpolicy = get_runvar('flaskpolicy',$r{buildjob});
+	    my $flask_policy_addr_r =
+		get_host_property($ho, 'UBootSetFlaskAddrR', undef);
+	    my $set_flask_addr_r =
+		$flask_policy_addr_r ?
+		"setenv flask_policy_addr_r $flask_policy_addr_r" : "";
+
 	    $xenhopt .= " flask=enforcing";
 	    $flask_commands = <<END;
 
-setenv flask_policy_addr_r 0x1200000
+${set_flask_addr_r}
 flaskpolicy=`readlink -f /boot/$flaskpolicy`
 ext2load scsi 0 \\\${flask_policy_addr_r} \$flaskpolicy
 fdt mknod /chosen module\@2
-- 
1.9.1

Re: [PATCH OSSTEST 2/2] uboot: make flask loading address host property

[Ian Campbell]

On Tue, 2015-03-24 at 11:45 +0000, Wei Liu wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH OSSTEST 2/2] uboot: make flask loading address host property

[Ian Campbell]

On Tue, 2015-03-24 at 11:45 +0000, Wei Liu wrote:
> Please set host property 'UBootSetFlaskAddrR' to 0x1200000.

Sigh, I wasn't having a very good day yesterday, since I completely
failed to do this.

I've now done:
OSSTEST_CONFIG=production-config ./mg-hosts setprops  marilith-n4 marilith-n5 -- UBootSetFlaskAddrR 0x1200000

and will make a note to figure out and set good addresses for the ARM
boards in the new colo.

Since flight 36721 hasn't run any x86 build jobs yet and it has already
failed the test-armhf-armhf*-xsm jobs I've killed it so it gets another
bite of the cherry.

Ian.

> ---
>  Osstest/Debian.pm | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
> index b829878..9f7705e 100644
> --- a/Osstest/Debian.pm
> +++ b/Osstest/Debian.pm
> @@ -158,10 +158,16 @@ sub setupboot_uboot ($$$$) {
>  	    # Use the flaskpolicy from tools build job because we might
>  	    # want to test cross releases policy compatibility.
>  	    my $flaskpolicy = get_runvar('flaskpolicy',$r{buildjob});
> +	    my $flask_policy_addr_r =
> +		get_host_property($ho, 'UBootSetFlaskAddrR', undef);
> +	    my $set_flask_addr_r =
> +		$flask_policy_addr_r ?
> +		"setenv flask_policy_addr_r $flask_policy_addr_r" : "";
> +
>  	    $xenhopt .= " flask=enforcing";
>  	    $flask_commands = <<END;
>  
> -setenv flask_policy_addr_r 0x1200000
> +${set_flask_addr_r}
>  flaskpolicy=`readlink -f /boot/$flaskpolicy`
>  ext2load scsi 0 \\\${flask_policy_addr_r} \$flaskpolicy
>  fdt mknod /chosen module\@2

Re: [PATCH OSSTEST 2/2] uboot: make flask loading address host property

[Ian Campbell]

On Tue, 2015-03-24 at 11:45 +0000, Wei Liu wrote:
> Please set host property 'UBootSetFlaskAddrR' to 0x1200000.

For the new boards (both cubietruck and arndale) I think 0x41200000 is
the correct choice.

That's 2M after xen_addr_r (as 0x1200000 is on Marilith) and doesn't
clash with anything.

I've set those properties already in the new facility in anticipation of
this patch hitting that before too long (otherwise I'll inevitably
forget...).

Ian.