On Mon, Apr 08, 2013 at 02:51:07PM +0200, Cyril Brulebois wrote: > Vincent McIntyre <vincent.mcint...@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 -- To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/20130419001654.ga21...@mayhem.atnf.csiro.au