On Fri, Sep 01, 2017 at 01:31:28PM +0200, Paul Lagerweij wrote:
> Hello,
>
> I've created a patch for util/grub.d/10_linux.in for dynamic ZFS boot
> environment entries. I've only tested the patch in Ubuntu 16.04.2 with GRUB
> 2.02~beta2, ZFS on Linux 0.6.5.6, Linux 4.4.0-83 and 4.4.0-81, but the
> patch should work with any root ZFS configuration, because it only changes
> the 10_linux shell script. It is designed to work with FreeBSD's beadm
> tool, for which I'm currently making a Debian package, but isn't dependent
> on beadm. I added the patch as an attachment and would like to request a
> review.
You seem to be missing an Signed off?
And perhaps an commit description?
>
> Thanks.
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index de9044c..c9a318e 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -20,6 +20,7 @@ set -e
> prefix="@prefix@"
> exec_prefix="@exec_prefix@"
> datarootdir="@datarootdir@"
> +zfs_be="0"
Why not just
zfs_be=
And then you can just check for zfs_be having an value instead of for 1 or 0?
Or alternatively,
zfs_be=0
and then you can do if [ $zfs_be -ne 0 ]; .. or such?
>
> . "$pkgdatadir/grub-mkconfig_lib"
>
> @@ -62,9 +63,15 @@ case x"$GRUB_FS" in
> fi;;
> xzfs)
> rpool=`${grub_probe} --device ${GRUB_DEVICE} --target=fs_label
> 2>/dev/null || true`
> - bootfs="`make_system_path_relative_to_its_root / | sed -e "s,@$,,"`"
> - LINUX_ROOT_DEVICE="ZFS=${rpool}${bootfs}"
> - ;;
> + zfs_active_bootfs="`zpool list -H -o bootfs ${rpool} || true`"
Is zpool usually in /sbin or such? Perhaps a full path?
> + if [ -n "${zfs_active_bootfs}" ] && [ "${zfs_active_bootfs}" != "-" ]
> && \
> + [ `echo ${zfs_active_bootfs} | grep -o '/' | wc -l` -gt 1 ]; then
> + zfs_be="1"
> + LINUX_ROOT_DEVICE="ZFS=${zfs_active_bootfs}"
> + else
> + bootfs="`make_system_path_relative_to_its_root / | sed -e "s,@$,,"`"
> + LINUX_ROOT_DEVICE="ZFS=${rpool}${bootfs}"
> + fi;;
Did you test this where zpool is not installed?
> esac
>
> title_correction_code=
> @@ -177,6 +184,16 @@ title_correction_code=
> # yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
> submenu_indentation=""
>
> +if [ "${zfs_be}" = 1 ]; then
> + while read ZFS_BE_NAME ZFS_ORIGIN; do
> + if [ "${ZFS_ORIGIN}" != "-" ]; then
> + zfs_be_list="${zfs_be_list} ${ZFS_BE_NAME}"
> + fi
> + done << EOF
> +`zfs list -H -t filesystem -S creation -o name,origin -d 1
> ${zfs_active_bootfs%/*} || true`
> +EOF
> +fi
> +
> is_top_level=true
> while [ "x$list" != "x" ] ; do
> linux=`version_find_latest $list`
> @@ -184,6 +201,10 @@ while [ "x$list" != "x" ] ; do
> basename=`basename $linux`
> dirname=`dirname $linux`
> rel_dirname=`make_system_path_relative_to_its_root $dirname`
> + # If ZFS BE support and /boot is in ZFS.
> + if [ "${zfs_be}" = 1 ] && [ -n "${rel_dirname}" ]; then
> + rel_dirname="/""${zfs_active_bootfs#*/}""@/boot"
> + fi
> version=`echo $basename | sed -e "s,^[^0-9]*-,,g"`
> alt_version=`echo $version | sed -e "s,\.old$,,g"`
> linux_root_device_thisversion="${LINUX_ROOT_DEVICE}"
> @@ -238,12 +259,53 @@ while [ "x$list" != "x" ] ; do
> is_top_level=false
> fi
>
> - linux_entry "${OS}" "${version}" advanced \
> - "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
> - if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
> - linux_entry "${OS}" "${version}" recovery \
> - "single ${GRUB_CMDLINE_LINUX}"
> - fi
> + for menu_entries_group in default ${zfs_be_list}; do
> + found_entry="0"
> + if [ "${menu_entries_group}" = "default" ]; then
> + found_entry="1"
> + os_title="${OS}"
> + else
> + found_zfs_be="0"
> + zfs_be_name=${menu_entries_group}
> + # If /boot is not in ZFS.
> + if [ -z "${rel_dirname}" ]; then
> + found_zfs_be="1"
> + else
> + zfs_be_mounted="`zfs list -H -o mounted ${zfs_be_name}`"
> + if [ "${zfs_be_mounted}" = "yes" ]; then
> + ZFSMNT=`mount | grep -m 1 "^${zfs_be_name} " | cut -d' ' -f3)`
> + else
> + ZFSMNT=`mktemp -d "/tmp/$(echo ${zfs_be_name} | sed
> 's^/^-^g').XXX" || true`
> + zfs set mountpoint=legacy ${zfs_be_name} || true
> + mount -t zfs -o ro ${zfs_be_name} ${ZFSMNT} || true
> + fi
> + if [ -n "${ZFSMNT}" ] && [ -f "${ZFSMNT}/boot/vmlinuz-${version}" ];
> then
> + found_zfs_be="1"
> + rel_dirname="/""${zfs_be_name#*/}""@/boot"
> + fi
> + if [ "${zfs_be_mounted}" != "yes" ]; then
> + umount ${ZFSMNT} && rm -rf ${ZFSMNT} || true
> + zfs set mountpoint=/ ${zfs_be_name} || true
> + fi
> + fi
> +
> + if [ "${found_zfs_be}" = 1 ]; then
> + found_entry="1"
> + os_title="${OS} (${zfs_be_name##*/} BE)"
> + gettext_printf "Found ZFS boot environment: (%s BE) %s\n"
> "${zfs_be_name##*/}" "$linux" >&2
> + linux_root_device_thisversion="ZFS=${zfs_be_name}"
> + fi
> + fi
> +
> + if [ "${found_entry}" = 1 ]; then
> + linux_entry "${os_title}" "${version}" advanced \
> + "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
> + if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
> + linux_entry "${os_title}" "${version}" recovery \
> + "single ${GRUB_CMDLINE_LINUX}"
> + fi
> + fi
> + done
>
> list=`echo $list | tr ' ' '\n' | fgrep -vx "$linux" | tr '\n' ' '`
> done
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
