[illumos-Developer] [REVIEW] 1303 Grub should support large sector disks

[Hans Rosenfeld]

On Sun, Jul 31, 2011 at 08:07:52PM +0100, Andrew Gabriel wrote:
> >Here is a webrev to fix this: http://cr.illumos.org/view/6qc99xkh/
> >
> >I took the liberty to also do some minor cleanups of the code I had to
> >touch. I tested it on my laptop, which has (at this time) two rpools,
> >one with ashift=12 on a 4k-disk, and another with ashift=9.
> >
> >The patched grub is able to boot both pools, regardless of which disk it
> >was loaded from.
> 
> A couple of (related) observations - don't know if they are really issues...
> 
> Your new code requires that the ashift is present in the nvlist.
> The original code did not require this to exist.
> Are you sure ashift is always present, even on the oldest bootable zpools?
> If not, it might be better to assume ashift=9 if it's missing, rather
> than bailing out.

I'm fairly certain that it always existed. It is documented in the
earliest on-disk-format description I could find.

> The call to vdev_get_params() seems to assume it can only fail due to
> ERR_NO_BOOTPATH, but you added a new reason (no ashift).
> Could this result in a misleading error message, perhaps?

The error value returned is ignored higher up in call stack. The only
message grub will ever print is something like "Filesystem could not be
mounted".


Hans


-- 
%SYSTEM-F-ANARCHISM, The operating system has been overthrown