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

Bug#611045: debian-installer: find a better way of detecting default GRUB bootloader install location



Package: grub-installer
Version: 1.117+deb8u1
Followup-For: Bug #611045

Dear Maintainer,

   * What led up to the situation?
Installation using USB stick emulated by QEMU

   * What exactly did you do (or not do) that was effective (or
     ineffective)?
Preseed grub-installer/bootdev=default

   * What was the outcome of this action?
grub-installer tried /dev/sda = USB stick

   * What outcome did you expect instead?
grub-installer should use /dev/vda

See attached patch.

-- System Information:
Debian Release: 8.6
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'stable'), (90, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 3.16.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
>From 4d37f26c2bb1030a1013ce457ce98864fda127d6 Mon Sep 17 00:00:00 2001
Message-Id: <4d37f26c2bb1030a1013ce457ce98864fda127d6.1478269339.git.hahn@univention.de>
From: Philipp Hahn <hahn@univention.de>
Date: Fri, 4 Nov 2016 15:11:47 +0100
Subject: [PATCH 1/2] Choose better default for bootdev
Organization: Univention GmbH, Bremen, Germany

`grub-mkdevicemap` enumerates the devices in random/BIOS(?) order:
> (hd0)  /dev/disk/by-id/usb-QEMU_QEMU_HARDDISK_1-0000:00:07.7-2-0:0
> (hd1)  /dev/vda
The current code always selects the first drive, which in this case is
the emulated USB stick containing the Hybrid-ISO. Together with
"grub-installer/bootdev=default" the installer tries to install GRUB to
'/dev/sda', which fails.

As we know the device name of the installation before calling
`grub-mkdevicemap`, use that knowledge to filter out those devices first
and only then select the default device from the remaining devices.
---
 grub-installer | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/grub-installer b/grub-installer
index 7655408..139a910 100755
--- a/grub-installer
+++ b/grub-installer
@@ -570,15 +570,6 @@ if [ "$frdev" ]; then
 	fi
 fi
 
-# Try to avoid using (hd0) as a boot device name.  Something which can be
-# turned into a stable by-id name is better.
-default_bootdev_os="$($chroot $ROOT grub-mkdevicemap --no-floppy -m - | head -n1 | cut -f2)"
-if [ "$default_bootdev_os" ]; then
-	default_bootdev="$($chroot $ROOT readlink -f "$default_bootdev_os")"
-else
-	default_bootdev="(hd0)"
-fi
-
 # Set a sensible default boot device, so that we aren't installing GRUB to
 # installation media which may be removed later.  The disk containing /cdrom
 # is very unlikely to be a sensible default.  If we had to fall back to
@@ -593,11 +584,16 @@ hybrid=false
 if db_get cdrom-detect/hybrid; then
 	hybrid="$RET"
 fi
+# Try to avoid using (hd0) as a boot device name.  Something which can be
+# turned into a stable by-id name is better.
+default_bootdev="$(
+	$chroot "$ROOT" /bin/sh -c 'grub-mkdevicemap --no-floppy -m -|while read grub dev;do readlink -f "$dev";done' |
+	grep -F -v ${cdsrc:+-e "$(device_to_disk "$cdsrc")"} ${hdsrc:+-e "$(device_to_disk "$hdsrc")"} |
+	head -n1)"
+: ${default_bootdev:='(hd0)'}
 case $ARCH:$grub_package in
     *:grub|*:grub-pc|sparc:grub-ieee1275)
-	if [ "$(device_to_disk "$cdsrc")" = "$default_bootdev" ] || \
-	   ([ -n "$hdsrc" ] && [ "$(device_to_disk "$hdsrc")" = "$default_bootdev" ]) || \
-	   ([ "$default_bootdev" = '(hd0)' ] && \
+	if ([ "$default_bootdev" = '(hd0)' ] && \
 	    (([ -n "$cdfs" ] && [ "$cdfs" != "iso9660" ]) || \
 	     [ "$hybrid" = true ])) || \
 	   ([ "$default_bootdev" != '(hd0)' ] && \
-- 
2.1.4


>From 8ed38114bb518b55cc5359f9055a9ce3b4e8df9d Mon Sep 17 00:00:00 2001
Message-Id: <8ed38114bb518b55cc5359f9055a9ce3b4e8df9d.1478269339.git.hahn@univention.de>
In-Reply-To: <4d37f26c2bb1030a1013ce457ce98864fda127d6.1478269339.git.hahn@univention.de>
References: <4d37f26c2bb1030a1013ce457ce98864fda127d6.1478269339.git.hahn@univention.de>
From: Philipp Hahn <hahn@univention.de>
Date: Fri, 4 Nov 2016 14:27:12 +0100
Subject: [PATCH 2/2] Remove use-less bask-slash line continuation
Organization: Univention GmbH, Bremen, Germany

\ after && and || and | is not required.
---
 debian/postinst                     |  6 +++---
 grub-installer                      | 38 ++++++++++++++++++-------------------
 otheros.sh                          |  2 +-
 rescue.d/81grub-efi-force-removable |  6 +++---
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/debian/postinst b/debian/postinst
index 4b12027..11b4847 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -21,11 +21,11 @@ die () {
 mountvirtfs () {
 	fstype="$1"
 	path="$2"
-	if grep -q "[[:space:]]$fstype\$" /proc/filesystems && \
+	if grep -q "[[:space:]]$fstype\$" /proc/filesystems &&
 	   ! grep -q "^[^ ]\+ \+$path " /proc/mounts; then
-		mkdir -p "$path" || \
+		mkdir -p "$path" ||
 			die grub-installer/mounterr "Error creating $path"
-		mount -t "$fstype" "$fstype" "$path" || \
+		mount -t "$fstype" "$fstype" "$path" ||
 			die grub-installer/mounterr "Error mounting $path"
 		trap "umount $path" HUP INT QUIT KILL PIPE TERM EXIT
 	fi
diff --git a/grub-installer b/grub-installer
index 139a910..68b59b6 100755
--- a/grub-installer
+++ b/grub-installer
@@ -133,7 +133,7 @@ hurd_convert () {
 
 # This should probably be rewritten using udevadm or similar.
 device_to_disk () {
-	echo "$1" | \
+	echo "$1" |
 		sed 's:\(/dev/\(cciss\|ida\|rs\)/c[0-9]d[0-9][0-9]*\|/dev/mmcblk[0-9]\|/dev/\(ad\|ada\|da\)[0-9]\+\|/dev/[hs]d[0-9]\+\|/dev/[a-z]\+\).*:\1:'
 }
 
@@ -266,7 +266,7 @@ bootfslabel=$(partmap $disc_offered || echo unknown)
 
 # Check if the boot file system is on Serial ATA RAID
 frdev=""
-if type dmraid >/dev/null 2>&1 && dmraid -s -c >/dev/null 2>&1 && \
+if type dmraid >/dev/null 2>&1 && dmraid -s -c >/dev/null 2>&1 &&
    db_get disk-detect/activate_dmraid && [ "$RET" = true ]; then
 	for frdisk in $(dmraid -s -c); do
 		if echo "$disc_offered" | grep -q "/$frdisk[0-9]\+"; then
@@ -279,9 +279,9 @@ if type dmraid >/dev/null 2>&1 && dmraid -s -c >/dev/null 2>&1 && \
 	done
 # Check if the boot file system is on multipath
 elif type multipath >/dev/null 2>&1; then
-	for frdisk in $(multipath -l 2>/dev/null | \
+	for frdisk in $(multipath -l 2>/dev/null |
 			grep '^mpath[0-9]\+ ' | cut -d ' ' -f 1); do
-		if echo "$disc_offered" | 	   \
+		if echo "$disc_offered" |
 			grep -q "^/dev/mapper/${frdisk}-part[0-9]\+"; then
 			frdev=/dev/mapper/$frdisk
 			frbootpart=${disc_offered#$frdev}
@@ -295,8 +295,8 @@ elif type multipath >/dev/null 2>&1; then
 		apt-install dmsetup
 		$chroot $ROOT dmsetup mknodes
 	fi
-elif type lvdisplay >/dev/null 2>&1 && \
-     (lvdisplay "$disc_offered" | grep -q 'LV Name' 2>/dev/null || \
+elif type lvdisplay >/dev/null 2>&1 &&
+     (lvdisplay "$disc_offered" | grep -q 'LV Name' 2>/dev/null ||
       [ -e "$(dirname "$disc_offered")/control" ]); then
 	# Don't set frdev/frdisk here, otherwise you'll end up in different
 	# code paths below ...
@@ -559,7 +559,7 @@ else
 fi
 
 if [ "$frdev" ]; then
-	if [ -e $ROOT$frdev ] && [ -e $ROOT$frdev$frbootpart ] && \
+	if [ -e $ROOT$frdev ] && [ -e $ROOT$frdev$frbootpart ] &&
 	   [ $frgrubroot -ge $((1 - $partition_offset)) ]; then
 		db_subst grub-installer/$frtype GRUBROOT $ROOT$frdev$frbootpart
 		q=grub-installer/$frtype
@@ -593,11 +593,11 @@ default_bootdev="$(
 : ${default_bootdev:='(hd0)'}
 case $ARCH:$grub_package in
     *:grub|*:grub-pc|sparc:grub-ieee1275)
-	if ([ "$default_bootdev" = '(hd0)' ] && \
-	    (([ -n "$cdfs" ] && [ "$cdfs" != "iso9660" ]) || \
-	     [ "$hybrid" = true ])) || \
-	   ([ "$default_bootdev" != '(hd0)' ] && \
-	    ! partmap "$default_bootdev" >/dev/null && \
+	if ([ "$default_bootdev" = '(hd0)' ] &&
+	    (([ -n "$cdfs" ] && [ "$cdfs" != "iso9660" ]) ||
+	     [ "$hybrid" = true ])) ||
+	   ([ "$default_bootdev" != '(hd0)' ] &&
+	    ! partmap "$default_bootdev" >/dev/null &&
 	    ! grub_probe -t fs -d "$default_bootdev" >/dev/null); then
 		db_fget grub-installer/bootdev seen
 		if [ "$RET" != true ]; then
@@ -973,7 +973,7 @@ make_active_partition () {
 			bootfs_split="$(split_device "$bootfs")"
 			bootfs_disk="${bootfs_split%% *}"
 			bootfs_part="${bootfs_split#* }"
-			if [ "$bootfs_disk" = "$bootdisk" ] && \
+			if [ "$bootfs_disk" = "$bootdisk" ] &&
 			   ([ "$bootfs_part" -ge 1 ] && [ "$bootfs_part" -le 4 ]); then
 				bootpart="$bootfs_part"
 			fi
@@ -1016,7 +1016,7 @@ update_grub
 # For dmraid we can end up with e.g. '(sil_aiahbgbgaaaj1)', or we can end up
 # with '(hd0,0)' when it should be a higher partition
 # TODO: This should really be better supported in update-grub
-if [ "$frdev" ] && \
+if [ "$frdev" ] &&
    ! grep -q "^# groot=(hd0,$frgrubroot)" $ROOT/boot/grub/$menu_file; then
 	info "Fixing up the grub root to '(hd0,$frgrubroot)'"
 	sed -i "/^# groot/s/(.*)/(hd0,$frgrubroot)/" $ROOT/boot/grub/$menu_file
@@ -1076,9 +1076,9 @@ if [ "$grub_version" = "grub" ] ; then
 			db_fset grub-installer/password-again seen false
 		done
 		if [ "$password" ]; then
-			password=$(echo -e "md5crypt\n$password" | \
+			password=$(echo -e "md5crypt\n$password" |
 				   $chroot $ROOT \
-				   grub --batch --device-map=/dev/null 2>&1 | \
+				   grub --batch --device-map=/dev/null 2>&1 |
 				   grep "^Encrypted:" | cut -d' ' -f2)
 		fi
 	fi
@@ -1182,7 +1182,7 @@ for os in $(cat /tmp/os-probed); do
 			fi
 			kernel=$(echo "$entry" | cut -d : -f4)
 			initrd=$(echo "$entry" | cut -d : -f5)
-			if echo "$kernel" | grep -q '^/boot/' && \
+			if echo "$kernel" | grep -q '^/boot/' &&
 			   [ "$mappedbootpart" != "$mappedpartition" ]; then
 				# separate /boot partition
 				kernel=$(echo "$kernel" | sed 's!^/boot!!')
@@ -1244,7 +1244,7 @@ case $ARCH in
     mipsel/loongson-2f)
 	# Configure PMON to load GRUB by default.
 	if [ ! -e $ROOT/boot.cfg ] && [ ! -e $ROOT/boot/boot.cfg ]; then
-		pmon_partition="$(grub_probe -d -t drive "$bootfs" | \
+		pmon_partition="$(grub_probe -d -t drive "$bootfs" |
 				  sed 's/.*,//; s/[^0-9]//g')"
 		if [ "$pmon_partition" ]; then
 			pmon_partition=$(($pmon_partition - 1))
@@ -1277,7 +1277,7 @@ db_progress INFO grub-installer/progress/step_update_etc
 if [ -e $ROOT/etc/kernel-img.conf ] ; then
 	sed -i 's/do_bootloader = yes/do_bootloader = no/' $ROOT/etc/kernel-img.conf
 fi
-if [ ! -e $ROOT/etc/kernel/postinst.d/zz-update-grub ] && \
+if [ ! -e $ROOT/etc/kernel/postinst.d/zz-update-grub ] &&
    [ -z "$(grep update-grub $ROOT/etc/kernel-img.conf)" ]; then
 	(
 		echo "postinst_hook = update-grub"
diff --git a/otheros.sh b/otheros.sh
index 751732c..03acbcd 100644
--- a/otheros.sh
+++ b/otheros.sh
@@ -72,7 +72,7 @@ EOF
 	# DOS/Windows can't deal with booting from a non-first hard drive
 	case $shortname in
 	    MS*|Win*)
-		if $chroot $ROOT dpkg --compare-versions $grub_debian_version gt 1.96+20090609-1 && \
+		if $chroot $ROOT dpkg --compare-versions $grub_debian_version gt 1.96+20090609-1 &&
 		  [ "$title" != "Windows Vista (loader)" ]; then
 			    cat >> $tmpfile <<EOF
 	drivemap -s (hd0) \$root
diff --git a/rescue.d/81grub-efi-force-removable b/rescue.d/81grub-efi-force-removable
index 8761511..d9d4707 100755
--- a/rescue.d/81grub-efi-force-removable
+++ b/rescue.d/81grub-efi-force-removable
@@ -27,11 +27,11 @@ die () {
 mountvirtfs () {
 	fstype="$1"
 	path="$2"
-	if grep -q "[[:space:]]$fstype\$" /proc/filesystems && \
+	if grep -q "[[:space:]]$fstype\$" /proc/filesystems &&
 	   ! grep -q "^[^ ]\+ \+$path " /proc/mounts; then
-		mkdir -p "$path" || \
+		mkdir -p "$path" ||
 			die grub-installer/mounterr "Error creating $path"
-		mount -t "$fstype" "$fstype" "$path" || \
+		mount -t "$fstype" "$fstype" "$path" ||
 			die grub-installer/mounterr "Error mounting $path"
 		EXTRA_PATHS="$EXTRA_PATHS $path"
 		trap "umount $EXTRA_PATHS" HUP INT QUIT KILL PIPE TERM EXIT
-- 
2.1.4


Reply to: