grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] Fix ARM multiboot2 breaking Fedora.
@ 2017-08-29 20:40 Konrad Rzeszutek Wilk
  2017-08-29 20:40 ` [PATCH v2 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64 Konrad Rzeszutek Wilk
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-08-29 20:40 UTC (permalink / raw)
  To: daniel.kiper, xen-devel, grub-devel

Since v1 [http://lists.gnu.org/archive/html/grub-devel/2017-08/msg00073.html]
 - Fixed up patch with failing invocation,
 - Redid patch #2 per Daniel's instructions.


Hey,

The first patch:
 [PATCH 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command

is a fix discovered on Fedora rawhide where I was surprised to see that
grub2-mkconfig would not create a configuration file anymore.

The second patch has been posted in the past
(https://lists.xen.org/archives/html/xen-devel/2017-03/txtCeHTNmz1hZ.txt)

 [PATCH 2/2] Use grub-file to figure out whether multiboot2 should be

and just allows us to multiboot2 instead of multiboot if the binary
supports it.


 util/grub.d/20_linux_xen.in | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)


Konrad Rzeszutek Wilk (2):
      Fix util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64
      Use grub-file to figure out whether multiboot2 should be used for Xen.gz



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64
  2017-08-29 20:40 [PATCH v2] Fix ARM multiboot2 breaking Fedora Konrad Rzeszutek Wilk
@ 2017-08-29 20:40 ` Konrad Rzeszutek Wilk
  2017-08-29 20:40 ` [PATCH v2 2/2] Use grub-file to figure out whether multiboot2 should be used for Xen.gz Konrad Rzeszutek Wilk
  2017-08-30 10:26 ` [PATCH v2] Fix ARM multiboot2 breaking Fedora Daniel Kiper
  2 siblings, 0 replies; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-08-29 20:40 UTC (permalink / raw)
  To: daniel.kiper, xen-devel, grub-devel; +Cc: Konrad Rzeszutek Wilk, Fu Wei

Commit d33045ce7ffcb7c1e4a60c14d5ca64b36e3c5abe introduced
the support for this, but it does not work under x86 (as it stops
20_linux_xen from running).

The 20_linux_xen is run under a shell and any exits from within it:

(For example on x86):
+ /usr/bin/grub2-file --is-arm64-efi /boot/xen-4.9.0.gz
[root@tst063 grub]# echo $?
1

will result in 20_linux_xen exiting without continuing
and also causing grub2-mkconfig to stop processing.

As in:

 [root@tst063 grub]# ./grub-mkconfig | tail
 Generating grub configuration file ...
 Found linux image: /boot/vmlinuz-4.13.0-0.rc5.git1.1.fc27.x86_64
 Found initrd image: /boot/initramfs-4.13.0-0.rc5.git1.1.fc27.x86_64.img
 Found linux image: /boot/vmlinuz-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2
 Found initrd image: /boot/initramfs-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2.img
 		echo	'Loading Linux 0-rescue-ec082ee24aea41b9b16aca52a6d10cc2 ...'
 		linux	/vmlinuz-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2 root=/dev/mapper/fedora_tst063-root ro single
 		echo	'Loading initial ramdisk ...'
 		initrd	/initramfs-0-rescue-ec082ee24aea41b9b16aca52a6d10cc2.img
 	}
 }

 ### END /usr/local/etc/grub.d/10_linux ###

 ### BEGIN /usr/local/etc/grub.d/20_linux_xen ###

 root@tst063 grub]#

And no more.

This patch wraps the invocation of grub-file to be a in subshell
and to process the return value in a conditional. That fixes
the issue.

RH-BZ 1486002: grub2-mkconfig does not work if xen.gz is installed.
CC: Fu Wei <fu.wei@linaro.org>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v1: Initial patch
v2: Add the failing output.
---
 util/grub.d/20_linux_xen.in | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index c002fc9..083bcef 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -206,13 +206,12 @@ while [ "x${xen_list}" != "x" ] ; do
     if [ "x$is_top_level" != xtrue ]; then
 	echo "	submenu '$(gettext_printf "Xen hypervisor, version %s" "${xen_version}" | grub_quote)' \$menuentry_id_option 'xen-hypervisor-$xen_version-$boot_device_id' {"
     fi
-    $grub_file --is-arm64-efi $current_xen
-    if [ $? -ne 0 ]; then
-	xen_loader="multiboot"
-	module_loader="module"
-    else
+    if ($grub_file --is-arm64-efi $current_xen); then
 	xen_loader="xen_hypervisor"
 	module_loader="xen_module"
+    else
+	xen_loader="multiboot"
+	module_loader="module"
     fi
     while [ "x$list" != "x" ] ; do
 	linux=`version_find_latest $list`
-- 
2.1.4



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] Use grub-file to figure out whether multiboot2 should be used for Xen.gz
  2017-08-29 20:40 [PATCH v2] Fix ARM multiboot2 breaking Fedora Konrad Rzeszutek Wilk
  2017-08-29 20:40 ` [PATCH v2 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64 Konrad Rzeszutek Wilk
@ 2017-08-29 20:40 ` Konrad Rzeszutek Wilk
  2017-08-30 10:26 ` [PATCH v2] Fix ARM multiboot2 breaking Fedora Daniel Kiper
  2 siblings, 0 replies; 5+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-08-29 20:40 UTC (permalink / raw)
  To: daniel.kiper, xen-devel, grub-devel; +Cc: Konrad Rzeszutek Wilk

The multiboot2 is much more preferable than multiboot. Especiall
if booting under EFI where multiboot does not have the functionality
to pass ImageHandler.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v2: Rebase on top of  d33045ce7ffcb7c1e4a60c14d5ca64b36e3c5abe
v3: Add 'else' in the conditional.
    Use a tab and four spaces instead of two tabs.
---
 util/grub.d/20_linux_xen.in | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index 083bcef..0cb0f4e 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -210,8 +210,13 @@ while [ "x${xen_list}" != "x" ] ; do
 	xen_loader="xen_hypervisor"
 	module_loader="xen_module"
     else
-	xen_loader="multiboot"
-	module_loader="module"
+	if ($grub_file --is-x86-multiboot2 $current_xen); then
+	    xen_loader="multiboot2"
+	    module_loader="module2"
+	else
+	    xen_loader="multiboot"
+	    module_loader="module"
+        fi
     fi
     while [ "x$list" != "x" ] ; do
 	linux=`version_find_latest $list`
-- 
2.1.4



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Fix ARM multiboot2 breaking Fedora.
  2017-08-29 20:40 [PATCH v2] Fix ARM multiboot2 breaking Fedora Konrad Rzeszutek Wilk
  2017-08-29 20:40 ` [PATCH v2 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64 Konrad Rzeszutek Wilk
  2017-08-29 20:40 ` [PATCH v2 2/2] Use grub-file to figure out whether multiboot2 should be used for Xen.gz Konrad Rzeszutek Wilk
@ 2017-08-30 10:26 ` Daniel Kiper
  2017-09-07 21:47   ` Daniel Kiper
  2 siblings, 1 reply; 5+ messages in thread
From: Daniel Kiper @ 2017-08-30 10:26 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, grub-devel

On Tue, Aug 29, 2017 at 04:40:51PM -0400, Konrad Rzeszutek Wilk wrote:
> Since v1 [http://lists.gnu.org/archive/html/grub-devel/2017-08/msg00073.html]
>  - Fixed up patch with failing invocation,
>  - Redid patch #2 per Daniel's instructions.
>
>
> Hey,
>
> The first patch:
>  [PATCH 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command
>
> is a fix discovered on Fedora rawhide where I was surprised to see that
> grub2-mkconfig would not create a configuration file anymore.
>
> The second patch has been posted in the past
> (https://lists.xen.org/archives/html/xen-devel/2017-03/txtCeHTNmz1hZ.txt)
>
>  [PATCH 2/2] Use grub-file to figure out whether multiboot2 should be
>
> and just allows us to multiboot2 instead of multiboot if the binary
> supports it.

LGMT. If there are no objections in a week or so I will apply both of them.

Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] Fix ARM multiboot2 breaking Fedora.
  2017-08-30 10:26 ` [PATCH v2] Fix ARM multiboot2 breaking Fedora Daniel Kiper
@ 2017-09-07 21:47   ` Daniel Kiper
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Kiper @ 2017-09-07 21:47 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, grub-devel

On Wed, Aug 30, 2017 at 12:26:28PM +0200, Daniel Kiper wrote:
> On Tue, Aug 29, 2017 at 04:40:51PM -0400, Konrad Rzeszutek Wilk wrote:
> > Since v1 [http://lists.gnu.org/archive/html/grub-devel/2017-08/msg00073.html]
> >  - Fixed up patch with failing invocation,
> >  - Redid patch #2 per Daniel's instructions.
> >
> >
> > Hey,
> >
> > The first patch:
> >  [PATCH 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command
> >
> > is a fix discovered on Fedora rawhide where I was surprised to see that
> > grub2-mkconfig would not create a configuration file anymore.
> >
> > The second patch has been posted in the past
> > (https://lists.xen.org/archives/html/xen-devel/2017-03/txtCeHTNmz1hZ.txt)
> >
> >  [PATCH 2/2] Use grub-file to figure out whether multiboot2 should be
> >
> > and just allows us to multiboot2 instead of multiboot if the binary
> > supports it.
>
> LGMT. If there are no objections in a week or so I will apply both of them.

Applied!

Thanks,

Daniel


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-09-07 21:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-29 20:40 [PATCH v2] Fix ARM multiboot2 breaking Fedora Konrad Rzeszutek Wilk
2017-08-29 20:40 ` [PATCH v2 1/2] Fix util/grub.d/20_linux_xen.in: Add xen_boot command support for aarch64 Konrad Rzeszutek Wilk
2017-08-29 20:40 ` [PATCH v2 2/2] Use grub-file to figure out whether multiboot2 should be used for Xen.gz Konrad Rzeszutek Wilk
2017-08-30 10:26 ` [PATCH v2] Fix ARM multiboot2 breaking Fedora Daniel Kiper
2017-09-07 21:47   ` Daniel Kiper

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).