On 28.01.2012 03:50, Richard Laager wrote: > On Fri, 2012-01-27 at 14:04 -0500, Zachary Bedell wrote: >> > I've had to forward port the other changes necessary to support build on Linux. I attach my zfs-related changed. I'll commit them before your patch and it already covers most of issues > Attached is the work I've done on this front. This includes the "allow > spaces in zpools" patch I previously submitted to grub-devel. And, the > changes in 10_linux.in are from dajhorn's Ubuntu packages. I already commented on 10_linux.in changes. They are pretty sloppy. (mostly is "it works for me and I don't care about other legitimate configs") > > > zfs-on-linux.patch > > > Index: grub/util/grub.d/10_linux.in > =================================================================== > --- grub.orig/util/grub.d/10_linux.in 2012-01-24 23:44:10.530591000 -0600 > +++ grub/util/grub.d/10_linux.in 2012-01-24 23:44:10.706928000 -0600 > @@ -56,8 +56,10 @@ > LINUX_ROOT_DEVICE=UUID=${GRUB_DEVICE_UUID} > fi > > -if [ "x`${grub_probe} --device ${GRUB_DEVICE} --target=fs 2>/dev/null || true`" = xbtrfs ] \ > - || [ "x`stat -f --printf=%T /`" = 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 -o "x${LINUX_ROOT_STAT}" = xbtrfs ]; then > rootsubvol="`make_system_path_relative_to_its_root /`" > rootsubvol="${rootsubvol#/}" > if [ "x${rootsubvol}" != x ]; then > @@ -76,6 +78,10 @@ > GRUB_CMDLINE_EXTRA="$GRUB_CMDLINE_EXTRA crashkernel=384M-2G:64M,2G-:128M" > fi > > +if [ "x${LINUX_ROOT_FS}" = xzfs ]; then > + GRUB_CMDLINE_LINUX="boot=zfs \$bootfs ${GRUB_CMDLINE_LINUX}" > +fi > + > linux_entry () > { > os="$1" > @@ -114,6 +120,12 @@ > fi > printf '%s\n' "${prepare_boot_cache}" > fi > + if [ "x${LINUX_ROOT_FS}" = xzfs ]; then > + cat<< EOF > + insmod zfsinfo > + zfs-bootfs (\$root) bootfs This makes 3 wrong assumptions in a row: - / and /boot may be different. - Linux may be in a non-root subvolume. Then the subvolid points to wrong one. - / may be unaccessible to GRUB altogether. In short: this command line part has to be generated on grub-mkconfig time and have a stable representation. I'd recommend UUID and subvolume name. > +EOF > + fi > if [ "x$5" != "xquiet" ]; then > message="$(gettext_printf "Loading Linux %s ..." ${version})" > cat<< EOF > Index: grub/util/getroot.c > =================================================================== > --- grub.orig/util/getroot.c 2012-01-24 23:44:04.105772000 -0600 > +++ grub/util/getroot.c 2012-01-27 20:48:50.875006000 -0600 > @@ -52,6 +52,8 @@ > #endif > > #ifdef __linux__ > +# include > +# include > # include > # include > #endif > @@ -115,6 +117,8 @@ > return path; > } > > +static char *find_device_from_pool (const char *poolname); > + > #ifdef __linux__ > > #define ESCAPED_PATH_MAX (4 * PATH_MAX) > @@ -263,7 +267,32 @@ > if (!*entries[i].device) > continue; > > - ret = strdup (entries[i].device); > + if (strcmp (entries[i].fstype, "zfs") == 0) > + { > + char *poolname = entries[i].device; > + char *poolname_i = poolname; > + char *poolname_j = poolname; > + /* Replace \040 with a space. Cut at the first slash. */ > + while (*poolname_j) > + { > + if (*poolname_j == '/') > + break; > + if (strncmp (poolname_j, "\\040", 4) == 0) > + { > + *poolname_i = ' '; > + poolname_i++; > + poolname_j += 4; > + continue; > + } > + *poolname_i = *poolname_j; > + poolname_i++; > + poolname_j++; > + } > + *poolname_i = '\0'; > + ret = find_device_from_pool (poolname); > + } > + else > + ret = strdup (entries[i].device); This is the same as what I've done for zfs-fuse except that: - unescaping is done before here since already many revisions. - It forgets subvolume handling. > if (relroot) > *relroot = strdup (entries[i].enc_root); > break; > @@ -280,13 +309,25 @@ > static char * > find_root_device_from_libzfs (const char *dir) > { > - char *device = NULL; > + char *device; > char *poolname; > char *poolfs; > > grub_find_zpool_from_dir (dir,&poolname,&poolfs); > if (! poolname) > return NULL; > + if (poolfs) > + free (poolfs); > + > + device = find_device_from_pool(poolname); > + free(poolname); > + return device; > +} > + > +static char * > +find_device_from_pool (const char *poolname) > +{ > + char *device = NULL; > Same as my fuse work. > #if defined(HAVE_LIBZFS)&& defined(HAVE_LIBNVPAIR) > { > @@ -357,7 +398,7 @@ > char cksum[257], notes[257]; > unsigned int dummy; > > - cmd = xasprintf ("zpool status %s", poolname); > + cmd = xasprintf ("zpool status \"%s\"", poolname); > fp = popen (cmd, "r"); > free (cmd); > > @@ -382,7 +423,10 @@ > st++; > break; > case 1: > - if (!strcmp (name, poolname)) > + /* Use strncmp() because poolname can technically have trailing > + spaces, which the sscanf() above will not catch. Since we've > + asked about this pool specifically, this should be safe. */ > + if (!strncmp (name, poolname, strlen(name))) > st++; > break; > case 2: > @@ -395,17 +439,71 @@ > > free (line); > } > + > +#ifdef __linux__ > + /* The name returned by zpool isn't necessarily directly under /dev. */ > + { > + const char *disk_naming_schemes[] = { > + "/dev/disk/by-id/%s", > + "/dev/disk/by-path/%s", > + "/dev/disk/by-uuid/%s", > + "/dev/disk/by-partuuid/%s", > + "/dev/disk/by-label/%s", > + "/dev/disk/by-partlabel/%s", > + "/dev/%s", Such a list is difficult to maintain. It's better to scan /dev/disk if /dev/ is unavailable. + mnttab = fopen ("/proc/mounts", "r"); + if (! mnttab) + mnttab = fopen ("/etc/mtab", "r"); + if (! mnttab) + return; /etc/mtab is unreliable. As for /proc/mounts : is there a reason to suppose that /proc/mounts would work when /proc/self/mountinfo doesn't > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'φ-coder/phcoder' Serbinenko