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

[Andrew Gabriel]

Hans Rosenfeld wrote:
> Hi,
>
> our Grub currently fails to boot large sector disks (with ashift != 9).
>
> 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.

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?

-- 
Andrew