Don't create grubenv on ZFS

GRUB can't write to ZFS. Creating a grubenv file leads to a misleading "sparse file not allowed" error on boot. The attached patch for grub-install skips the creation of a grubenv file on ZFS.

Richard

@@ -460,7 +460,9 @@
 grub_device="`"$grub_probe" --device-map="${device_map}" --target=device "${grubdir}"`" || exit 1
 
 if ! test -f "${grubdir}"/grubenv; then
-    "$grub_editenv" "${grubdir}"/grubenv create
+    if ! test "x`"$grub_probe" --target=fs "${grubdir}"`" = xzfs; then
+        "$grub_editenv" "${grubdir}"/grubenv create
+    fi
 fi
 
 # Create the core image. First, auto-detect the filesystem module.

This isn't specific to ZFS. BtrFS, squash4, all variants of cpio, tar and romfs are affected by the same problem. Could you expand your patch to handle those?

Regards
Vladimir 'φ-coder/phcoder' Serbinenko

I suspect that handling them by name is not the best way to do this. How can I tell if a filesystem doesn't support writing?

Richard

I think it's the best way. One could use grub-fstest blocklist but it has other problems. While btrfs and zfs don't hook blocklist on purpose (COW and, more importantly, checksums), others are just almost never sector-aligned or are compressed.

Regards
Vladimir 'φ-coder/phcoder' Serbinenko

I've attached an updated patch.

I also noticed a whitespace inconsistency when looking at another case statement. I've attached a patch.

Both patches are -p0 and have a ChangeLog entry (in a separate file, which I think is the convention).

Richard

@@ -469,7 +469,12 @@
 grub_device="`"$grub_probe" --device-map="${device_map}" --target=device "${grubdir}"`" || exit 1
 
 if ! test -f "${grubdir}"/grubenv; then
-    "$grub_editenv" "${grubdir}"/grubenv create
+    case "`"$grub_probe" --target=fs "${grubdir}"`" in
+        btrfs | cpiofs | newc | odc | romfs | squash4 | tarfs | zfs)
+	    ;;
+        *)
+            "$grub_editenv" "${grubdir}"/grubenv create
+    esac
 fi
 
 # Create the core image. First, auto-detect the filesystem module.

@@ -0,0 +1,5 @@
+2012-02-03  Richard Laager <[email protected]>
+
+	* util/grub-install.in: Don't create a grubenv file on filesystems
+	to which GRUB cannot write.
+

@@ -0,0 +1,4 @@
+2012-02-03  Richard Laager <[email protected]>
+
+	* util/grub-install.in: Fix some inconsistent whitespace.
+

@@ -486,13 +486,13 @@
 # filesystem will be accessible).
 partmap_module=
 for x in `echo "${grub_device}" | xargs "$grub_probe" --device-map="${device_map}" --target=partmap --device 2> /dev/null`; do
-   case "$x" in
-       netbsd | openbsd) 
-	   partmap_module="$partmap_module part_bsd";;
-       "") ;;
-       *)
-	   partmap_module="$partmap_module part_$x";;
-   esac
+    case "$x" in
+        netbsd | openbsd)
+            partmap_module="$partmap_module part_bsd";;
+        "") ;;
+        *)
+            partmap_module="$partmap_module part_$x";;
+    esac
 done
 
 # Device abstraction module, if any (lvm, raid).

I think that not writing /boot/grub/grubenv is a poor work around for the problem of grubenv not being writable.

1: Even if it doesn't now, I would expect an error message when save_env is run and no grubenv is present, so there should be an error message either way.

2: There are uses for grubenv that do not involve writing to it at boot. Fedora's grub2 documentation recommends grub-set-default as the best way to change the default menu entry:
http://fedoraproject.org/wiki/Grub2#Setting_default_entry and if I understand correctly this should continue to work on any filesystem which grub can read.

3: It can't be determined with certainty that grubenv will be writeable at boot. I have run into problems where I installed grub to an SD card and then flipped the write protect switch on the card, which could not have been anticipated at grub-install time. For this reason I think the focus should be on graceful degradation and understandable error messages when save_env is called with a read only grubenv (and possibly there should be ways to disable calls to grubenv in grub-mkconfig, especially in the case of Ubuntu's recordfail modifications).

I do however think that some utilities should warn (but not otherwise change their behavior) when grubenv does not appear to be writable. The prime example of this would be grub-reboot. When grub-reboot is called with a filesytem / abstraction which will prevent grub from writing to the grubenv at boot, grub-reboot cannot possibly function. It already degrades to basically having no effect, so there is no harm in making the changes to grubenv on the off chance the user knows better, but it should produce a clear warning that grub-reboot cannot function without a grubenv writeable by grub at boot.

Jordan Uggla (Jordan_U on irc.freenode.net)

You make good points. I don't have a problem with the idea that save_env should throw an error when it can't actually *save* the grubenv.

So a better way to look at my problem is to say, "I don't want my grub.cfg to call save_env by default."

This then becomes an downstream Ubuntu bug, because my grub.cfg is only calling save_env by default for recordfail. I've submitted a patch to Ubuntu which disables recordfail for these filesystems:
https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/913336/comments/7

So I withdraw zfs-no-grubenv.patch.

Richard