Bug#706112: [PATCH 0/4] grub-installer: Support menu selection of grub boot disk
On Sun, Jun 09, 2013 at 01:47:36AM +0200, Cyril Brulebois wrote:
> Vincent McIntyre <vincent.mcintyre@csiro.au> (30/04/2013):
> > gah. The patch was not ok. My apologies for such a gross error.
> > I caught this by testing with 1.86 as downloaded from the archive.
>
> Thanks, Vincent.
>
> Applied locally. I'll try and play with it, and see when that can be
> pushed to unstable, and maybe into {the next,an upcoming} wheezy point
> release.
Attached is a patch series I did a few weeks ago.
I haven't had time to test carefully but I should get them out there.
0001 of the series fixes the issue I raised earlier in the thread,
in a slightly different way. kibi suggests this should be considered
for inclusion in time for the wheezy point release.
0002 & 0003 are cleanups.
0004 introduces naming the disks in the same way as grub-pc.
All of the above will need testing, particular 0004.
>From d8a3a9149f1a123caece540910608a772893f307 Mon Sep 17 00:00:00 2001
From: Vince McIntyre <vincent.mcintyre@csiro.au>
Date: Fri, 3 May 2013 23:33:28 +1000
Subject: [PATCH 1/4] Add missing break in $bootdev question loop.
In version 1.86 of grub-installer, the while loop in which
grub-installer/bootdev is asked can result in the following
sequence of events:
$state is set to 1
grub-installer/only_debian is asked and gets a 'false' answer
$state is set to 2
$bootdev is set by select_bootdev (asks grub-installer/choose_bootdev).
The device pointed to by $bootdev exists,
so grub-installer/bootdev is never queued to be asked,
thus db_go returns true
and the code sets $bootdev to the value of grub-installer/bootdev,
which has not been set yet.
This results in grub-install being run with a blank value for $bootdev.
To avoid this, break out of the loop if select_bootdev returns
a suitable value for $bootdev. Otherwise, ask grub-installer/bootdev.
Signed-off-by: Vince McIntyre <vincent.mcintyre@csiro.au>
---
grub-installer | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/grub-installer b/grub-installer
index 71e10c8..e0ce818 100755
--- a/grub-installer
+++ b/grub-installer
@@ -703,9 +703,11 @@ while : ; do
unset previous_state
fi
- if [ ! -e "$bootdev" ]; then
- db_input critical grub-installer/bootdev || true
+ if [ -e "$bootdev" ]; then
+ break
fi
+
+ db_input critical grub-installer/bootdev || true
if ! db_go; then
if [ "$q" ]; then
state=1
--
1.8.2.1
>From 190e6576a205b9d166ee47e834b664b1037a10e7 Mon Sep 17 00:00:00 2001
From: Vince McIntyre <vincent.mcintyre@csiro.au>
Date: Fri, 3 May 2013 23:43:33 +1000
Subject: [PATCH 2/4] Remove unnecessary else clause.
Signed-off-by: Vince McIntyre <vincent.mcintyre@csiro.au>
---
grub-installer | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/grub-installer b/grub-installer
index e0ce818..bc1a482 100755
--- a/grub-installer
+++ b/grub-installer
@@ -684,9 +684,8 @@ while : ; do
fi
if [ -e "$bootdev" ] ; then
break
- else
- state=2
fi
+ state=2
else
# Exit to menu if /boot is on SATA RAID/multipath; we
# don't support device selection in that case
--
1.8.2.1
>From 76b8a7059e410a83ab54e9378a2167dcb8d94bb3 Mon Sep 17 00:00:00 2001
From: Vince McIntyre <vincent.mcintyre@csiro.au>
Date: Fri, 3 May 2013 23:44:46 +1000
Subject: [PATCH 3/4] Same line in both branches of if statement.
Signed-off-by: Vince McIntyre <vincent.mcintyre@csiro.au>
---
grub-installer | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/grub-installer b/grub-installer
index bc1a482..fc5078a 100755
--- a/grub-installer
+++ b/grub-installer
@@ -685,7 +685,6 @@ while : ; do
if [ -e "$bootdev" ] ; then
break
fi
- state=2
else
# Exit to menu if /boot is on SATA RAID/multipath; we
# don't support device selection in that case
@@ -693,8 +692,8 @@ while : ; do
db_progress STOP
exit 10
fi
- state=2
fi
+ state=2
elif [ "$state" = 2 ]; then
if [ "$previous_state" != "1" ]; then
--
1.8.2.1
>From 18d8175cfcb4807772e957cd904132e839b51720 Mon Sep 17 00:00:00 2001
From: Vince McIntyre <vincent.mcintyre@csiro.au>
Date: Sat, 4 May 2013 00:32:50 +1000
Subject: [PATCH 4/4] Merge describe_disks() from grub-pc.
grub-pc's postinst displays disk descriptions in a clear & concise way.
Use that for the disk descriptions in the choose_bootdev question.
This introduces another debconf variable, disk_description.
Signed-off-by: Vince McIntyre <vincent.mcintyre@csiro.au>
---
debian/grub-installer.templates | 6 +++
grub-installer | 86 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 90 insertions(+), 2 deletions(-)
mode change 100755 => 100644 grub-installer
diff --git a/debian/grub-installer.templates b/debian/grub-installer.templates
index e439ad0..b52fc8e 100644
--- a/debian/grub-installer.templates
+++ b/debian/grub-installer.templates
@@ -112,6 +112,12 @@ _Description: GRUB password:
.
If you do not wish to set a GRUB password, leave this field blank.
+Template: grub-installer/disk_description
+Type: text
+# Disk sizes are in decimal megabytes, to match how disk manufacturers
+# usually describe them.
+_Description: ${DEVICE} (${SIZE} MB; ${MODEL})
+
Template: grub-installer/password-again
Type: password
# :sl2:
diff --git a/grub-installer b/grub-installer
old mode 100755
new mode 100644
index fc5078a..5d2fcf0
--- a/grub-installer
+++ b/grub-installer
@@ -218,6 +218,89 @@ devices_to_ids()
echo "$ids"
}
+# sysfs_size, camctrl_size, describe_disk lifted from grub-pc.postinst
+
+# for Linux
+sysfs_size()
+{
+ local num_sectors sector_size size
+ # Try to find out the size without relying on a partitioning tool being
+ # installed. This isn't too hard on Linux 2.6 with sysfs, but we have to
+ # try a couple of variants on detection of the sector size.
+ if [ -e "$1/size" ]; then
+ num_sectors="$(cat "$1/size")"
+ sector_size=512
+ if [ -e "$1/queue/logical_block_size" ]; then
+ sector_size="$(cat "$1/queue/logical_block_size")"
+ elif [ -e "$1/queue/hw_sector_size" ]; then
+ sector_size="$(cat "$1/queue/hw_sector_size")"
+ fi
+ size="$(expr "$num_sectors" \* "$sector_size" / 1000 / 1000)"
+ fi
+ [ "$size" ] || size='???'
+ echo "$size"
+}
+
+# for kFreeBSD
+camcontrol_size()
+{
+ local num_sectors sector_size size=
+
+ if num_sectors="$(camcontrol readcap "$1" -q -s -N)"; then
+ sector_size="$(camcontrol readcap "$1" -q -b)"
+ size="$(expr "$num_sectors" \* "$sector_size" / 1000 / 1000)"
+ fi
+
+ [ "$size" ] || size='???'
+ echo "$size"
+}
+
+describe_disk()
+{
+ # Returns description string in $RET, like a debconf command
+ local disk id base size
+ disk="$1"
+ id="$2"
+
+ model=
+ case $(uname -s) in
+ Linux)
+ if which udevadm >/dev/null 2>&1; then
+ size="$(sysfs_size "/sys$(udevadm info -n "$disk" -q path)")"
+ else
+ base="${disk#/dev/}"
+ base="$(printf %s "$base" | sed 's,/,!,g')"
+ size="$(sysfs_size "/sys/block/$base")"
+ fi
+
+ if which udevadm >/dev/null 2>&1; then
+ model="$(udevadm info -n "$disk" -q property | sed -n 's/^ID_MODEL=//p')"
+ if [ -z "$model" ]; then
+ model="$(udevadm info -n "$disk" -q property | sed -n 's/^DM_NAME=//p')"
+ if [ -z "$model" ]; then
+ model="$(udevadm info -n "$disk" -q property | sed -n 's/^MD_NAME=//p')"
+ if [ -z "$model" ] && which dmsetup >/dev/null 2>&1; then
+ model="$(dmsetup info -c --noheadings -o name "$disk" 2>/dev/null || true)"
+ fi
+ fi
+ fi
+ fi
+ ;;
+ GNU/kFreeBSD)
+ disk_basename=$(basename "$disk")
+ size="$(camcontrol_size "$disk_basename")"
+ model="$(camcontrol inquiry "$disk_basename" | sed -ne "s/^pass0: <\([^>]*\)>.*/\1/p")"
+ ;;
+ esac
+
+ [ "$model" ] || model='???'
+ db_subst grub-installer/disk_description DEVICE "$disk"
+ db_subst grub-installer/disk_description SIZE "$size"
+ db_subst grub-installer/disk_description MODEL "$model"
+ db_metaget grub-installer/disk_description description
+}
+
+
rootfs=$(findfs /)
bootfs=$(findfs /boot)
[ -n "$bootfs" ] || bootfs="$rootfs"
@@ -604,7 +687,6 @@ select_bootdev() {
# Let's trust grub-mkdevicemap to select the most suitable ones
# and correctly handle systems with no /dev/disk/by-id.
# Use disk id string as a shortcut way to describe it.
- # FIXME switch to grub-pc's far more elegant disk_descriptions()
dev_list=
dev_descr=
devices="$($chroot $ROOT grub-mkdevicemap --no-floppy -m - | cut -f2)"
@@ -612,7 +694,7 @@ select_bootdev() {
disk_id="$(device_to_id $grubdev)"
dev="$(readlink -f "$disk_id")"
dev_list="${dev_list:+$dev_list, }$dev"
- descr="$(echo $disk_id |sed -e 's+^.*/++' |sed -e 's+,+\\,+g')"
+ descr="$(describe_disk "$dev" "$disk_id")"
if [ "$dev" = "$disk_id" ]; then
dev_descr="${dev_descr:+$dev_descr, }$dev"
else
--
1.8.2.1
Reply to: