Re: [Patch] Enable libzfs detection on Linux

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09.08.2011 19:48, Zachary Bedell wrote:
> Attached is a patch which allows Grub to detect and use libzfs on Linux as 
> provided by the ZfsOnLinux project.  
>
> Most of this work was originally done by Darik Horn (dajhorn on GitHub) 
> against the Grub 1.99 releases as patched by Ubuntu.  I've forward ported the 
> patches to trunk and separated some of the Ubuntu-specific stuff.  These 
> apply against trunk independent of the other ZFS work I've been doing.
>
> Changes include:
>
> configure.ac:  
>  * Add check for libspl to bring in Solaris Porting Library for Linux support.
>  * Include reference to Linux ZFS's config header to get necessary defines 
> for other ZFS headers on Linux.
>  * Add checks for libavl, libefi, libunicode, libzpool -- Linux decomposes 
> these libraries which are lumped into libzfs on other platforms.
>
> getroot.c:
>  * Add ref to mnttab.h from libspl to allow getextmntent to work on Linux.
>  * Change the order of the grub_find_root_device_from_mountinfo vs 
> find_root_device_from_libzfs calls so that libzfs runs first, falling down to 
> mountinfo if zfs isn't detected.
>  * Scan /proc/mounts and /etc/mtab in addition to /etc/mnttab to discover 
> mounted filesystems in grub_find_zpool_from_dir.
>

/etc/mtab is just a regular file and in many cases is out-of-sync with real 
state of affairs. Should be ignored altogether. Use of /etc/mnttab is 
unfortunate but I know of no other way on other platforms (since I haven't 
looked into it).

> These patches have been in use by a number of folks using ZfsOnLinux for some 
> time, and they've been robust on those systems.  I've tried to ensure the 
> changes won't impact non-Linux platforms, though I'm not sure I trust my 
> knowledge of autoconf enough to be positive there are no side effects.
>
You forget the effect of other code changes (below)
> - FILE *mnttab = fopen ("/etc/mnttab", "r");
> +    FILE *mnttab;
> +    mnttab = fopen ("/proc/mounts", "r");
/proc on FreeBSD is very different from Linux one. Don't try
/proc/mounts except if you have Linux.
> +
> +    if (! mnttab)
> +      mnttab = fopen ("/etc/mtab", "r");
> +
> +    if (! mnttab)
> +      mnttab = fopen ("/etc/mnttab", "r");
> +
>      if (! mnttab)
>        return;
>  
> -if [ "x`${grub_probe} --device ${GRUB_DEVICE} --target=fs 2>/dev/null || 
> true`" = xbtrfs ]; then
> +LINUX_ROOT_FS=`${grub_probe} --device ${GRUB_DEVICE} --target=fs 2>/dev/null 
> || true`
> +LINUX_ROOT_STAT=`stat -f --printf=%T / || true`
> +
> +if [ "x${LINUX_ROOT_FS}" = xbtrfs ] || [ "x${LINUX_ROOT_STAT}" = xbtrfs ]; 
> then
This changes logic for btrfs. I don't think it's necessary or good to
just change it.
> +if [ "x${LINUX_ROOT_FS}" = xzfs ]; then
> +  GRUB_CMDLINE_LINUX="boot=zfs \$bootfs ${GRUB_CMDLINE_LINUX}"
> +fi
> +
>    fi
> +  if [ "x${LINUX_ROOT_FS}" = xzfs ]; then
> +    cat << EOF
> +     insmod zfsinfo
> +     zfs-bootfs (\$root) bootfs
> +EOF
In this place $root refers to whereever kernel is. So if /boot is
separate it will be wrong. Moreover you completely forget the possible
subvolumes. One could have e.g.
FreeBSD in /freebsd/@/...
GNU/Linux in /gnu/linux/@
/boot in /boot/@
In this case $bootfs has to take subvolume into account.
Also nothing guarantees that / is accessible from GRUB proper at all.
The ZFS in question may be on e.g. SAN. You need to figure parameters in
10_linux, not on boot time.


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature