[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#793786: initramfs-tools: fix the broken netconsole feature in initramfs-tools



Dear Ian,

Thanks for your feedback!

After reading your comment, I reviewed the whole procedure of loading
netconsole module during initramfs-tools booting, previously and after
my patch, I start to consider that original "modprobe netconsole
netconsole=$param" line in init file is merely a dirty hack. The real
fix is to load the netconsole module with param inside load_modules()
routine.

So I created v3 patchset, which looks more clean to me.
Of course patchset v2 & v3 both works well as I tested. So I let the
package maintainer to choose which is better to merge.


Dear Maintainer,

Enclosed is the updated patchset v3.

Changelog:
v1 => v2: for 0002 patch: commit log change
v2 => v3: for 0001 patch: change loading netconsole module timing from
separately in init script to load_modules() routine
v2 pros: limit the affected range only when netconsole is set; cons:
netconsole module would be load twice, so seems a dirty hack
v3 pros: clean code; cons: add a condition judging on each module
loading (overhead?)

Both v2 and v3 are fine to me.
Please simply pick one when to merge. Thank you!

Cheers,
Roger
From d2b8ed2613b482830f605813d9a1ba44af2db154 Mon Sep 17 00:00:00 2001
From: Roger Shimizu <rogershimizu@gmail.com>
Date: Sun, 2 Aug 2015 13:10:46 +0900
Subject: [PATCH v3 1/2] load netconsole with param in load_modules() routine

As v0.120, if netconsole is set in kernel command line options, that
module would be loaded twice, first in load_modules() routine WITHOUT
module param, and second in init script with param. However, the 2nd
one won't success becuase the module is already loaded in the 1st place.
Since load_modules() routine honors module param set in
/etc/initramfs-tools/modules, as a result, the netconsole param setting
in kernel command line options is just ignored.

So here comes the fix, which to load netconsole module with param in
load_mdoules() routine, only once.

The fix can well handle the following use case:
 - netconsole param is set in kernel command line options, and there's
only module name entry in /etc/initramfs-tools/modules
 - netconsole param is set in /etc/initramfs-tools/modules only
 - netconsole param is both set in kernel command line options and
/etc/initramfs-tools/modules, which the latter one will be used.

Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
---
 init              | 2 --
 scripts/functions | 6 +++++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/init b/init
index abf7f25..2bb4580 100755
--- a/init
+++ b/init
@@ -203,8 +203,6 @@ maybe_break modules
 load_modules
 [ "$quiet" != "y" ] && log_end_msg
 
-[ -n "${netconsole}" ] && modprobe netconsole netconsole="${netconsole}"
-
 maybe_break premount
 [ "$quiet" != "y" ] && log_begin_msg "Running /scripts/init-premount"
 run_scripts /scripts/init-premount
diff --git a/scripts/functions b/scripts/functions
index 8c1bb1f..e407e17 100644
--- a/scripts/functions
+++ b/scripts/functions
@@ -92,7 +92,11 @@ load_modules()
 			if [ "$com" = "#" ]; then
 				continue
 			fi
-			modprobe $m
+			if [ "$m" = "netconsole" -a -n "$netconsole" ]; then
+				modprobe netconsole netconsole="$netconsole"
+			else
+				modprobe $m
+			fi
 		done
 	fi
 }
-- 
2.1.4

From b759651c992ed77e7b11a5acc4641c13eac72e02 Mon Sep 17 00:00:00 2001
From: Roger Shimizu <rogershimizu@gmail.com>
Date: Sun, 2 Aug 2015 13:11:17 +0900
Subject: [PATCH v3 2/2] redirect debug info to netconsole

Redirect if debug and netconsole are both set in command line options.
The redirecting code is added in both debug and netconsole entry to
make the redirection as early as possible.
Debug info saving to file feature is still available if 'debug' is set
but 'netconsole' is not set in command line options.

Original idea was from Ian's post:
http://www.hellion.org.uk/blog/posts/debugging-initramfs-over-netconsole/

Signed-off-by: Roger Shimizu <rogershimizu@gmail.com>
Signed-off-by: Ian Campbell <ijc@debian.org>
---
 init | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/init b/init
index 2bb4580..0435a2d 100755
--- a/init
+++ b/init
@@ -146,7 +146,11 @@ for x in $(cat /proc/cmdline); do
 	debug)
 		debug=y
 		quiet=n
-		exec >/run/initramfs/initramfs.debug 2>&1
+		if [ -n "${netconsole}" ]; then
+			exec >/dev/kmsg 2>&1
+		else
+			exec >/run/initramfs/initramfs.debug 2>&1
+		fi
 		set -x
 		;;
 	debug=*)
@@ -165,6 +169,7 @@ for x in $(cat /proc/cmdline); do
 		;;
 	netconsole=*)
 		netconsole=${x#netconsole=}
+		[ "x$debug" = "xy" ] && exec >/dev/kmsg 2>&1
 		;;
 	BOOTIF=*)
 		BOOTIF=${x#BOOTIF=}
-- 
2.1.4


Reply to: