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

[Garrett D'Amore]

On 07/29/2011 02:17 PM, Albert Lee wrote:
> On Fri, Jul 29, 2011 at 12:47 PM, Garrett D'Amore<garrett at damore.org>  wrote:
>> On 07/29/2011 08:47 AM, 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.
>>>
>>>
>>> Hans
>>>
>> This looks really very good.  Nice job Hans!  Do you want to start an RTI
>> for this?  (Would be good to get another reviewer as well.)
>>
>>     - Garrett
>>
> Code looks fine, but stylistically it seems strange to have ub_array
> now declared as char * in various places, with the funny iterator and
> and implicitly casting to uberblock_phys_t * rather than explicitly
> casting to char *.

Actually, I like the way this is done... the uberblock array members are 
now variable sized (because of the value of ashift may vary), so using 
char * is the best way to increment.

The one possible problem I see is a lint warning that could occur when 
you cast char * to something else -- it complains about the possibility 
for a misalignment.  The way to fix that is to cast through void *.

I don't think we lint grub though. ;-)

     - Garrett

> -Albert
>
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer