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
Description: Binary data
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
