[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#983107: os-prober: generic subvolume support for btrfs



Package: os-prober
Version: 1.78
Severity: normal

Issue:
Currently Debian os-prober support only btrfs root-filesystem on the root of
the btrfs, i.e., ID 5 (FS_TREE).  This makes auto generated grub.cfg to miss
Linux install to btrfs for some Ubuntu and Suse since they put root-system
under @ subvolume.

Existing patch in other distro:
Ubuntu ships patched os-prober 1.77 to address its subvolume use (@ as root-
filesystem) with hardcoded path and very rudamental check for /lib directory.

--------
diff -pruN 1.77/linux-boot-probes/common/50mounted-tests 1.77ubuntu1/linux-
boot-probes/common/50mounted-tests
--- 1.77/linux-boot-probes/common/50mounted-tests       2018-08-10
19:23:18.000000000 +0000
+++ 1.77ubuntu1/linux-boot-probes/common/50mounted-tests        2020-11-02
11:12:51.000000000 +0000
@@ -54,6 +54,19 @@ if type grub-mount >/dev/null 2>&1 && \
        mounted=1
        type="$(grub-probe -d "$partition" -t fs)"
        [ "$type" ] || type=fuseblk
+
+       case "$type" in
+           btrfs)
+                       if [ -x "$tmpmnt/@/lib" ] && \
+                          ! mount --bind "$tmpmnt/@" "$tmpmnt"; then
+                               warn "failed to mount btrfs subvolume @ on
$partition"
+                               if ! umount $tmpmnt; then
+                                       warn "failed to umount $tmpmnt"
+                               fi
+                               mounted=
+                       fi
+                       ;;
+       esac
 fi

 if [ "$mounted" ]; then
------

Discussion:
Since we should offer the choice for the subbvolume name, this hardcoding "@"
here is not elegant.  We should get it as:
-----
RSUBVOL=$(btrfs subvolume get-default $tmpmnt |cut -d' ' -f 9)
-----

Proposed fix:
So updated patch should be more like
---
+       case "$type" in
+           btrfs)
+                       RSUBVOL=$(btrfs subvolume get-default $tmpmnt |cut -d'
' -f 9)
+                       if [ -n "$RSUBVOL" ] && [ -x "$tmpmnt/$RSUBVOL/lib" ]
&& \
+                          ! mount --bind "$tmpmnt/$RSUBVOL" "$tmpmnt"; then
+                               warn "failed to mount btrfs subvolume $RSUBVOL
on $partition"
+                               if ! umount $tmpmnt; then
+                                       warn "failed to umount $tmpmnt"
+                               fi
+                               mounted=
+                       fi
+                       ;;
+       esac
---

This is much simpler patch than ones discussed in
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=688336
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=699189

Since we expect any sane person set-default to the root-filesystem.

You could add fall-back with "elif" to use Ubuntu specific hardcoded subvolume
name.

This should make Debian friendly to older Ubuntu with btrfs.

What do you think?

Osamu



-- System Information:
Debian Release: bullseye/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.0-3-amd64 (SMP w/12 CPU threads)
Kernel taint flags: TAINT_WARN
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US:en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages os-prober depends on:
ii  grub-common  2.04-15
ii  libc6        2.31-9

os-prober recommends no packages.

os-prober suggests no packages.

-- no debconf information


Reply to: