On Sun, Jun 09, 2013 at 01:47:36AM +0200, Cyril Brulebois wrote: > Vincent McIntyre <vincent.mcint...@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.mcint...@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.mcint...@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.mcint...@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.mcint...@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.mcint...@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.mcint...@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.mcint...@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.mcint...@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