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

Re: d-i wheezy rc2 preparation, take 2 [rfc patch]



On Mon, Apr 08, 2013 at 02:51:07PM +0200, Cyril Brulebois wrote:
> Vincent McIntyre <vincent.mcintyre@csiro.au> (08/04/2013):
> > May I remind people about #696877 (when installing from USB stick,
> >  grub writes the MBR to the wrong device - the USB stick).
> > 
> > I think this problem has been around in some form or other in squeeze
> > (eg #666974) and possibly before and I really think it needs fixing.
> > It's going to need code changes (hopefully small) and possibly
> > translation changes.
> > 
> > I have looked into this myself but the grub-installer code is too
> > complex for me to debug, it needs someone more familiar with all the
> > corner cases in the code.
> 
> I very much know about this one, but thanks for making sure.
> 
> Yet, we lack the code currently, and nobody has stepped in to provide
> it, meaning rc2 will have an unfixed grub-installer.
> 
> In an ideal world, someone would submit a fix before r0, we could
> think about shipping an rc3 with it, and profit for r0.
> 

Hokay. Below is a very rough patch.
The basic problem seems to be the assignment in the 'state=1' condition:
   bootdev="$default_bootdev"
   break
because sometimes, default_bootdev is guessed incorrectly and at other
times the user has preseeded a value that gets ignored by the code above.

My approach is to add a new question (choose_bootdev) and provide a list
of disks to select from. If there is doubt about whether default_bootdev
has the right value, the question is asked. Just disk devices are shown;
if the user wants to install to a partition or floppy etc they should
enter it manually. The question is asked within a function (select_bootdev).
If that function returns an empty string or a nonexisting device,
the grub-installer should ask grub-installer/bootdev as a fallback.

I think it will run but I can't work out how to get d-i to accept
the new question into questions.dat. I could use a little help there.

Cheers
Vince

diff --git a/debian/grub-installer.templates b/debian/grub-installer.templates
index 888a656..e43d8ed 100644
--- a/debian/grub-installer.templates
+++ b/debian/grub-installer.templates
@@ -86,6 +86,20 @@ _Description: Device for boot loader installation:
     drive;
   - "/dev/fd0" will install GRUB to a floppy.
 
+Template: grub-installer/choose_bootdev
+Type: select
+# :sl2:
+_Description: Select disk for boot loader installation:
+ You need to make the newly installed system bootable, by installing
+ the GRUB boot loader on a bootable device. The usual way to do this is to
+ install GRUB on the master boot record of your first hard drive. If you
+ prefer, you can install GRUB elsewhere on the drive, or to another drive,
+ or even to a floppy.
+ .
+ Select a disk from the list below. GRUB will be installed to the master
+ boot record of that disk. If you want to install GRUB somewhere other than
+ the master boot record of the listed disks, select 'Enter device manually'.
+
 Template: grub-installer/password
 Type: password
 # :sl2:
diff --git a/grub-installer b/grub-installer
index f01eda1..9a9ad7b 100755
--- a/grub-installer
+++ b/grub-installer
@@ -217,6 +217,37 @@ devices_to_ids()
 	echo "$ids"
 }
 
+
+# Produce a comma-separated list of '/dev/X (id)' strings suitable for debconf.
+# If you give an argument, that <device> should be the only one returned.
+# Examples:
+#  echo $(device_list)
+#  /dev/sda (ata-SAMSUNG_SSD_PM810_2.5__7mm_128GB_S0NRNEABC02304), /dev/sdb (ata-WDC_WD2002FAEX-007BA0_WD-WMAY05111513)
+#  echo $(device_list /dev/sdb)
+#  /dev/sdb (ata-WDC_WD2002FAEX-007BA0_WD-WMAY05111513)
+device_list()
+{
+	local arg id path dev disk
+	arg="$1"
+
+	[ -d /dev/disk/by-id ] || return
+	# /dev/disk/by-id has multiple links for the same physical disk.
+	# Sorting the output on only one column gives just one disk,path pair
+	# for each of the physical disks
+	cached_devices="$(
+		for path in /dev/disk/by-id/*; do
+			[ -e "$path" ] || continue
+			id="$(echo "$path" | sed -e 's+^.*/++')"
+			dev="$(readlink -f "$path")"
+			disk="$(device_to_disk "$dev")"
+			[ -n "$arg" ] && dev="$arg"
+			[ "$dev" = "$disk" ] && printf '%s (%s)\n' "$disk" "$id"
+		done | \
+		sort -k1,1 -s -u | awk '{printf "%s, ", $0}' | sed -e 's/ ,$//'
+	)"
+	echo "$cached_devices"
+}
+
 rootfs=$(findfs /)
 bootfs=$(findfs /boot)
 [ -n "$bootfs" ] || bootfs="$rootfs"
@@ -590,6 +621,52 @@ esac
 db_progress STEP 1
 db_progress INFO grub-installer/progress/step_bootdev
 
+select_bootdev() {
+	local manual_entry bootdev_choices default_choice chosen result
+
+	result=""
+	default_choice="$1"
+
+	manual_entry="Enter device manually"
+	bootdev_choices="$(device_list)"
+	bootdev_choices="$manual_entry, $bootdev_choices"
+
+	log "Bootdev Choices: '$bootdev_choices'"
+	# not clear if we need to do this inside this function
+	#db_fset  grub-installer/choose_bootdev seen false
+	db_subst grub-installer/choose_bootdev choices "$bootdev_choices"
+	chosen="$(device_list $default_choice)"
+	if [ -n "$chosen" ] ;then
+		db_set grub-installer/choose_bootdev "$chosen"
+	fi
+
+	# what priority - critical, or high?
+	db_input high grub-installer/choose_bootdev || true
+	if ! db_go; then
+		log "Returning to menu"
+		db_progress STOP
+		exit 10
+	fi
+	if [ "$RET" = "$manual_entry" ] ; then
+		result=""
+	else
+		result="(echo "$RET" | cut -d' ' -f1)"
+	fi
+
+	echo "result"
+}
+
+# make sure this question is displayed at least once
+db_fset  grub-installer/choose_bootdev seen false
+
+if [ "$bootdev" != "dummy" ] && [ ! "$frdev" ]; then
+    # check for a preseeded value
+    db_get grub-installer/bootdev || true
+    if [ -n "$RET" ] ; then
+	bootdev="$RET"
+    fi
+fi
+
 while : ; do
 	if [ "$state" = 1 ]; then
 		db_input high $q || true
@@ -600,8 +677,17 @@ while : ; do
 		fi
 		db_get $q
 		if [ "$RET" = true ]; then
-			bootdev="$default_bootdev"
-			break
+			# default_bootdev can be guessed incorrectly.
+			# If the user supplied a value for bootdev,
+			# ask them to resolve any conflict.
+			if [ "$bootdev" != "$default_bootdev" ] ; then
+				bootdev="$(select_bootdev "$bootdev")"
+			fi
+			if [ -e "$bootdev" ] ; then
+			    break
+			else
+			    state=2
+			fi
 		else
 			# Exit to menu if /boot is on SATA RAID/multipath; we
 			# don't support device selection in that case
@@ -612,7 +698,13 @@ while : ; do
 			state=2
 		fi
 	elif [ "$state" = 2 ]; then
-		db_input critical grub-installer/bootdev || true
+
+		# this won't necessarily display a question
+		bootdev="$(select_bootdev "$bootdev")"
+
+		if [ ! -e "$bootdev" ]; then
+		    db_input critical grub-installer/bootdev || true
+		fi
 		if ! db_go; then
 			if [ "$q" ]; then
 				state=1


Reply to: