Re: Bug#747653: grub2-common: update-grub adds both devices and a line feed for BTRFS RAID 1 setup

[Andrei Borzenkov <arvidjaar@gmail.com>]

В Sat, 06 Dec 2014 12:01:11 +0100
Martin Steigerwald <Martin@lichtvoll.de> пишет:

> Am Montag, 2. Juni 2014, 19:39:22 schrieb Andrey Borzenkov:
> > В Sat, 10 May 2014 20:53:34 +0200
> > 
> > Martin Steigerwald <Martin@Lichtvoll.de> пишет:
> > > Package: grub2-common
> > > Version: 2.02~beta2-10
> > > Severity: normal
> > > 
> > > Dear Maintainer,
> > > 
> > > I am booting my Debian system via a BTRFS RAID 1 which spans a logical
> > > volume on a Crucial MSATA and Intel SATA SSD each.
> > > 
> > > After running update-grub I am getting this in /boot/grub/grub.cfg:
> > >                 echo    'Linux 3.15.0-rc5-tp520 wird geladen …'
> > >                 linux   /vmlinuz-3.15.0-rc5-tp520
> > >                 root=/dev/mapper/sata-debian
> > >         
> > >         /dev/mapper/msata-debian ro rootflags=subvol=debian 
> > >         init=/bin/systemd resume=/dev/mapper/sata-swap>         
> > >                 echo    'Initiale Ramdisk wird geladen …'
> > >                 initrd  /initrd.img-3.15.0-rc5-tp520
> > > 
> > > update-grub basically adds both devices of the BTRFS RAID 1 device
> > > separated by a line feed. For mounting BTRFS RAID 1 tough one of them
> > > is enough, once btrfs device scan is run, for which I currently use an
> > > script for initramfs-tools as a work-around as it didn´t work out of
> > > the box on my last tests[1].
> > > 
> > > This behaviour is due to grub-probe which is called by grub-mkconfig
> > > at line 139
> > > 
> > > 138 # Device containing our userland.  Typically used for root= parameter.
> > > 139 GRUB_DEVICE="`${grub_probe} --target=device /`"
> > > 140 GRUB_DEVICE_UUID="`${grub_probe} --device ${GRUB_DEVICE}
> > > --target=fs_uuid 2> /dev/null`" || true
> > > 
> > > which is called by update-grub returns both devices with a
> > > linefeed:
> > > 
> > > merkaba:~> grub-probe --target=device /
> > > /dev/mapper/sata-debian
> > > /dev/mapper/msata-debian
> > > 
> > > grub-probe is an ELF binary.
> > > 
> > > The following little change workarounds the issue for me:
> > > 
> > > merkaba:~> diff -u /usr/sbin/grub-mkconfig.dist /usr/sbin/grub-mkconfig
> > > --- /usr/sbin/grub-mkconfig.dist        2014-05-08 14:35:25.000000000
> > > +0200
> > > +++ /usr/sbin/grub-mkconfig     2014-05-10 20:46:00.380096263 +0200
> > > @@ -136,7 +136,7 @@
> > > 
> > >  fi
> > >  
> > >  # Device containing our userland.  Typically used for root= parameter.
> > > 
> > > -GRUB_DEVICE="`${grub_probe} --target=device /`"
> > > +GRUB_DEVICE="`${grub_probe} --target=device / | head -1`"
> > > 
> > >  GRUB_DEVICE_UUID="`${grub_probe} --device ${GRUB_DEVICE} --target=fs_uuid
> > >  2> /dev/null`" || true
> > >  
> > >  # Device containing our /boot partition.  Usually the same as
> > >  GRUB_DEVICE.
> > > 
> > > But I suppose the real fix is to be made in the binary grub-probe.
> > 
> > No, grub-probe is correct; grub needs to know all devices so it can
> > have full information which drivers it requires to access them.
> > 
> > See also
> > https://lists.gnu.org/archive/html/grub-devel/2014-05/msg00005.html
> > 
> > I suggest you discuss it with Colin, but for now I tend to think, fix
> > should go into 10_linux. May be always use UUID for btrfs.
> > 
> > But this sounds like new can of worms :(
> 
> Any way on how to proceed on this one?
> 

Try patch below

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] fix linux kernel root= argument with multidev btrfs

Btrfs may reside on multiple devices, but only one of them should be
passed to root= kernel argument. Add two helper functions:

- get_root_device: select "best" device for the list. For single device
just return it to skip probing; for multiple devices try to find LVM as
before but fall back to the first device if not found.

- find_abstraction: return device with requested abstraction driver from
the given list.

Refactor code to use them and reimplement uses_abstraction on top of
find_abstraction.

Also replace check for LVM with uses_abstraction in 10_kfreebsd.

---
 util/grub-mkconfig_lib.in   | 50 ++++++++++++++++++++++++++++++++++-----------
 util/grub.d/10_kfreebsd.in  |  8 +++-----
 util/grub.d/10_linux.in     | 10 +++++----
 util/grub.d/20_linux_xen.in | 10 +++++----
 4 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
index 29ef865..ecae264 100644
--- a/util/grub-mkconfig_lib.in
+++ b/util/grub-mkconfig_lib.in
@@ -283,21 +283,27 @@ gettext_printf () {
   printf "$(gettext "$gettext_printf_format")" "$@"
 }
 
-uses_abstraction () {
-  device="$1"
-  old_ifs="$IFS"
+find_abstraction () {
+  abstraction="$1"
+  devices="$2"
+  old_IFS="$IFS"
   IFS='
 '
-
-  abstraction="`"${grub_probe}" --device ${device} --target=abstraction`"
-  for module in ${abstraction}; do
-    if test "x${module}" = "x$2"; then
-      IFS="$old_ifs"
-      return 0
-    fi
+  set $devices
+  IFS="$old_IFS"
+
+  for device in "$@"; do
+    for module in `"${grub_probe}" --device "${device}" --target=abstraction`; do
+      if test "x${module}" = "x${abstraction}"; then
+	printf "%s" "${device}"
+	return
+      fi
+    done
   done
-  IFS="$old_ifs"
-  return 1
+}
+
+uses_abstraction () {
+  test -n "`find_abstraction "$2" "$1"`"
 }
 
 print_option_help () {
@@ -368,3 +374,23 @@ grub_add_tab () {
   sed -e "s/^/$grub_tab/"
 }
 
+get_root_device() {
+  devices="$1"
+  old_IFS="$IFS"
+  IFS='
+'
+  set $devices
+  IFS="$old_IFS"
+
+  if [ x"$#" = x1 ]; then
+    printf "%s" "$1"
+    return
+  fi
+
+  out="`find_abstraction lvm "$devices"`"
+  if [ x"$out" = x ]; then
+    out="$1"
+  fi
+
+  printf "%s" "$out"
+}
diff --git a/util/grub.d/10_kfreebsd.in b/util/grub.d/10_kfreebsd.in
index 65f6c32..d9d0199 100644
--- a/util/grub.d/10_kfreebsd.in
+++ b/util/grub.d/10_kfreebsd.in
@@ -112,11 +112,9 @@ EOF
 
   load_kfreebsd_module acpi true
 
-  for abstraction in dummy $(${grub_probe} -t abstraction --device ${GRUB_DEVICE}) ; do
-    case $abstraction in
-      lvm) load_kfreebsd_module geom_linux_lvm false ;;
-    esac
-  done
+  if uses_abstraction "${GRUB_DEVICE}" lvm; then
+    load_kfreebsd_module geom_linux_lvm false
+  fi
 
   case "${kfreebsd_fs}" in
     zfs)
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index a2a84dc..68aa1e7 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -44,11 +44,13 @@ case ${GRUB_DEVICE} in
 esac
 
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
-    || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
-    || uses_abstraction "${GRUB_DEVICE}" lvm; then
-  LINUX_ROOT_DEVICE=${GRUB_DEVICE}
+    || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}"; then
+  LINUX_ROOT_DEVICE="`get_root_device "${GRUB_DEVICE}"`"
 else
-  LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
+  LINUX_ROOT_DEVICE="`find_abstraction lvm "${GRUB_DEVICE}"`"
+  if [ x"${LINUX_ROOT_DEVICE}" = x ]; then
+    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
+  fi
 fi
 
 case x"$GRUB_FS" in
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index de34c8d..b9426e9 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -44,11 +44,13 @@ case ${GRUB_DEVICE} in
 esac
 
 if [ "x${GRUB_DEVICE_UUID}" = "x" ] || [ "x${GRUB_DISABLE_LINUX_UUID}" = "xtrue" ] \
-    || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}" \
-    || uses_abstraction "${GRUB_DEVICE}" lvm; then
-  LINUX_ROOT_DEVICE=${GRUB_DEVICE}
+    || ! test -e "/dev/disk/by-uuid/${GRUB_DEVICE_UUID}"; then
+  LINUX_ROOT_DEVICE="`get_root_device "${GRUB_DEVICE}"`"
 else
-  LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
+  LINUX_ROOT_DEVICE="`find_abstraction lvm "${GRUB_DEVICE}"`"
+  if [ x"${LINUX_ROOT_DEVICE}" = x ]; then
+    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
+  fi
 fi
 
 # Allow overriding GRUB_CMDLINE_LINUX and GRUB_CMDLINE_LINUX_DEFAULT.
-- 
tg: (17328db..) u/btrfs-multidev (depends on: master)