public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling
@ 2021-01-13 14:35 Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 1/6] kernel-install.eclass: Move common to kernel-install_install_all Michał Górny
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 14:35 UTC (permalink / raw
  To: gentoo-dev; +Cc: Michał Górny

Hi,

Here's another set of patches to dist-kernel eclasses.  This includes:

1. Improving postinst/config error handling to clearly indicate how
   to retry via 'emerge --config'.  The message now includes SLOT
   as well.

2. Adding support for uefi=yes in dracut, i.e. creating combined kernel
   + initramfs UEFI executable.  Requested by prometheanfire.

The final patch replaces (rebases) the last patch from the mount-boot
+ kernel-install series.  It requires the mount-boot.eclass patches
merged to function.


Michał Górny (6):
  kernel-install.eclass: Move common to kernel-install_install_all
  kernel-install.eclass: Update symlink before installing
  kernel-install.eclass: Include SLOT in --config suggestion
  kernel-install.eclass: Improve failed install error messages
  dist-kernel-utils.eclass: Support dracut's uefi=yes option
  kernel-install.eclass: Improve error message on /boot problems

 eclass/dist-kernel-utils.eclass | 35 ++++++++++++++--
 eclass/kernel-install.eclass    | 71 ++++++++++++++++++++-------------
 2 files changed, 76 insertions(+), 30 deletions(-)

-- 
2.30.0



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

* [gentoo-dev] [PATCH 1/6] kernel-install.eclass: Move common to kernel-install_install_all
  2021-01-13 14:35 [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling Michał Górny
@ 2021-01-13 14:35 ` Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 2/6] kernel-install.eclass: Update symlink before installing Michał Górny
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 14:35 UTC (permalink / raw
  To: gentoo-dev; +Cc: Michał Górny

Move the code shared by pkg_postinst() and pkg_config() to a new
kernel-install_install_all() function.  After all, the purpose
of pkg_config() is to repeat what pkg_postinst() does normally.
Keeping it in a common function improves maintainability.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/kernel-install.eclass | 62 +++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/eclass/kernel-install.eclass b/eclass/kernel-install.eclass
index 3b4af9e51c07..b1d48b6b3b5a 100644
--- a/eclass/kernel-install.eclass
+++ b/eclass/kernel-install.eclass
@@ -331,6 +331,34 @@ kernel-install_pkg_preinst() {
 	# (no-op)
 }
 
+# @FUNCTION: kernel-install_install_all
+# @USAGE: <ver>
+# @DESCRIPTION:
+# Build an initramfs for the kernel and install the kernel.  This is
+# called from pkg_postinst() and pkg_config().  <ver> is the full
+# kernel version.
+kernel-install_install_all() {
+	debug-print-function ${FUNCNAME} "${@}"
+
+	[[ ${#} -eq 1 ]] || die "${FUNCNAME}: invalid arguments"
+	local ver=${1}
+
+	mount-boot_pkg_preinst
+
+	local image_path=$(dist-kernel_get_image_path)
+	if use initramfs; then
+		# putting it alongside kernel image as 'initrd' makes
+		# kernel-install happier
+		dist-kernel_build_initramfs \
+			"${EROOT}/usr/src/linux-${ver}/${image_path%/*}/initrd" \
+			"${ver}"
+	fi
+
+	dist-kernel_install_kernel "${ver}" \
+		"${EROOT}/usr/src/linux-${ver}/${image_path}" \
+		"${EROOT}/usr/src/linux-${ver}/System.map"
+}
+
 # @FUNCTION: kernel-install_pkg_postinst
 # @DESCRIPTION:
 # Build an initramfs for the kernel, install it and update
@@ -338,22 +366,10 @@ kernel-install_pkg_preinst() {
 kernel-install_pkg_postinst() {
 	debug-print-function ${FUNCNAME} "${@}"
 
-	if [[ -z ${ROOT} ]]; then
-		mount-boot_pkg_preinst
-
-		local ver="${PV}${KV_LOCALVERSION}"
-		local image_path=$(dist-kernel_get_image_path)
-		if use initramfs; then
-			# putting it alongside kernel image as 'initrd' makes
-			# kernel-install happier
-			dist-kernel_build_initramfs \
-				"${EROOT}/usr/src/linux-${ver}/${image_path%/*}/initrd" \
-				"${ver}"
-		fi
+	local ver="${PV}${KV_LOCALVERSION}"
 
-		dist-kernel_install_kernel "${ver}" \
-			"${EROOT}/usr/src/linux-${ver}/${image_path}" \
-			"${EROOT}/usr/src/linux-${ver}/System.map"
+	if [[ -z ${ROOT} ]]; then
+		kernel-install_install_all "${ver}"
 	fi
 
 	kernel-install_update_symlink "${EROOT}/usr/src/linux" "${ver}"
@@ -391,21 +407,7 @@ kernel-install_pkg_postrm() {
 kernel-install_pkg_config() {
 	[[ -z ${ROOT} ]] || die "ROOT!=/ not supported currently"
 
-	mount-boot_pkg_preinst
-
-	local ver="${PV}${KV_LOCALVERSION}"
-	local image_path=$(dist-kernel_get_image_path)
-	if use initramfs; then
-		# putting it alongside kernel image as 'initrd' makes
-		# kernel-install happier
-		dist-kernel_build_initramfs \
-			"${EROOT}/usr/src/linux-${ver}/${image_path%/*}/initrd" \
-			"${ver}"
-	fi
-
-	dist-kernel_install_kernel "${ver}" \
-		"${EROOT}/usr/src/linux-${ver}/${image_path}" \
-		"${EROOT}/usr/src/linux-${ver}/System.map"
+	kernel-install_install_all "${PV}${KV_LOCALVERSION}"
 }
 
 _KERNEL_INSTALL_ECLASS=1
-- 
2.30.0



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

* [gentoo-dev] [PATCH 2/6] kernel-install.eclass: Update symlink before installing
  2021-01-13 14:35 [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 1/6] kernel-install.eclass: Move common to kernel-install_install_all Michał Górny
@ 2021-01-13 14:35 ` Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 3/6] kernel-install.eclass: Include SLOT in --config suggestion Michał Górny
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 14:35 UTC (permalink / raw
  To: gentoo-dev; +Cc: Michał Górny

Update symlink before actually installing the kernel.  This primarily
ensures that module rebuilds triggered by the upgrade will work
correctly even if postinst fails.  Besides, pkg_config() retries
installing the kernel but does not update the symlink.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/kernel-install.eclass | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/eclass/kernel-install.eclass b/eclass/kernel-install.eclass
index b1d48b6b3b5a..33ff69fc2528 100644
--- a/eclass/kernel-install.eclass
+++ b/eclass/kernel-install.eclass
@@ -367,12 +367,11 @@ kernel-install_pkg_postinst() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	local ver="${PV}${KV_LOCALVERSION}"
+	kernel-install_update_symlink "${EROOT}/usr/src/linux" "${ver}"
 
 	if [[ -z ${ROOT} ]]; then
 		kernel-install_install_all "${ver}"
 	fi
-
-	kernel-install_update_symlink "${EROOT}/usr/src/linux" "${ver}"
 }
 
 # @FUNCTION: kernel-install_pkg_prerm
-- 
2.30.0



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

* [gentoo-dev] [PATCH 3/6] kernel-install.eclass: Include SLOT in --config suggestion
  2021-01-13 14:35 [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 1/6] kernel-install.eclass: Move common to kernel-install_install_all Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 2/6] kernel-install.eclass: Update symlink before installing Michał Górny
@ 2021-01-13 14:35 ` Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages Michał Górny
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 14:35 UTC (permalink / raw
  To: gentoo-dev; +Cc: Michał Górny

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/kernel-install.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/kernel-install.eclass b/eclass/kernel-install.eclass
index 33ff69fc2528..cfd8ec0b7c58 100644
--- a/eclass/kernel-install.eclass
+++ b/eclass/kernel-install.eclass
@@ -308,7 +308,7 @@ kernel-install_pkg_pretend() {
 			elog "If you decide to install linux-firmware later, you can rebuild"
 			elog "the initramfs via issuing a command equivalent to:"
 			elog
-			elog "    emerge --config ${CATEGORY}/${PN}"
+			elog "    emerge --config ${CATEGORY}/${PN}:${SLOT}"
 		fi
 	fi
 }
-- 
2.30.0



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

* [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages
  2021-01-13 14:35 [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling Michał Górny
                   ` (2 preceding siblings ...)
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 3/6] kernel-install.eclass: Include SLOT in --config suggestion Michał Górny
@ 2021-01-13 14:35 ` Michał Górny
  2021-01-13 14:49   ` Ulrich Mueller
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option Michał Górny
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 6/6] kernel-install.eclass: Improve error message on /boot problems Michał Górny
  5 siblings, 1 reply; 17+ messages in thread
From: Michał Górny @ 2021-01-13 14:35 UTC (permalink / raw
  To: gentoo-dev; +Cc: Michał Górny

Support and use nonfatal to provide a detailed error message when kernel
postinst fails, in particular the correct 'emerge --config' command.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/dist-kernel-utils.eclass |  4 ++--
 eclass/kernel-install.eclass    | 42 +++++++++++++++++++++++----------
 2 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/eclass/dist-kernel-utils.eclass b/eclass/dist-kernel-utils.eclass
index d92642a25a0a..d65dc0924b40 100644
--- a/eclass/dist-kernel-utils.eclass
+++ b/eclass/dist-kernel-utils.eclass
@@ -43,7 +43,7 @@ dist-kernel_build_initramfs() {
 
 	ebegin "Building initramfs via dracut"
 	dracut --force "${output}" "${version}"
-	eend ${?} || die "Building initramfs failed"
+	eend ${?} || die -n "Building initramfs failed"
 }
 
 # @FUNCTION: dist-kernel_get_image_path
@@ -89,7 +89,7 @@ dist-kernel_install_kernel() {
 	# note: .config is taken relatively to System.map;
 	# initrd relatively to bzImage
 	installkernel "${version}" "${image}" "${map}"
-	eend ${?} || die "Installing the kernel failed"
+	eend ${?} || die -n "Installing the kernel failed"
 }
 
 # @FUNCTION: dist-kernel_reinstall_initramfs
diff --git a/eclass/kernel-install.eclass b/eclass/kernel-install.eclass
index cfd8ec0b7c58..f9b1834266dd 100644
--- a/eclass/kernel-install.eclass
+++ b/eclass/kernel-install.eclass
@@ -343,20 +343,36 @@ kernel-install_install_all() {
 	[[ ${#} -eq 1 ]] || die "${FUNCNAME}: invalid arguments"
 	local ver=${1}
 
-	mount-boot_pkg_preinst
-
-	local image_path=$(dist-kernel_get_image_path)
-	if use initramfs; then
-		# putting it alongside kernel image as 'initrd' makes
-		# kernel-install happier
-		dist-kernel_build_initramfs \
-			"${EROOT}/usr/src/linux-${ver}/${image_path%/*}/initrd" \
-			"${ver}"
-	fi
+	local success=
+	while :; do
+		mount-boot_pkg_preinst
+
+		local image_path=$(dist-kernel_get_image_path)
+		if use initramfs; then
+			# putting it alongside kernel image as 'initrd' makes
+			# kernel-install happier
+			nonfatal dist-kernel_build_initramfs \
+				"${EROOT}/usr/src/linux-${ver}/${image_path%/*}/initrd" \
+				"${ver}" || break
+		fi
 
-	dist-kernel_install_kernel "${ver}" \
-		"${EROOT}/usr/src/linux-${ver}/${image_path}" \
-		"${EROOT}/usr/src/linux-${ver}/System.map"
+		nonfatal dist-kernel_install_kernel "${ver}" \
+			"${EROOT}/usr/src/linux-${ver}/${image_path}" \
+			"${EROOT}/usr/src/linux-${ver}/System.map" || break
+
+		success=1
+		break
+	done
+
+	if [[ ! ${success} ]]; then
+		eerror
+		eerror "The kernel files were copied to disk successfully but the kernel"
+		eerror "was not deployed successfully.  Once you resolve the problems,"
+		eerror "please run the equivalent of the following command to try again:"
+		eerror
+		eerror "    emerge --config ${CATEGORY}/${PN}:${SLOT}"
+		die "Kernel install failed, please fix the problems and run emerge --config ${CATEGORY}/${PN}:${SLOT}"
+	fi
 }
 
 # @FUNCTION: kernel-install_pkg_postinst
-- 
2.30.0



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

* [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option
  2021-01-13 14:35 [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling Michał Górny
                   ` (3 preceding siblings ...)
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages Michał Górny
@ 2021-01-13 14:35 ` Michał Górny
  2021-01-13 17:47   ` Matthew Thode
  2021-01-13 19:01   ` Emily Rowlands
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 6/6] kernel-install.eclass: Improve error message on /boot problems Michał Górny
  5 siblings, 2 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 14:35 UTC (permalink / raw
  To: gentoo-dev; +Cc: Michał Górny

Support dracut's uefi=yes configuration option that creates a combined
UEFI stub, kernel and initramfs in a single UEFI executable.  If such
an output is detected, install it in place of the actual kernel image
and stub out the duplicate initrd to save space.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/dist-kernel-utils.eclass | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/eclass/dist-kernel-utils.eclass b/eclass/dist-kernel-utils.eclass
index d65dc0924b40..50ad15f8b1fd 100644
--- a/eclass/dist-kernel-utils.eclass
+++ b/eclass/dist-kernel-utils.eclass
@@ -41,8 +41,20 @@ dist-kernel_build_initramfs() {
 	local output=${1}
 	local version=${2}
 
+	local rel_image_path=$(dist-kernel_get_image_path)
+	local image=${output%/*}/${rel_image_path##*/}
+
+	local args=(
+		--force
+		# if uefi=yes is used, dracut needs to locate the kernel image
+		--kernel-image "${image}"
+
+		# positional arguments
+		"${output}" "${version}"
+	)
+
 	ebegin "Building initramfs via dracut"
-	dracut --force "${output}" "${version}"
+	dracut "${args[@]}"
 	eend ${?} || die -n "Building initramfs failed"
 }
 
@@ -85,6 +97,23 @@ dist-kernel_install_kernel() {
 	local image=${2}
 	local map=${3}
 
+	# if dracut is used in eufi=yes mode, initrd will actually
+	# be a combined kernel+initramfs UEFI executable.  we can easily
+	# recognize it by PE magic (vs cpio for a regular initramfs)
+	local initrd=${image%/*}/initrd
+	local magic
+	[[ -s ${initrd} ]] && read -n 2 magic < "${initrd}"
+	if [[ ${magic} == MZ ]]; then
+		einfo "Combined UEFI kernel+initramfs executable found"
+		# install the combined executable in place of kernel
+		image=${initrd}.uefi
+		mv "${initrd}" "${image}" || die
+		# put an empty file in place of initrd.  installing a duplicate
+		# file would waste disk space, and removing it entirely provokes
+		# kernel-install to regenerate it via dracut.
+		> "${initrd}"
+	fi
+
 	ebegin "Installing the kernel via installkernel"
 	# note: .config is taken relatively to System.map;
 	# initrd relatively to bzImage
-- 
2.30.0



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

* [gentoo-dev] [PATCH 6/6] kernel-install.eclass: Improve error message on /boot problems
  2021-01-13 14:35 [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling Michał Górny
                   ` (4 preceding siblings ...)
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option Michał Górny
@ 2021-01-13 14:35 ` Michał Górny
  5 siblings, 0 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 14:35 UTC (permalink / raw
  To: gentoo-dev; +Cc: Michał Górny

Use the newly-introduced mount-boot.eclass nonfatal support to amend
the error message with the instruction to run 'emerge --config' rather
than rebuild the whole kernel.

Signed-off-by: Michał Górny <mgorny@gentoo.org>
---
 eclass/kernel-install.eclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/eclass/kernel-install.eclass b/eclass/kernel-install.eclass
index f9b1834266dd..4263ec9f30a1 100644
--- a/eclass/kernel-install.eclass
+++ b/eclass/kernel-install.eclass
@@ -345,7 +345,7 @@ kernel-install_install_all() {
 
 	local success=
 	while :; do
-		mount-boot_pkg_preinst
+		nonfatal mount-boot_check_status || break
 
 		local image_path=$(dist-kernel_get_image_path)
 		if use initramfs; then
-- 
2.30.0



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

* Re: [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages Michał Górny
@ 2021-01-13 14:49   ` Ulrich Mueller
  2021-01-13 16:22     ` Michał Górny
  0 siblings, 1 reply; 17+ messages in thread
From: Ulrich Mueller @ 2021-01-13 14:49 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev

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

>>>>> On Wed, 13 Jan 2021, Michał Górny wrote:
> +	local success=
> +	while :; do
> +		mount-boot_pkg_preinst
> +
> +		local image_path=$(dist-kernel_get_image_path)
> +		if use initramfs; then
> +			# putting it alongside kernel image as 'initrd' makes
> +			# kernel-install happier
> +			nonfatal dist-kernel_build_initramfs \
> +				"${EROOT}/usr/src/linux-${ver}/${image_path%/*}/initrd" \
> +				"${ver}" || break
> +		fi
>  
> -	dist-kernel_install_kernel "${ver}" \
> -		"${EROOT}/usr/src/linux-${ver}/${image_path}" \
> -		"${EROOT}/usr/src/linux-${ver}/System.map"
> +		nonfatal dist-kernel_install_kernel "${ver}" \
> +			"${EROOT}/usr/src/linux-${ver}/${image_path}" \
> +			"${EROOT}/usr/src/linux-${ver}/System.map" || break
> +
> +		success=1
> +		break
> +	done

Looks like this loop can run only once, so it is redundant?

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

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

* Re: [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages
  2021-01-13 14:49   ` Ulrich Mueller
@ 2021-01-13 16:22     ` Michał Górny
  2021-01-13 20:53       ` Ulrich Mueller
  0 siblings, 1 reply; 17+ messages in thread
From: Michał Górny @ 2021-01-13 16:22 UTC (permalink / raw
  To: gentoo-dev

On Wed, 2021-01-13 at 15:49 +0100, Ulrich Mueller wrote:
> > > > > > On Wed, 13 Jan 2021, Michał Górny wrote:
> > +	local success=
> > +	while :; do
> > +		mount-boot_pkg_preinst
> > +
> > +		local image_path=$(dist-kernel_get_image_path)
> > +		if use initramfs; then
> > +			# putting it alongside kernel image as
> > 'initrd' makes
> > +			# kernel-install happier
> > +			nonfatal dist-kernel_build_initramfs \
> > +				"${EROOT}/usr/src/linux-
> > ${ver}/${image_path%/*}/initrd" \
> > +				"${ver}" || break
> > +		fi
> >  
> > -	dist-kernel_install_kernel "${ver}" \
> > -		"${EROOT}/usr/src/linux-${ver}/${image_path}" \
> > -		"${EROOT}/usr/src/linux-${ver}/System.map"
> > +		nonfatal dist-kernel_install_kernel "${ver}" \
> > +			"${EROOT}/usr/src/linux-
> > ${ver}/${image_path}" \
> > +			"${EROOT}/usr/src/linux-${ver}/System.map"
> > || break
> > +
> > +		success=1
> > +		break
> > +	done
> 
> Looks like this loop can run only once, so it is redundant?

It's the old C trick for convenient error handling.  Do you have any
other suggestion?  I suppose we could use a nested function if you think
that's nicer.

-- 
Best regards,
Michał Górny




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

* Re: [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option Michał Górny
@ 2021-01-13 17:47   ` Matthew Thode
  2021-01-13 17:58     ` Michał Górny
  2021-01-13 19:01   ` Emily Rowlands
  1 sibling, 1 reply; 17+ messages in thread
From: Matthew Thode @ 2021-01-13 17:47 UTC (permalink / raw
  To: gentoo-dev

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

On 21-01-13 15:35:15, Michał Górny wrote:
> Support dracut's uefi=yes configuration option that creates a combined
> UEFI stub, kernel and initramfs in a single UEFI executable.  If such
> an output is detected, install it in place of the actual kernel image
> and stub out the duplicate initrd to save space.
> 
> Signed-off-by: Michał Górny <mgorny@gentoo.org>
> ---
>  eclass/dist-kernel-utils.eclass | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/eclass/dist-kernel-utils.eclass b/eclass/dist-kernel-utils.eclass
> index d65dc0924b40..50ad15f8b1fd 100644
> --- a/eclass/dist-kernel-utils.eclass
> +++ b/eclass/dist-kernel-utils.eclass
> @@ -41,8 +41,20 @@ dist-kernel_build_initramfs() {
>  	local output=${1}
>  	local version=${2}
>  
> +	local rel_image_path=$(dist-kernel_get_image_path)
> +	local image=${output%/*}/${rel_image_path##*/}
> +
> +	local args=(
> +		--force
> +		# if uefi=yes is used, dracut needs to locate the kernel image
> +		--kernel-image "${image}"
> +
> +		# positional arguments
> +		"${output}" "${version}"
> +	)
> +
>  	ebegin "Building initramfs via dracut"
> -	dracut --force "${output}" "${version}"
> +	dracut "${args[@]}"
>  	eend ${?} || die -n "Building initramfs failed"
>  }
>  
> @@ -85,6 +97,23 @@ dist-kernel_install_kernel() {
>  	local image=${2}
>  	local map=${3}
>  
> +	# if dracut is used in eufi=yes mode, initrd will actually
> +	# be a combined kernel+initramfs UEFI executable.  we can easily
> +	# recognize it by PE magic (vs cpio for a regular initramfs)
> +	local initrd=${image%/*}/initrd
> +	local magic
> +	[[ -s ${initrd} ]] && read -n 2 magic < "${initrd}"
> +	if [[ ${magic} == MZ ]]; then

This magic header is matched by both the kernel and the new uefi
executible.  I think a better check would be...

if file "${image_path}" | grep -q 'EFI application'; then

> +		einfo "Combined UEFI kernel+initramfs executable found"
> +		# install the combined executable in place of kernel
> +		image=${initrd}.uefi
> +		mv "${initrd}" "${image}" || die
> +		# put an empty file in place of initrd.  installing a duplicate
> +		# file would waste disk space, and removing it entirely provokes
> +		# kernel-install to regenerate it via dracut.
> +		> "${initrd}"
> +	fi
> +
>  	ebegin "Installing the kernel via installkernel"
>  	# note: .config is taken relatively to System.map;
>  	# initrd relatively to bzImage
> -- 
> 2.30.0

I want to make sure you are not overwriting the original kernel image,
if you do then subsiquent --config runs will include the kernel image,
making the final image grow.

In order to avoid that I did some file juggling, but it looks like it's
cleaner here.

https://gist.github.com/prometheanfire/aeffa1c3f92d3d2af312b3b6915051fb
'worked' but was kinda unclean.


-- 
Matthew Thode (prometheanfire)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option
  2021-01-13 17:47   ` Matthew Thode
@ 2021-01-13 17:58     ` Michał Górny
  0 siblings, 0 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 17:58 UTC (permalink / raw
  To: gentoo-dev

On Wed, 2021-01-13 at 11:47 -0600, Matthew Thode wrote:
> On 21-01-13 15:35:15, Michał Górny wrote:
> > Support dracut's uefi=yes configuration option that creates a combined
> > UEFI stub, kernel and initramfs in a single UEFI executable.  If such
> > an output is detected, install it in place of the actual kernel image
> > and stub out the duplicate initrd to save space.
> > 
> > Signed-off-by: Michał Górny <mgorny@gentoo.org>
> > ---
> >  eclass/dist-kernel-utils.eclass | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/eclass/dist-kernel-utils.eclass b/eclass/dist-kernel-utils.eclass
> > index d65dc0924b40..50ad15f8b1fd 100644
> > --- a/eclass/dist-kernel-utils.eclass
> > +++ b/eclass/dist-kernel-utils.eclass
> > @@ -41,8 +41,20 @@ dist-kernel_build_initramfs() {
> >  	local output=${1}
> >  	local version=${2}
> >  
> > 
> > 
> > 
> > +	local rel_image_path=$(dist-kernel_get_image_path)
> > +	local image=${output%/*}/${rel_image_path##*/}
> > +
> > +	local args=(
> > +		--force
> > +		# if uefi=yes is used, dracut needs to locate the kernel image
> > +		--kernel-image "${image}"
> > +
> > +		# positional arguments
> > +		"${output}" "${version}"
> > +	)
> > +
> >  	ebegin "Building initramfs via dracut"
> > -	dracut --force "${output}" "${version}"
> > +	dracut "${args[@]}"
> >  	eend ${?} || die -n "Building initramfs failed"
> >  }
> >  
> > 
> > 
> > 
> > @@ -85,6 +97,23 @@ dist-kernel_install_kernel() {
> >  	local image=${2}
> >  	local map=${3}
> >  
> > 
> > 
> > 
> > +	# if dracut is used in eufi=yes mode, initrd will actually
> > +	# be a combined kernel+initramfs UEFI executable.  we can easily
> > +	# recognize it by PE magic (vs cpio for a regular initramfs)
> > +	local initrd=${image%/*}/initrd
> > +	local magic
> > +	[[ -s ${initrd} ]] && read -n 2 magic < "${initrd}"
> > +	if [[ ${magic} == MZ ]]; then
> 
> This magic header is matched by both the kernel and the new uefi
> executible.  I think a better check would be...
> 
> if file "${image_path}" | grep -q 'EFI application'; then

...but we're not testing image but initrd which can't be the kernel
otherwise.

> 
> > +		einfo "Combined UEFI kernel+initramfs executable found"
> > +		# install the combined executable in place of kernel
> > +		image=${initrd}.uefi
> > +		mv "${initrd}" "${image}" || die
> > +		# put an empty file in place of initrd.  installing a duplicate
> > +		# file would waste disk space, and removing it entirely provokes
> > +		# kernel-install to regenerate it via dracut.
> > +		> "${initrd}"
> > +	fi
> > +
> >  	ebegin "Installing the kernel via installkernel"
> >  	# note: .config is taken relatively to System.map;
> >  	# initrd relatively to bzImage
> > -- 
> > 2.30.0
> 
> I want to make sure you are not overwriting the original kernel image,
> if you do then subsiquent --config runs will include the kernel image,
> making the final image grow.
> 
> In order to avoid that I did some file juggling, but it looks like it's
> cleaner here.

I still don't understand why it has been overwriting the kernel for you.

> 
> https://gist.github.com/prometheanfire/aeffa1c3f92d3d2af312b3b6915051fb
> 'worked' but was kinda unclean.
> 
> 

-- 
Best regards,
Michał Górny




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

* Re: [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option
  2021-01-13 14:35 ` [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option Michał Górny
  2021-01-13 17:47   ` Matthew Thode
@ 2021-01-13 19:01   ` Emily Rowlands
  2021-01-13 21:44     ` Michał Górny
  1 sibling, 1 reply; 17+ messages in thread
From: Emily Rowlands @ 2021-01-13 19:01 UTC (permalink / raw
  To: gentoo-dev; +Cc: emily.rowlands+gentoo

On 13/01/2021 14:35, Michał Górny wrote:
> @@ -85,6 +97,23 @@ dist-kernel_install_kernel() {
>  	local image=${2}
>  	local map=${3}
>  
> +	# if dracut is used in eufi=yes mode, initrd will actually

Minor typo: this should be "uefi=yes", rather than eufi.

Emily Rowlands


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

* Re: [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages
  2021-01-13 16:22     ` Michał Górny
@ 2021-01-13 20:53       ` Ulrich Mueller
  2021-01-13 21:23         ` Michał Górny
  0 siblings, 1 reply; 17+ messages in thread
From: Ulrich Mueller @ 2021-01-13 20:53 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev

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

>>>>> On Wed, 13 Jan 2021, Michał Górny wrote:

>> Looks like this loop can run only once, so it is redundant?

> It's the old C trick for convenient error handling.

Newfangled contraptions. What's wrong with goto? :)

> Do you have any other suggestion?  I suppose we could use a nested
> function if you think that's nicer.

No, it's fine. Maybe an explanatory comment would be helpful, because I
think that code isn't obvious.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

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

* Re: [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages
  2021-01-13 20:53       ` Ulrich Mueller
@ 2021-01-13 21:23         ` Michał Górny
  2021-01-14  9:22           ` Ulrich Mueller
  0 siblings, 1 reply; 17+ messages in thread
From: Michał Górny @ 2021-01-13 21:23 UTC (permalink / raw
  To: gentoo-dev

On Wed, 2021-01-13 at 21:53 +0100, Ulrich Mueller wrote:
> > > > > > On Wed, 13 Jan 2021, Michał Górny wrote:
> 
> > > Looks like this loop can run only once, so it is redundant?
> 
> > It's the old C trick for convenient error handling.
> 
> Newfangled contraptions. What's wrong with goto? :)
> 
> > Do you have any other suggestion?  I suppose we could use a nested
> > function if you think that's nicer.
> 
> No, it's fine. Maybe an explanatory comment would be helpful, because
> I
> think that code isn't obvious.

.

  # loop for the purpose of allowing error handling via 'break'

Does that sound explanatory enough?

-- 
Best regards,
Michał Górny




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

* Re: [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option
  2021-01-13 19:01   ` Emily Rowlands
@ 2021-01-13 21:44     ` Michał Górny
  0 siblings, 0 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-13 21:44 UTC (permalink / raw
  To: gentoo-dev; +Cc: emily.rowlands+gentoo

On Wed, 2021-01-13 at 19:01 +0000, Emily Rowlands wrote:
> On 13/01/2021 14:35, Michał Górny wrote:
> > @@ -85,6 +97,23 @@ dist-kernel_install_kernel() {
> >  	local image=${2}
> >  	local map=${3}
> >  
> > 
> > +	# if dracut is used in eufi=yes mode, initrd will actually
> 
> Minor typo: this should be "uefi=yes", rather than eufi.

Thanks, fixed.

-- 
Best regards,
Michał Górny




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

* Re: [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages
  2021-01-13 21:23         ` Michał Górny
@ 2021-01-14  9:22           ` Ulrich Mueller
  2021-01-14  9:26             ` Michał Górny
  0 siblings, 1 reply; 17+ messages in thread
From: Ulrich Mueller @ 2021-01-14  9:22 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev

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

>>>>> On Wed, 13 Jan 2021, Michał Górny wrote:

>   # loop for the purpose of allowing error handling via 'break'

> Does that sound explanatory enough?

"Not an actual loop but allows error handling with 'break'"

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 507 bytes --]

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

* Re: [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages
  2021-01-14  9:22           ` Ulrich Mueller
@ 2021-01-14  9:26             ` Michał Górny
  0 siblings, 0 replies; 17+ messages in thread
From: Michał Górny @ 2021-01-14  9:26 UTC (permalink / raw
  To: gentoo-dev

On Thu, 2021-01-14 at 10:22 +0100, Ulrich Mueller wrote:
> > > > > > On Wed, 13 Jan 2021, Michał Górny wrote:
> 
> >   # loop for the purpose of allowing error handling via 'break'
> 
> > Does that sound explanatory enough?
> 
> "Not an actual loop but allows error handling with 'break'"

Thanks.  Updated.

-- 
Best regards,
Michał Górny




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

end of thread, other threads:[~2021-01-14  9:26 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-13 14:35 [gentoo-dev] [PATCH 0/6] kernel-install.eclass: dracut uefi=yes support + improved error handling Michał Górny
2021-01-13 14:35 ` [gentoo-dev] [PATCH 1/6] kernel-install.eclass: Move common to kernel-install_install_all Michał Górny
2021-01-13 14:35 ` [gentoo-dev] [PATCH 2/6] kernel-install.eclass: Update symlink before installing Michał Górny
2021-01-13 14:35 ` [gentoo-dev] [PATCH 3/6] kernel-install.eclass: Include SLOT in --config suggestion Michał Górny
2021-01-13 14:35 ` [gentoo-dev] [PATCH 4/6] kernel-install.eclass: Improve failed install error messages Michał Górny
2021-01-13 14:49   ` Ulrich Mueller
2021-01-13 16:22     ` Michał Górny
2021-01-13 20:53       ` Ulrich Mueller
2021-01-13 21:23         ` Michał Górny
2021-01-14  9:22           ` Ulrich Mueller
2021-01-14  9:26             ` Michał Górny
2021-01-13 14:35 ` [gentoo-dev] [PATCH 5/6] dist-kernel-utils.eclass: Support dracut's uefi=yes option Michał Górny
2021-01-13 17:47   ` Matthew Thode
2021-01-13 17:58     ` Michał Górny
2021-01-13 19:01   ` Emily Rowlands
2021-01-13 21:44     ` Michał Górny
2021-01-13 14:35 ` [gentoo-dev] [PATCH 6/6] kernel-install.eclass: Improve error message on /boot problems Michał Górny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox