[Patch] Enable libzfs detection on Linux

[Patch] Enable libzfs detection on Linux

[Zachary Bedell]

[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]

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.

grub.d/10_linux.in:
 * Detect ZFS root and include reference to zfsinfo.mod if needed.
 * Detect the bootfs dataset from the zpool attribute.
 * Adjust kernel parameters to omit root= and replace with boot=zfs and reference to the detected bootfs if available.
 * If bootfs isn't available from the pool, delegates to detection (hopefully) present in the initrd.


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.

The attached patch is intended for trunk, but I also have a version rolled for the 1.99 branch if that would be useful.

Best regards,
Zac Bedell


[-- Attachment #2: grub-zfs-linux.patch --]
[-- Type: application/octet-stream, Size: 5420 bytes --]

diff --git a/configure.ac b/configure.ac
index e6d7265..3137869 100644
--- a/configure.ac
+++ b/configure.ac
@@ -295,9 +295,14 @@ else
   AC_PATH_PROG(HELP2MAN, help2man)
 fi
 
+# The Solaris Portability Layer is required to link grub against zfs-lib on Linux.
+AC_CHECK_LIB([spl], [getextmntent], [ LIBS="$LIBS -pthread -lspl"
+  AC_DEFINE([HAVE_LIBSPL], [1], [Define to 1 if you have the SPL library.])],)
+
 # Check for functions and headers.
 AC_CHECK_FUNCS(posix_memalign memalign asprintf vasprintf getextmntent)
-AC_CHECK_HEADERS(libzfs.h libnvpair.h sys/param.h sys/mount.h sys/mnttab.h sys/mkdev.h)
+AC_CHECK_HEADERS(libzfs.h libnvpair.h sys/param.h sys/mount.h sys/mnttab.h sys/mkdev.h,[],[],[$ac_includes_default
+#include <zfs-linux/zfs_config.h>])
 
 AC_CHECK_MEMBERS([struct statfs.f_fstypename],,,[$ac_includes_default
 #include <sys/param.h>
@@ -922,17 +927,19 @@ AC_CHECK_LIB([lzma], [lzma_code],
                         [Define to 1 if you have the LZMA library.])],)
 AC_SUBST([LIBLZMA])
 
-AC_CHECK_LIB([zfs], [libzfs_init],
-             [LIBZFS="-lzfs"
-              AC_DEFINE([HAVE_LIBZFS], [1],
-                        [Define to 1 if you have the ZFS library.])],)
-AC_SUBST([LIBZFS])
+# These libraries and zpool below are external to libzfs on Linux,
+# but usually internal or intrinsic on other platforms.
+AC_CHECK_LIB([avl], [avl_create], [LIBS="$LIBS -lavl"])
+AC_CHECK_LIB([efi], [efi_alloc_and_init], [LIBS="$LIBS -lefi"])
+AC_CHECK_LIB([unicode], [u8_strcmp], [LIBS="$LIBS -lunicode"])
 
-AC_CHECK_LIB([nvpair], [nvlist_print],
-             [LIBNVPAIR="-lnvpair"
-              AC_DEFINE([HAVE_LIBNVPAIR], [1],
-                        [Define to 1 if you have the NVPAIR library.])],)
+AC_CHECK_LIB([nvpair], [nvlist_print], [LIBS="$LIBS -lnvpair" LIBNVPAIR="$LIBS"
+  AC_DEFINE([HAVE_LIBNVPAIR], [1], [Define to 1 if you have the NVPAIR library.])],)
 AC_SUBST([LIBNVPAIR])
+AC_CHECK_LIB([zpool], [zfs_prop_init], [LIBS="$LIBS -lzpool"])
+AC_CHECK_LIB([zfs], [libzfs_init], [LIBS="$LIBS -lzfs" LIBZFS="$LIBS"
+  AC_DEFINE([HAVE_LIBZFS], [1], [Define to 1 if you have the ZFS library.])],)
+AC_SUBST([LIBZFS])
 
 LIBS=""
 
diff --git a/util/getroot.c b/util/getroot.c
index 7106458..0282a3d 100644
--- a/util/getroot.c
+++ b/util/getroot.c
@@ -34,6 +34,17 @@
 #include <stdint.h>
 #include <grub/util/misc.h>
 #include <grub/cryptodisk.h>
+#if defined(HAVE_LIBSPL) && defined(__linux__)
+# include <sys/ioctl.h>
+/*
+ * The Solaris Compatibility Layer provides getextmntent on Linux, which is
+ * required for grub-probe to recognize a native ZFS root filesystem on
+ * a Linux system. This typedef is required because including the SPL
+ * types.h here conflicts with an earlier Linux types.h inclusion.
+ */
+  typedef unsigned int uint_t;
+# include <libspl/sys/mnttab.h>
+#endif
 
 #ifdef HAVE_DEVICE_MAPPER
 # include <libdevmapper.h>
@@ -598,16 +609,16 @@ grub_guess_root_device (const char *dir)
   struct stat st;
   dev_t dev;
 
-#ifdef __linux__
-  if (!os_dev)
-    os_dev = grub_find_root_device_from_mountinfo (dir, NULL);
-#endif /* __linux__ */
-
 #if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR)
   if (!os_dev)
     os_dev = find_root_device_from_libzfs (dir);
 #endif
 
+#ifdef __linux__
+  if (!os_dev)
+    os_dev = grub_find_root_device_from_mountinfo (dir, NULL);
+#endif /* __linux__ */
+
   if (os_dev)
     {
       char *tmp = os_dev;
@@ -1407,7 +1418,15 @@ grub_find_zpool_from_dir (const char *dir, char **poolname, char **poolfs)
     if (stat (dir, &st) != 0)
       return;
 
-    FILE *mnttab = fopen ("/etc/mnttab", "r");
+    FILE *mnttab;
+    mnttab = fopen ("/proc/mounts", "r");
+
+    if (! mnttab)
+      mnttab = fopen ("/etc/mtab", "r");
+
+    if (! mnttab)
+      mnttab = fopen ("/etc/mnttab", "r");
+
     if (! mnttab)
       return;
 
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 97e7c65..20b204f 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -51,7 +51,10 @@ else
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
 fi
 
-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
   rootsubvol="`make_system_path_relative_to_its_root /`"
   rootsubvol="${rootsubvol#/}"
   if [ "x${rootsubvol}" != x ]; then
@@ -59,6 +62,10 @@ if [ "x`${grub_probe} --device ${GRUB_DEVICE} --target=fs 2>/dev/null || true`"
   fi
 fi
 
+if [ "x${LINUX_ROOT_FS}" = xzfs ]; then
+  GRUB_CMDLINE_LINUX="boot=zfs \$bootfs ${GRUB_CMDLINE_LINUX}"
+fi
+
 linux_entry ()
 {
   os="$1"
@@ -113,10 +120,20 @@ EOF
     fi
     printf '%s\n' "${prepare_boot_cache}"
   fi
+  if [ "x${LINUX_ROOT_FS}" = xzfs ]; then
+    cat << EOF
+	insmod zfsinfo
+	zfs-bootfs (\$root) bootfs
+EOF
+    # ZFS doesn't want root=... or read-only.
+    rootentry=""
+  else
+    rootentry="root=${linux_root_device_thisversion} ro"
+  fi
   message="$(gettext_printf "Loading Linux %s ..." ${version})"
   cat << EOF
 	echo	'$message'
-	linux	${rel_dirname}/${basename} root=${linux_root_device_thisversion} ro ${args}
+	linux	${rel_dirname}/${basename} ${rootentry} ${args}
 EOF
   if test -n "${initrd}" ; then
     message="$(gettext_printf "Loading initial ramdisk ...")"

Re: [Patch] Enable libzfs detection on Linux

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 3472 bytes --]

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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] Enable libzfs detection on Linux

[Zachary Bedell]

[-- Attachment #1: Type: text/plain, Size: 3632 bytes --]

Finally getting back to this & trying address concerns below:

On Aug 18, 2011, at 12:49 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> On 09.08.2011 19:48, Zachary Bedell wrote:
>> 
>> * 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).

Easy enough to remove mtab.

>> 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 I'm reading the pre-existing ifdef's there, the code added for /proc/mounts wouldn't apply on FreeBSD (assuming the comments there aren't lying).  The ifdef around line 1399:

#if defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) && defined(HAVE_STRUCT_STATFS_F_MNTFROMNAME)
  /* FreeBSD and GNU/kFreeBSD.  */

would be hit on BSD and thus exclude the /proc/mounts code.  That said, easy enough to add an extra '#ifdef __linux__' around the proc code so it doesn't fire on Solaris and yank the mtab code.

>> -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.

Looking back, I think this may have been the result of forward porting this patch from an older Grub codebase.  I've changed it to restore the original btrfs logic from trunk.

>> +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.

Taking a closer look at these, I don't think they're necessary with the initrd scripts used by the current Linux implementations (one in Ubuntu & the Dracut scripts in the ZFSonLinux distribution being the two I'm aware of).  The only thing required in the second half of 10_linux was the change to exclude root=… and the ro attribute for ZFS.  Once grub loads kernel & initrd, the initrd does the work of finding the root pool and importing it using the full ZFS kernel module, so there's no need to use Grub's logic to do the same.

Remixed patch is attached.


[-- Attachment #2: grub-zfs-linux.patch --]
[-- Type: application/octet-stream, Size: 5610 bytes --]

diff --git a/configure.ac b/configure.ac
index e6d7265..3137869 100644
--- a/configure.ac
+++ b/configure.ac
@@ -295,9 +295,14 @@ else
   AC_PATH_PROG(HELP2MAN, help2man)
 fi
 
+# The Solaris Portability Layer is required to link grub against zfs-lib on Linux.
+AC_CHECK_LIB([spl], [getextmntent], [ LIBS="$LIBS -pthread -lspl"
+  AC_DEFINE([HAVE_LIBSPL], [1], [Define to 1 if you have the SPL library.])],)
+
 # Check for functions and headers.
 AC_CHECK_FUNCS(posix_memalign memalign asprintf vasprintf getextmntent)
-AC_CHECK_HEADERS(libzfs.h libnvpair.h sys/param.h sys/mount.h sys/mnttab.h sys/mkdev.h)
+AC_CHECK_HEADERS(libzfs.h libnvpair.h sys/param.h sys/mount.h sys/mnttab.h sys/mkdev.h,[],[],[$ac_includes_default
+#include <zfs-linux/zfs_config.h>])
 
 AC_CHECK_MEMBERS([struct statfs.f_fstypename],,,[$ac_includes_default
 #include <sys/param.h>
@@ -922,17 +927,19 @@ AC_CHECK_LIB([lzma], [lzma_code],
                         [Define to 1 if you have the LZMA library.])],)
 AC_SUBST([LIBLZMA])
 
-AC_CHECK_LIB([zfs], [libzfs_init],
-             [LIBZFS="-lzfs"
-              AC_DEFINE([HAVE_LIBZFS], [1],
-                        [Define to 1 if you have the ZFS library.])],)
-AC_SUBST([LIBZFS])
+# These libraries and zpool below are external to libzfs on Linux,
+# but usually internal or intrinsic on other platforms.
+AC_CHECK_LIB([avl], [avl_create], [LIBS="$LIBS -lavl"])
+AC_CHECK_LIB([efi], [efi_alloc_and_init], [LIBS="$LIBS -lefi"])
+AC_CHECK_LIB([unicode], [u8_strcmp], [LIBS="$LIBS -lunicode"])
 
-AC_CHECK_LIB([nvpair], [nvlist_print],
-             [LIBNVPAIR="-lnvpair"
-              AC_DEFINE([HAVE_LIBNVPAIR], [1],
-                        [Define to 1 if you have the NVPAIR library.])],)
+AC_CHECK_LIB([nvpair], [nvlist_print], [LIBS="$LIBS -lnvpair" LIBNVPAIR="$LIBS"
+  AC_DEFINE([HAVE_LIBNVPAIR], [1], [Define to 1 if you have the NVPAIR library.])],)
 AC_SUBST([LIBNVPAIR])
+AC_CHECK_LIB([zpool], [zfs_prop_init], [LIBS="$LIBS -lzpool"])
+AC_CHECK_LIB([zfs], [libzfs_init], [LIBS="$LIBS -lzfs" LIBZFS="$LIBS"
+  AC_DEFINE([HAVE_LIBZFS], [1], [Define to 1 if you have the ZFS library.])],)
+AC_SUBST([LIBZFS])
 
 LIBS=""
 
diff --git a/util/getroot.c b/util/getroot.c
index 7106458..592a02f 100644
--- a/util/getroot.c
+++ b/util/getroot.c
@@ -34,6 +34,17 @@
 #include <stdint.h>
 #include <grub/util/misc.h>
 #include <grub/cryptodisk.h>
+#if defined(HAVE_LIBSPL) && defined(__linux__)
+# include <sys/ioctl.h>
+/*
+ * The Solaris Compatibility Layer provides getextmntent on Linux, which is
+ * required for grub-probe to recognize a native ZFS root filesystem on
+ * a Linux system. This typedef is required because including the SPL
+ * types.h here conflicts with an earlier Linux types.h inclusion.
+ */
+  typedef unsigned int uint_t;
+# include <libspl/sys/mnttab.h>
+#endif
 
 #ifdef HAVE_DEVICE_MAPPER
 # include <libdevmapper.h>
@@ -598,16 +609,16 @@ grub_guess_root_device (const char *dir)
   struct stat st;
   dev_t dev;
 
-#ifdef __linux__
-  if (!os_dev)
-    os_dev = grub_find_root_device_from_mountinfo (dir, NULL);
-#endif /* __linux__ */
-
 #if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR)
   if (!os_dev)
     os_dev = find_root_device_from_libzfs (dir);
 #endif
 
+#ifdef __linux__
+  if (!os_dev)
+    os_dev = grub_find_root_device_from_mountinfo (dir, NULL);
+#endif /* __linux__ */
+
   if (os_dev)
     {
       char *tmp = os_dev;
@@ -1399,7 +1410,7 @@ grub_find_zpool_from_dir (const char *dir, char **poolname, char **poolfs)
     *poolname = xstrdup (mnt.f_mntfromname);
   }
 #elif defined(HAVE_GETEXTMNTENT)
-  /* Solaris.  */
+  /* Solaris and ZFSonLinux (but not FUSE).  */
   {
     struct stat st;
     struct extmnttab mnt;
@@ -1407,7 +1418,17 @@ grub_find_zpool_from_dir (const char *dir, char **poolname, char **poolfs)
     if (stat (dir, &st) != 0)
       return;
 
-    FILE *mnttab = fopen ("/etc/mnttab", "r");
+    FILE *mnttab = NULL;
+
+#ifdef __linux__
+    /* Look in proc only for Linux.  Solaris (and anything else with 
+       HAVE_GETEXTMNTENT) won't need it. */
+    mnttab = fopen ("/proc/mounts", "r");
+#endif
+
+    if (! mnttab)
+      mnttab = fopen ("/etc/mnttab", "r");
+
     if (! mnttab)
       return;
 
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 97e7c65..5624607 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -51,12 +51,15 @@ else
   LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
 fi
 
-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`
+if [ "x${LINUX_ROOT_FS}" = xbtrfs ] ; then
   rootsubvol="`make_system_path_relative_to_its_root /`"
   rootsubvol="${rootsubvol#/}"
   if [ "x${rootsubvol}" != x ]; then
     GRUB_CMDLINE_LINUX="rootflags=subvol=${rootsubvol} ${GRUB_CMDLINE_LINUX}"
   fi
+elif [ "x${LINUX_ROOT_FS}" = xzfs ]; then
+  GRUB_CMDLINE_LINUX="boot=zfs ${GRUB_CMDLINE_LINUX}"
 fi
 
 linux_entry ()
@@ -113,10 +116,16 @@ EOF
     fi
     printf '%s\n' "${prepare_boot_cache}"
   fi
+  if [ "x${LINUX_ROOT_FS}" = xzfs ]; then
+    # ZFS doesn't want root=... or read-only.
+    rootentry=""
+  else
+    rootentry="root=${linux_root_device_thisversion} ro"
+  fi
   message="$(gettext_printf "Loading Linux %s ..." ${version})"
   cat << EOF
 	echo	'$message'
-	linux	${rel_dirname}/${basename} root=${linux_root_device_thisversion} ro ${args}
+	linux	${rel_dirname}/${basename} ${rootentry} ${args}
 EOF
   if test -n "${initrd}" ; then
     message="$(gettext_printf "Loading initial ramdisk ...")"

[-- Attachment #3: Type: text/plain, Size: 654 bytes --]



FWIW, my commit comment locally for this was:
 * Adjusts autoconf logic to properly detect libzfs on Linux.
 * Includes additional headers necessary for libspl.
 * Changes order that filesystems are detected in to allow ZFS a chance to be found.
 * Add necessary boot parameters & detection logic to grub.d script for Linux.

Sorry for the interminable delay in getting back to this.  Debugging this took a rather "creative" turn as I found a few more cases where pools that worked in the ZFS driver were rejected by Grub.  I have a second patch that fixes a number of these cases which I'll be posting shortly.

Best regards,
Zac Bedell

Re: [Patch] Enable libzfs detection on Linux

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 11005 bytes --]

On 14.09.2011 20:39, Zachary Bedell wrote:
> Finally getting back to this & trying address concerns below:
>
> On Aug 18, 2011, at 12:49 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>> > On 09.08.2011 19:48, Zachary Bedell wrote:
>>> >> 
>>> >> * 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).
> Easy enough to remove mtab.
>
>>> >> 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 I'm reading the pre-existing ifdef's there, the code added for /proc/mounts wouldn't apply on FreeBSD (assuming the comments there aren't lying).  The ifdef around line 1399:
>
> #if defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) && defined(HAVE_STRUCT_STATFS_F_MNTFROMNAME)
>   /* FreeBSD and GNU/kFreeBSD.  */
>
> would be hit on BSD and thus exclude the /proc/mounts code.  That said, easy enough to add an extra '#ifdef __linux__' around the proc code so it doesn't fire on Solaris and yank the mtab code.
>
>>> >> -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.
> Looking back, I think this may have been the result of forward porting this patch from an older Grub codebase.  I've changed it to restore the original btrfs logic from trunk.
>
>>> >> +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.
> Taking a closer look at these, I don't think they're necessary with the initrd scripts used by the current Linux implementations (one in Ubuntu & the Dracut scripts in the ZFSonLinux distribution being the two I'm aware of).  The only thing required in the second half of 10_linux was the change to exclude root=… and the ro attribute for ZFS.  Once grub loads kernel & initrd, the initrd does the work of finding the root pool and importing it using the full ZFS kernel module, so there's no need to use Grub's logic to do the same.
>
> Remixed patch is attached.
>
> grub-zfs-linux.patch
>
>
> diff --git a/configure.ac b/configure.ac
> index e6d7265..3137869 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -295,9 +295,14 @@ else
>    AC_PATH_PROG(HELP2MAN, help2man)
>  fi
>  
> +# The Solaris Portability Layer is required to link grub against zfs-lib on Linux.
> +AC_CHECK_LIB([spl], [getextmntent], [ LIBS="$LIBS -pthread -lspl"
> +  AC_DEFINE([HAVE_LIBSPL], [1], [Define to 1 if you have the SPL library.])],)
> +
>  # Check for functions and headers.
>  AC_CHECK_FUNCS(posix_memalign memalign asprintf vasprintf getextmntent)
> -AC_CHECK_HEADERS(libzfs.h libnvpair.h sys/param.h sys/mount.h sys/mnttab.h sys/mkdev.h)
> +AC_CHECK_HEADERS(libzfs.h libnvpair.h sys/param.h sys/mount.h sys/mnttab.h sys/mkdev.h,[],[],[$ac_includes_default
> +#include <zfs-linux/zfs_config.h>])
>  
This will fail on Solaris or FreeBSD due to missing zfs-linux/zfs_config.h.
>  AC_CHECK_MEMBERS([struct statfs.f_fstypename],,,[$ac_includes_default
>  #include <sys/param.h>
> @@ -922,17 +927,19 @@ AC_CHECK_LIB([lzma], [lzma_code],
>                          [Define to 1 if you have the LZMA library.])],)
>  AC_SUBST([LIBLZMA])
>  
> -AC_CHECK_LIB([zfs], [libzfs_init],
> -             [LIBZFS="-lzfs"
> -              AC_DEFINE([HAVE_LIBZFS], [1],
> -                        [Define to 1 if you have the ZFS library.])],)
> -AC_SUBST([LIBZFS])
> +# These libraries and zpool below are external to libzfs on Linux,
> +# but usually internal or intrinsic on other platforms.
> +AC_CHECK_LIB([avl], [avl_create], [LIBS="$LIBS -lavl"])
> +AC_CHECK_LIB([efi], [efi_alloc_and_init], [LIBS="$LIBS -lefi"])
> +AC_CHECK_LIB([unicode], [u8_strcmp], [LIBS="$LIBS -lunicode"])
>  
Why do you need those libraries? I see no reference to these functions.
> -AC_CHECK_LIB([nvpair], [nvlist_print],
> -             [LIBNVPAIR="-lnvpair"
> -              AC_DEFINE([HAVE_LIBNVPAIR], [1],
> -                        [Define to 1 if you have the NVPAIR library.])],)
> +AC_CHECK_LIB([nvpair], [nvlist_print], [LIBS="$LIBS -lnvpair" LIBNVPAIR="$LIBS"
> +  AC_DEFINE([HAVE_LIBNVPAIR], [1], [Define to 1 if you have the NVPAIR library.])],)
>  AC_SUBST([LIBNVPAIR])
> +AC_CHECK_LIB([zpool], [zfs_prop_init], [LIBS="$LIBS -lzpool"])
> +AC_CHECK_LIB([zfs], [libzfs_init], [LIBS="$LIBS -lzfs" LIBZFS="$LIBS"
> +  AC_DEFINE([HAVE_LIBZFS], [1], [Define to 1 if you have the ZFS library.])],)
> +AC_SUBST([LIBZFS])
>  
>  LIBS=""
>  
> diff --git a/util/getroot.c b/util/getroot.c
> index 7106458..592a02f 100644
> --- a/util/getroot.c
> +++ b/util/getroot.c
> @@ -34,6 +34,17 @@
>  #include <stdint.h>
>  #include <grub/util/misc.h>
>  #include <grub/cryptodisk.h>
> +#if defined(HAVE_LIBSPL) && defined(__linux__)
> +# include <sys/ioctl.h>
> +/*
> + * The Solaris Compatibility Layer provides getextmntent on Linux, which is
> + * required for grub-probe to recognize a native ZFS root filesystem on
> + * a Linux system. This typedef is required because including the SPL
> + * types.h here conflicts with an earlier Linux types.h inclusion.
> + */
> +  typedef unsigned int uint_t;
> +# include <libspl/sys/mnttab.h>
> +#endif
>  
>  #ifdef HAVE_DEVICE_MAPPER
>  # include <libdevmapper.h>
> @@ -598,16 +609,16 @@ grub_guess_root_device (const char *dir)
>    struct stat st;
>    dev_t dev;
>  
> -#ifdef __linux__
> -  if (!os_dev)
> -    os_dev = grub_find_root_device_from_mountinfo (dir, NULL);
> -#endif /* __linux__ */
> -
>  #if defined(HAVE_LIBZFS) && defined(HAVE_LIBNVPAIR)
>    if (!os_dev)
>      os_dev = find_root_device_from_libzfs (dir);
>  #endif
>  
> +#ifdef __linux__
> +  if (!os_dev)
> +    os_dev = grub_find_root_device_from_mountinfo (dir, NULL);
> +#endif /* __linux__ */
> +
>    if (os_dev)
>      {
>        char *tmp = os_dev;
> @@ -1399,7 +1410,7 @@ grub_find_zpool_from_dir (const char *dir, char **poolname, char **poolfs)
>      *poolname = xstrdup (mnt.f_mntfromname);
>    }
>  #elif defined(HAVE_GETEXTMNTENT)
> -  /* Solaris.  */
> +  /* Solaris and ZFSonLinux (but not FUSE).  */
>    {
>      struct stat st;
>      struct extmnttab mnt;
> @@ -1407,7 +1418,17 @@ grub_find_zpool_from_dir (const char *dir, char **poolname, char **poolfs)
>      if (stat (dir, &st) != 0)
>        return;
>  
> -    FILE *mnttab = fopen ("/etc/mnttab", "r");
> +    FILE *mnttab = NULL;
> +
> +#ifdef __linux__
> +    /* Look in proc only for Linux.  Solaris (and anything else with 
> +       HAVE_GETEXTMNTENT) won't need it. */
> +    mnttab = fopen ("/proc/mounts", "r");
> +#endif
> +
> +    if (! mnttab)
> +      mnttab = fopen ("/etc/mnttab", "r");
> +
>      if (! mnttab)
>        return;
>  
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index 97e7c65..5624607 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -51,12 +51,15 @@ else
>    LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID}
>  fi
>  
> -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`
> +if [ "x${LINUX_ROOT_FS}" = xbtrfs ] ; then
>    rootsubvol="`make_system_path_relative_to_its_root /`"
>    rootsubvol="${rootsubvol#/}"
>    if [ "x${rootsubvol}" != x ]; then
>      GRUB_CMDLINE_LINUX="rootflags=subvol=${rootsubvol} ${GRUB_CMDLINE_LINUX}"
>    fi
> +elif [ "x${LINUX_ROOT_FS}" = xzfs ]; then
> +  GRUB_CMDLINE_LINUX="boot=zfs ${GRUB_CMDLINE_LINUX}"
>  fi
>  
>  linux_entry ()
> @@ -113,10 +116,16 @@ EOF
>      fi
>      printf '%s\n' "${prepare_boot_cache}"
>    fi
> +  if [ "x${LINUX_ROOT_FS}" = xzfs ]; then
> +    # ZFS doesn't want root=... or read-only.
> +    rootentry=""
> +  else
> +    rootentry="root=${linux_root_device_thisversion} ro"
> +  fi
>    message="$(gettext_printf "Loading Linux %s ..." ${version})"
>    cat << EOF
>  	echo	'$message'
> -	linux	${rel_dirname}/${basename} root=${linux_root_device_thisversion} ro ${args}
> +	linux	${rel_dirname}/${basename} ${rootentry} ${args}
>  EOF
>    if test -n "${initrd}" ; then
>      message="$(gettext_printf "Loading initial ramdisk ...")"
>
>
>
>
> FWIW, my commit comment locally for this was:
>  * Adjusts autoconf logic to properly detect libzfs on Linux.
>  * Includes additional headers necessary for libspl.
>  * Changes order that filesystems are detected in to allow ZFS a chance to be found.
>  * Add necessary boot parameters & detection logic to grub.d script for Linux.
>
> Sorry for the interminable delay in getting back to this.  Debugging this took a rather "creative" turn as I found a few more cases where pools that worked in the ZFS driver were rejected by Grub.  I have a second patch that fixes a number of these cases which I'll be posting shortly.
>
> Best regards,
> Zac Bedell
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] Enable libzfs detection on Linux

[Robert Millan]

Hi Zachary,

2011/9/14 Zachary Bedell <pendorbound@gmail.com>:
> FWIW, my commit comment locally for this was:
>  * Adjusts autoconf logic to properly detect libzfs on Linux.
>  * Includes additional headers necessary for libspl.

Excuse me if I missed something, but weren't you holding the position
that libzfs ABI was too unstable and relying on it from external
programs was a bad idea?

Recently Debian has had severe problems in this area because of this.
C.f. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=645305

Last month I sent a proof of concept patch that implements this idea
(in "getroot for ZFS without libzfs?" thread).  Did you see this part
of the thread?

-- 
Robert Millan

Re: [Patch] Enable libzfs detection on Linux

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

On 10.11.2011 21:02, Robert Millan wrote:
> Hi Zachary,
>
> 2011/9/14 Zachary Bedell <pendorbound@gmail.com>:
>> FWIW, my commit comment locally for this was:
>>  * Adjusts autoconf logic to properly detect libzfs on Linux.
>>  * Includes additional headers necessary for libspl.
> Excuse me if I missed something, but weren't you holding the position
> that libzfs ABI was too unstable and relying on it from external
> programs was a bad idea?
>
> Recently Debian has had severe problems in this area because of this.
> C.f. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=645305
>
> Last month I sent a proof of concept patch that implements this idea
> (in "getroot for ZFS without libzfs?" thread).  Did you see this part
> of the thread?
The patch has several problems. It seems to definitely commit us to the
improper assumption of uniqueness of ZFS names. Even when there are
several zpools with the same name you can still mount them by unique ID.
Or a devices with conflicting name could have been added after one pool
was mounted.
Also it goes against the general direction towards deprecation of both
device.map and complete scanning of devices in both tools and proper (it
creates problems like floppy seeks or hangs with some drivers) (see my
series of patches for "pulling" devices and lazy scans). If zpool has
something similar to mdadm --export we can use it.


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] Enable libzfs detection on Linux

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

On 10.11.2011 21:02, Robert Millan wrote:
> Hi Zachary,
>
> 2011/9/14 Zachary Bedell <pendorbound@gmail.com>:
>> FWIW, my commit comment locally for this was:
>>  * Adjusts autoconf logic to properly detect libzfs on Linux.
>>  * Includes additional headers necessary for libspl.
> Excuse me if I missed something, but weren't you holding the position
> that libzfs ABI was too unstable and relying on it from external
> programs was a bad idea?
>
> Recently Debian has had severe problems in this area because of this.
> C.f. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=645305
>
> Last month I sent a proof of concept patch that implements this idea
> (in "getroot for ZFS without libzfs?" thread).  Did you see this part
> of the thread?
And I dislike this pseudo device.


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] Enable libzfs detection on Linux

[Seth Goldberg]

On 11/10/11 12:02, Robert Millan wrote:
> Hi Zachary,
>
> 2011/9/14 Zachary Bedell<pendorbound@gmail.com>:
>> FWIW, my commit comment locally for this was:
>>   * Adjusts autoconf logic to properly detect libzfs on Linux.
>>   * Includes additional headers necessary for libspl.
>
> Excuse me if I missed something, but weren't you holding the position
> that libzfs ABI was too unstable and relying on it from external
> programs was a bad idea?
>
> Recently Debian has had severe problems in this area because of this.
> C.f. http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=645305
>
> Last month I sent a proof of concept patch that implements this idea
> (in "getroot for ZFS without libzfs?" thread).  Did you see this part
> of the thread?
>

Hi,

   I had a conversation with Vladimir about this.  I think retaining 
libzfs is a good thing, Debian bugs notwithstanding.  libzfs provides an 
interfaces that enables a consumer to find zpool on system without 
scanning the world (the idea being that the kernel has already done 
that, and has cached that information), so one need not have to deal 
with devices that might become unresponsive to a user-level process when 
scanned.  It also allows one to enumerate pools that might have the same 
name (in which case you'd use the pool's GUID (in base 10) to access the 
pool).  So libzfs, whatever its stability level, still seems like a 
better plan than going out to disks directly and pulling metadata.

(FWIW, I looked at that Debian bug, and IMHO, the problem there was that 
libzfs wasn't properly versioned in the first place-- if the person who 
added or changed the libzfs API didn't version it, they're just asking 
for trouble.  The proper solution in that case would have been adding 
versioning and not pulling the whole library).

  --S