[PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[Ian Campbell]

setupboot_grub2 now supports submenus, so we can reduce our delta vs
upstream a bit.

I started by extracting 20_linux_xen from
http://snapshot.debian.org/archive/debian/20130703T094657Z/pool/main/g/grub2/grub-common_1.99-27%2Bdeb7u2_amd64.deb
and then applying the patch at
http://savannah.gnu.org/file/grub.patch?file_id=32276 (the patch from
grub bug #42420 at http://savannah.gnu.org/bugs/?43420) and
reinstating the comment at the top of the file (modified to drop the
reference to the Debian bug.

This left me with some spurious changes:

    @@ -93,7 +93,7 @@ linux_entry ()
           if test ! -e "${xen_dirname}/${xenpolicy}" ; then
              return
           fi
    -      xen_args=`echo $xen_args flask=enforcing`
    +      xen_args=`echo $xen_args flask_enabled=1 flask_enforcing=1`
           if ${recovery} ; then
              title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and Linux %s (recovery mode)")"
           else
    @@ -137,7 +137,6 @@ EOF
            echo    '$message'
            module  ${rel_dirname}/${xenpolicy}
     EOF
    -  fi
       cat << EOF
     }
     EOF

I think these are bugs in the patch in the grub BTS, which were fixed
while iterating over the XSM series in osstest but didn't make it into
the upstream version, the fixes to those bugs are reverted byu the
above. So I have manually reverted them.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei.Lui@citrix.com
Cc: longtaox.pang@intel.com
---
v2: Posting as a standalone patch instead of in the nestedhvm series.

This should go in after "parsing grub which has 'submenu' primitive"
which is part of the nestedhvm test series from Longtao Pang. Needs
testing on top of that patch.

Wei, if you agree wrt those changes I'll update the bug, or perhaps
you want to?
---
 overlay/etc/grub.d/20_linux_xen | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/overlay/etc/grub.d/20_linux_xen b/overlay/etc/grub.d/20_linux_xen
index 5315e2a..aaead1b 100755
--- a/overlay/etc/grub.d/20_linux_xen
+++ b/overlay/etc/grub.d/20_linux_xen
@@ -1,7 +1,7 @@
 #! /bin/sh
 
 # Copied from the identically named file in grub-common 1.99-27+deb7u2.
-# This version fixed Debian bug #690538 and GRUB bug #43420.
+# This version fixes GRUB bug #43420.
 
 set -e
 
@@ -173,6 +173,7 @@ while [ "x${xen_list}" != "x" ] ; do
     xen_dirname=`dirname ${current_xen}`
     rel_xen_dirname=`make_system_path_relative_to_its_root $xen_dirname`
     xen_version=`echo $xen_basename | sed -e "s,.gz$,,g;s,^xen-,,g"`
+    echo "submenu \"Xen ${xen_version}\" {"
     while [ "x$list" != "x" ] ; do
 	linux=`version_find_latest $list`
 	echo "Found linux image: $linux" >&2
@@ -214,5 +215,6 @@ while [ "x${xen_list}" != "x" ] ; do
 
 	list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`
     done
+    echo "}"
     xen_list=`echo $xen_list | tr ' ' '\n' | grep -vx $current_xen | tr '\n' ' '`
 done
-- 
2.1.4

Re: [PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[Wei Liu]

On Wed, May 06, 2015 at 11:41:11AM +0100, Ian Campbell wrote:
> setupboot_grub2 now supports submenus, so we can reduce our delta vs
> upstream a bit.
> 
> I started by extracting 20_linux_xen from
> http://snapshot.debian.org/archive/debian/20130703T094657Z/pool/main/g/grub2/grub-common_1.99-27%2Bdeb7u2_amd64.deb
> and then applying the patch at
> http://savannah.gnu.org/file/grub.patch?file_id=32276 (the patch from
> grub bug #42420 at http://savannah.gnu.org/bugs/?43420) and
> reinstating the comment at the top of the file (modified to drop the
> reference to the Debian bug.
> 
> This left me with some spurious changes:
> 
>     @@ -93,7 +93,7 @@ linux_entry ()
>            if test ! -e "${xen_dirname}/${xenpolicy}" ; then
>               return
>            fi
>     -      xen_args=`echo $xen_args flask=enforcing`
>     +      xen_args=`echo $xen_args flask_enabled=1 flask_enforcing=1`

This is due to we changed flask command line option. The uploaded
version used the old options.

>            if ${recovery} ; then
>               title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and Linux %s (recovery mode)")"
>            else
>     @@ -137,7 +137,6 @@ EOF
>             echo    '$message'
>             module  ${rel_dirname}/${xenpolicy}
>      EOF
>     -  fi

I don't really remember the exact detail of this "fi". But if the
resulting file after reverting this "fi" looks correct to you then this
is OK.

>        cat << EOF
>      }
>      EOF
> 
> I think these are bugs in the patch in the grub BTS, which were fixed
> while iterating over the XSM series in osstest but didn't make it into
> the upstream version, the fixes to those bugs are reverted byu the
> above. So I have manually reverted them.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei.Lui@citrix.com

Wrong email address. I wonder how this email arrived in my inbox. :-)

> Cc: longtaox.pang@intel.com
> ---
> v2: Posting as a standalone patch instead of in the nestedhvm series.
> 
> This should go in after "parsing grub which has 'submenu' primitive"
> which is part of the nestedhvm test series from Longtao Pang. Needs
> testing on top of that patch.
> 
> Wei, if you agree wrt those changes I'll update the bug, or perhaps
> you want to?

Can you please do that since you have the most up to date patch at hand.
Thanks!

Wei.

> ---
>  overlay/etc/grub.d/20_linux_xen | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/overlay/etc/grub.d/20_linux_xen b/overlay/etc/grub.d/20_linux_xen
> index 5315e2a..aaead1b 100755
> --- a/overlay/etc/grub.d/20_linux_xen
> +++ b/overlay/etc/grub.d/20_linux_xen
> @@ -1,7 +1,7 @@
>  #! /bin/sh
>  
>  # Copied from the identically named file in grub-common 1.99-27+deb7u2.
> -# This version fixed Debian bug #690538 and GRUB bug #43420.
> +# This version fixes GRUB bug #43420.
>  
>  set -e
>  
> @@ -173,6 +173,7 @@ while [ "x${xen_list}" != "x" ] ; do
>      xen_dirname=`dirname ${current_xen}`
>      rel_xen_dirname=`make_system_path_relative_to_its_root $xen_dirname`
>      xen_version=`echo $xen_basename | sed -e "s,.gz$,,g;s,^xen-,,g"`
> +    echo "submenu \"Xen ${xen_version}\" {"
>      while [ "x$list" != "x" ] ; do
>  	linux=`version_find_latest $list`
>  	echo "Found linux image: $linux" >&2
> @@ -214,5 +215,6 @@ while [ "x${xen_list}" != "x" ] ; do
>  
>  	list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`
>      done
> +    echo "}"
>      xen_list=`echo $xen_list | tr ' ' '\n' | grep -vx $current_xen | tr '\n' ' '`
>  done
> -- 
> 2.1.4

Re: [PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[Ian Campbell]

On Wed, 2015-05-06 at 13:10 +0100, Wei Liu wrote:
> On Wed, May 06, 2015 at 11:41:11AM +0100, Ian Campbell wrote:
> > setupboot_grub2 now supports submenus, so we can reduce our delta vs
> > upstream a bit.
> > 
> > I started by extracting 20_linux_xen from
> > http://snapshot.debian.org/archive/debian/20130703T094657Z/pool/main/g/grub2/grub-common_1.99-27%2Bdeb7u2_amd64.deb
> > and then applying the patch at
> > http://savannah.gnu.org/file/grub.patch?file_id=32276 (the patch from
> > grub bug #42420 at http://savannah.gnu.org/bugs/?43420) and
> > reinstating the comment at the top of the file (modified to drop the
> > reference to the Debian bug.
> > 
> > This left me with some spurious changes:
> > 
> >     @@ -93,7 +93,7 @@ linux_entry ()
> >            if test ! -e "${xen_dirname}/${xenpolicy}" ; then
> >               return
> >            fi
> >     -      xen_args=`echo $xen_args flask=enforcing`
> >     +      xen_args=`echo $xen_args flask_enabled=1 flask_enforcing=1`
> 
> This is due to we changed flask command line option. The uploaded
> version used the old options.
> 
> >            if ${recovery} ; then
> >               title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and Linux %s (recovery mode)")"
> >            else
> >     @@ -137,7 +137,6 @@ EOF
> >             echo    '$message'
> >             module  ${rel_dirname}/${xenpolicy}
> >      EOF
> >     -  fi
> 
> I don't really remember the exact detail of this "fi". But if the
> resulting file after reverting this "fi" looks correct to you then this
> is OK.

The resulting file is correct, I think the patch in the bug report has
an unclosed if in it.

> >        cat << EOF
> >      }
> >      EOF
> > 
> > I think these are bugs in the patch in the grub BTS, which were fixed
> > while iterating over the XSM series in osstest but didn't make it into
> > the upstream version, the fixes to those bugs are reverted byu the
> > above. So I have manually reverted them.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Wei.Lui@citrix.com
> 
> Wrong email address. I wonder how this email arrived in my inbox. :-)

I also did "--cc wei" on my git send-email and your address is correct
in my ~/.gitaliases.

> 
> > Cc: longtaox.pang@intel.com
> > ---
> > v2: Posting as a standalone patch instead of in the nestedhvm series.
> > 
> > This should go in after "parsing grub which has 'submenu' primitive"
> > which is part of the nestedhvm test series from Longtao Pang. Needs
> > testing on top of that patch.
> > 
> > Wei, if you agree wrt those changes I'll update the bug, or perhaps
> > you want to?
> 
> Can you please do that since you have the most up to date patch at hand.
> Thanks!

Sure.

Re: [PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[Ian Campbell]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

On Wed, 2015-05-06 at 13:52 +0100, Ian Campbell wrote:
> > Can you please do that since you have the most up to date patch at hand.
> > Thanks!
> 
> Sure.

Actually, I don't have a login for savanah.gnu.org, since I suppose you
already do would you mind doing it? (attached again for convenience)

Ian.

[-- Attachment #2: grub.v2.patch --]
[-- Type: text/x-patch, Size: 2670 bytes --]

--- /home/ianc/tmp/x/etc/grub.d/20_linux_xen	2013-07-03 04:39:20.000000000 +0100
+++ overlay/etc/grub.d/20_linux_xen	2015-04-21 11:09:57.777812773 +0100
@@ -81,10 +85,27 @@
   recovery="$4"
   args="$5"
   xen_args="$6"
-  if ${recovery} ; then
-    title="$(gettext_quoted "%s, with Xen %s and Linux %s (recovery mode)")"
+  xsm="$7"
+  # If user wants to enable XSM support, make sure there's
+  # corresponding policy file.
+  if ${xsm} ; then
+      xenpolicy=`echo xenpolicy-$xen_version`
+      if test ! -e "${xen_dirname}/${xenpolicy}" ; then
+	  return
+      fi
+      xen_args=`echo $xen_args flask=enforcing`
+      if ${recovery} ; then
+	  title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and Linux %s (recovery mode)")"
+      else
+	  title="$(gettext_quoted "%s, with Xen %s (XSM enabled) and Linux %s")"
+      fi
   else
-    title="$(gettext_quoted "%s, with Xen %s and Linux %s")"
+      xenpolicy=""
+      if ${recovery} ; then
+	  title="$(gettext_quoted "%s, with Xen %s and Linux %s (recovery mode)")"
+      else
+	  title="$(gettext_quoted "%s, with Xen %s and Linux %s")"
+      fi
   fi
   printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${xen_version}" "${version}"
   if ! ${recovery} ; then
@@ -110,6 +131,13 @@
 	module	${rel_dirname}/${initrd}
 EOF
   fi
+  if test -n "${xenpolicy}" ; then
+    message="$(gettext_printf "Loading XSM policy ...")"
+    cat << EOF
+	echo	'$message'
+	module	${rel_dirname}/${xenpolicy}
+EOF
+  fi
   cat << EOF
 }
 EOF
@@ -133,7 +161,7 @@
 if [ "x${linux_list}" = "x" ] ; then
     exit 0
 fi
-xen_list=`for i in /boot/xen*; do
+xen_list=`for i in /boot/xen[-.]*; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
 prepare_boot_cache=
@@ -175,10 +203,14 @@
 	fi
 
 	linux_entry "${OS}" "${version}" "${xen_version}" false \
-	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}"
+	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}" false
+	linux_entry "${OS}" "${version}" "${xen_version}" false \
+	    "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" "${GRUB_CMDLINE_XEN} ${GRUB_CMDLINE_XEN_DEFAULT}" true
 	if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
 	    linux_entry "${OS}" "${version}" "${xen_version}" true \
-		"single ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN}"
+		"single ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN}" false
+	    linux_entry "${OS}" "${version}" "${xen_version}" true \
+		"single ${GRUB_CMDLINE_LINUX}" "${GRUB_CMDLINE_XEN}" true
 	fi
 
 	list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[Wei Liu]

On Wed, May 06, 2015 at 01:55:48PM +0100, Ian Campbell wrote:
> On Wed, 2015-05-06 at 13:52 +0100, Ian Campbell wrote:
> > > Can you please do that since you have the most up to date patch at hand.
> > > Thanks!
> > 
> > Sure.
> 
> Actually, I don't have a login for savanah.gnu.org, since I suppose you
> already do would you mind doing it? (attached again for convenience)
> 

No problem. I will just upload the attached version.

Wei.

Re: [PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[Ian Campbell]

On Wed, 2015-05-06 at 14:01 +0100, Wei Liu wrote:
> On Wed, May 06, 2015 at 01:55:48PM +0100, Ian Campbell wrote:
> > On Wed, 2015-05-06 at 13:52 +0100, Ian Campbell wrote:
> > > > Can you please do that since you have the most up to date patch at hand.
> > > > Thanks!
> > > 
> > > Sure.
> > 
> > Actually, I don't have a login for savanah.gnu.org, since I suppose you
> > already do would you mind doing it? (attached again for convenience)
> > 
> 
> No problem. I will just upload the attached version.

Thanks!

Re: [PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST v2] grub: remove patch to disable submenu from 20_linux_xen overlay"):
> --- /home/ianc/tmp/x/etc/grub.d/20_linux_xen	2013-07-03 04:39:20.000000000 +0100
> +++ overlay/etc/grub.d/20_linux_xen	2015-04-21 11:09:57.777812773 +0100

This looks good to me, apart from my comments on IRC as follows:

16:24 <Diziet> So the new diff seems to duplicate the if ${recovery}
               but [grub] upstream don't care about that ?
16:25 <Diziet> ISTM better would be if ${recovery}; then
               title="${title} (recovery..." but the gettext would
               need lifting out and urgh.

In any case, with my osstest hat on:

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

Ian.