Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Bug#554790: grub-pc: store grub-pc/install_devices using by-id

169 views
Skip to first unread message

Colin Watson

unread,
Jan 13, 2010, 12:20:03 PM1/13/10
to
tags 554790 patch
user ubuntu...@lists.ubuntu.com
usertags 554790 origin-ubuntu ubuntu-patch lucid
thanks


Preface (possibly unnecessary since I'm following up to an existing bug,
but let's summarise anyway):

Currently, grub-pc stores traditional Linux device names in
grub-pc/install_devices. This is suboptimal because those device names
are not stable: particularly when removable devices are involved, but
also otherwise, it's quite possible for /dev/sda and /dev/sdb to switch
places from boot to boot. I believe this to be the cause of many bug
reports to the effect that /boot/grub/ has got out of sync with GRUB's
embedded core.img. The general problem is not new to GRUB 2, although
the precise manifestation is.

Now that the installer is setting up /etc/fstab to use UUIDs rather than
traditional device names, we're very close to finally being free of our
dependency on Linux's device probing order, and on x86 I believe that
GRUB is the last remaining piece. Using embedding rather than
blocklists helped, but we also need to make sure that we consistently
install GRUB to the same disk.

UUIDs are properties of filesystems, not disks or partitions, and the
MBR of a hard disk does not have a UUID. For disks, the choices for
stable identifiers are based on either the hardware serial number or the
bus path taken to reach the disk. In this case, it seems most
appropriate to use identifiers based on the hardware serial number,
namely those in /dev/disk/by-id/, which uniquely identify a given
physical disk, or a numbered partition on a given physical disk, but not
any kind of logical construct on that disk (so they aren't suitable for
filesystem mounting where you care much more about the contents).

I believe that the appropriate fix for this class of bugs is to store
/dev/disk/by-id/ in grub-pc/install_devices if possible. This doesn't
work for people not using Linux or people not using udev, so in that
case we can simply fall back to the previous unreliable situation since
we have no better alternative.


The following patch is a *preliminary* attempt at fixing this bug. I've
only tested it relatively lightly. The dpkg --compare-versions command
will need to be adjusted for whatever version eventually merges this
code. Translations will need to be set up using debconf-updatepo once
the text is OK. I think most of the rest is reasonable, but I would
appreciate detailed review.


* Store grub-pc/install_devices as persistent device names under
/dev/disk/by-id/. Migrate previous device names to that, with explicit
confirmation to make sure we got the right ones. If the devices we were
told to install to ever go away, ask again.
* If the user opts to install GRUB nowhere, make sure they really mean it.

=== modified file 'debian/grub-pc.templates.in'
--- debian/grub-pc.templates.in 2009-12-14 13:36:49 +0000
+++ debian/grub-pc.templates.in 2010-01-13 16:49:06 +0000
@@ -23,6 +23,7 @@ _Description: Chainload from menu.lst?

Template: grub-pc/install_devices
Type: multiselect
+Choices-C: ${RAW_CHOICES}
Choices: ${CHOICES}
# Intentionally not marked for translations yet; will do after a review period
Description: GRUB install devices:
@@ -40,6 +41,52 @@ Description: GRUB install devices:
However, this forces GRUB to use the blocklist mechanism, which makes it
less reliable, and therefore is not recommended.

+Template: grub-pc/install_devices_disks_changed
+Type: multiselect
+Choices-C: ${RAW_CHOICES}
+Choices: ${CHOICES}
+# Intentionally not marked for translations yet; will do after a review period
+Description: GRUB install devices:
+ The GRUB boot loader was previously installed to a disk that is no longer
+ present, or whose normally unique identifier has changed for some reason.
+ It is important to make sure that the installed GRUB stays in sync with
+ other components such as grub.cfg or with newer Linux images it will have
+ to load, and so you should check again to make sure that GRUB is installed
+ to the appropriate boot devices.
+ .
+ If you're unsure which drive is designated as boot drive by your BIOS, it is
+ often a good idea to install GRUB to all of them.
+ .
+ Note: It is possible to install GRUB to partition boot records as well.
+ However, this forces GRUB to use the blocklist mechanism, which makes it
+ less reliable, and therefore is not recommended.
+
+Template: grub-pc/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-pc/partition_description
+Type: text
+# The "-" is used to indicate indentation. Leading spaces may not work.
+Description: - ${DEVICE} (${SIZE} MB)
+
+Template: grub-pc/install_devices_empty
+Type: boolean
+Default: false
+_Description: Continue without installing GRUB?
+ You chose not to install GRUB to any devices. If you continue, the boot
+ loader may not be properly configured, and when your computer next starts
+ up it will use whatever was previously in the boot sector. If there is an
+ earlier version of GRUB 2 in the boot sector, it may be unable to load
+ modules or handle the current configuration file.
+ .
+ If you are already running a different boot loader and want to carry on
+ doing so, or if this is a special environment where you do not need a boot
+ loader, then you should continue anyway. Otherwise, you should install
+ GRUB somewhere.
+
Template: grub-pc/postrm_purge_boot_grub
Type: boolean
Default: false

=== modified file 'debian/postinst.in'
--- debian/postinst.in 2009-12-14 13:36:49 +0000
+++ debian/postinst.in 2010-01-13 16:58:23 +0000
@@ -16,6 +16,124 @@ merge_debconf_into_conf()
fi
}

+cached_available_ids=
+available_ids()
+{
+ local id path
+
+ if [ "$cached_available_ids" ]; then
+ echo "$cached_available_ids"
+ return
+ fi
+
+ [ -d /dev/disk/by-id ] || return
+ cached_available_ids="$(
+ for id in $(ls /dev/disk/by-id | sort); do
+ path="/dev/disk/by-id/$id"
+ printf '%s %s\n' "$path" "$(readlink -f "$path")"
+ done | sort -k2 -s -u | cut -d' ' -f1
+ )"
+ echo "$cached_available_ids"
+}
+
+device_to_id()
+{
+ local id
+ for id in $(available_ids); do
+ if [ "$(readlink -f "$id")" = "$(readlink -f "$1")" ]; then
+ echo "$id"
+ return
+ fi
+ done
+}
+
+devices_to_ids()
+{
+ local device id ids
+ ids=
+ for device; do
+ id="$(device_to_id "$device")"
+ if [ "$id" ]; then
+ ids="${ids:+$ids, }$id"
+ fi
+ done
+ echo "$ids"
+}
+
+all_partitions()
+{
+ local id ids
+ ids=
+ for id in $(available_ids); do
+ if [ "$id" != "$1" ] && [ "${id%-part*}" = "$1" ]; then
+ ids="${ids:+$ids }$id"
+ fi
+ done
+ echo "$ids"
+}
+
+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"
+}
+
+# Returns value in $RET, like a debconf command.
+describe_disk()
+{
+ local disk id base size
+ disk="$1"
+ id="$2"
+
+ base="${disk#/dev/}"
+ base="$(printf %s "$base" | sed 's,/,!,g')"
+ size="$(sysfs_size "/sys/block/$base")"
+
+ model=
+ if which udevadm >/dev/null 2>&1; then
+ model="$(udevadm info -n "$disk" -q property | sed -n 's/^ID_MODEL=//p')"
+ fi
+ [ "$model" ] || model='???'
+
+ db_subst grub-pc/disk_description DEVICE "$disk"
+ db_subst grub-pc/disk_description SIZE "$size"
+ db_subst grub-pc/disk_description MODEL "$model"
+ db_metaget grub-pc/disk_description description
+}
+
+# Returns value in $RET, like a debconf command.
+describe_partition()
+{
+ local disk part id diskbase partbase size
+ disk="$1"
+ part="$2"
+ id="$3"
+
+ diskbase="${disk#/dev/}"
+ diskbase="$(printf %s "$diskbase" | sed 's,/,!,g')"
+ partbase="${part#/dev/}"
+ partbase="$(printf %s "$partbase" | sed 's,/,!,g')"
+ size="$(sysfs_size "/sys/block/$diskbase/$partbase")"
+
+ db_subst grub-pc/partition_description DEVICE "$part"
+ db_subst grub-pc/partition_description SIZE "$size"
+ db_metaget grub-pc/partition_description description
+}
+
case "$1" in
configure)
. /usr/share/debconf/confmodule
@@ -87,14 +205,78 @@ case "$1" in
touch /boot/grub/grub.cfg
fi
else
- db_subst grub-pc/install_devices CHOICES `grub-mkdevicemap -m - | sed -e "/^(fd[0-9]\+)/d;s,.*\t,,g" | tr '\n' ',' | sed -e 's/,$//g;s/,/, /g'`
- db_input high grub-pc/install_devices || true
- db_go
- db_get grub-pc/install_devices
- for i in `echo $RET | sed -e 's/,/ /g'` ; do
- if grub-install --force --no-floppy $i ; then
- # We just installed GRUB 2; then also generate grub.cfg.
- touch /boot/grub/grub.cfg
+ while :; do
+ question=grub-pc/install_devices
+ if dpkg --compare-versions "$2" lt 1.98~20100101-1ubuntu2; then
+ # Migrate to new by-id naming scheme.
+ db_get grub-pc/install_devices
+ db_set grub-pc/install_devices "$(devices_to_ids $RET)"
+ db_fset grub-pc/install_devices seen false
+ db_fset grub-pc/install_devices_empty seen false
+ else
+ db_get grub-pc/install_devices
+ valid=1
+ for device in $RET; do
+ if [ ! -e "$device" ]; then
+ valid=0
+ break
+ fi
+ done
+ if [ "$valid" = 0 ]; then
+ question=grub-pc/install_devices_disks_changed
+ db_set "$question" "$RET"
+ db_fset "$question" seen false
+ db_fset grub-pc/install_devices_empty seen false
+ fi
+ fi
+
+ devices="$(grub-mkdevicemap -m - | sed -e "/^(fd[0-9]\+)/d;s,.*\t,,g")"
+ ids=
+ descriptions=
+ for device in $devices; do
+ disk_id="$(device_to_id "$device")"
+ if [ "$disk_id" ]; then
+ ids="${ids:+$ids, }$disk_id"
+ describe_disk "$device" "$disk_id"
+ RET="$(printf %s "$RET" | sed 's/,/\\,/g')"
+ descriptions="${descriptions:+$descriptions, }$RET"
+ for id in $(all_partitions "$disk_id"); do
+ ids="${ids:+$ids, }$id"
+ describe_partition "$device" "$(readlink -f "$id")" "$id"
+ RET="$(printf %s "$RET" | sed 's/,/\\,/g')"
+ descriptions="${descriptions:+$descriptions, }$RET"
+ done
+ fi
+ done
+ db_subst "$question" RAW_CHOICES "$ids"
+ db_subst "$question" CHOICES "$descriptions"
+ db_input high "$question" || true
+ db_go
+ db_get "$question"
+ for i in `echo $RET | sed -e 's/,/ /g'` ; do
+ real_device="$(readlink -f "$i")"
+ if grub-install --force --no-floppy $real_device ; then
+ # We just installed GRUB 2; then also generate grub.cfg.
+ touch /boot/grub/grub.cfg
+ fi
+ done
+ if [ "$question" != grub-pc/install_devices ]; then
+ db_set grub-pc/install_devices "$RET"
+ db_fset grub-pc/install_devices seen true
+ fi
+
+ db_get grub-pc/install_devices
+ if [ -z "$RET" ]; then
+ db_input critical grub-pc/install_devices_empty || true
+ db_go
+ db_get grub-pc/install_devices_empty
+ if [ "$RET" = true ]; then
+ break
+ else
+ db_fset grub-pc/install_devices_empty seen false
+ fi
+ else
+ break
fi
done
fi


Thanks,

--
Colin Watson [cjwa...@ubuntu.com]

--
To UNSUBSCRIBE, email to debian-bugs-...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listm...@lists.debian.org

Robert Millan

unread,
Jan 15, 2010, 11:50:02 AM1/15/10
to
tags 554790 - patch
thanks

For the record, Colin said he will commit his patch soon to Bazaar repo
after he's finished testing it (he's now in pkg-grub team).

--
Robert Millan

"Be the change you want to see in the world" -- Gandhi

0 new messages