[caiman-discuss] CR request for: 9026 vtoc partition size is not cylinder size adjusted on disk bigger than 1.96TB

[Jan Damborsky]

Hi Jack,


Jack Schwartz wrote:
> Hi Jan.
>
> On 07/08/09 02:54, Jan Damborsky wrote:
>> Hi Jack,
>>
>> thank you very much for your comments -
>> please see my response in line.
>>
>> Jan
>>
>>
>> Jack Schwartz wrote:
>>> Hi Jan.
>>>
>>> Here are my comments:
>>>
>>> ti_dm.c:
>>>
>>> 284: Seems to me that nsecs is best defined as a diskaddr_t instead 
>>> of a uint32_t. 
>>
>> I am not quite convinced that we need to use diskaddr_t for nsecs,
>> since nsecs variable doesn't hold sector address, but it contains
>> different kind of information - number of sectors per cylinder.
>> This is calculated as
>>
>> nsecs = (uint32_t)geom.dkg_nhead * geom.dkg_nsect
>>
>> and both multiplicands are 'unsigned short' with size of 16 bits:
>> http://docs.sun.com/app/docs/doc/820-7598/bjbdq?a=view
>>
>> And product will always fit into 32bits.
> OK. uint32_t is fine.
>
> <snip>
>>> 345, 362, 379: "i" is still an int.  The first format specifier here 
>>> should be %d
>>
>> Agreed - looking at those lines, '%d' is used
>> as format specifier for "i". It doesn't seem
>> that changes are needed.
> Yes, you are correct.  My mistake; sorry for the noise.

No problem at all :-)

Thank you very much for your time to review those changes !
Jan

>
> Looks fine.
>
>    Thanks,
>    Jack
>
>>
>>>
>>> ti_dm.h: good eye!
>>
>> Thank you :-)
>>
>>>
>>> The rest looks fine to me.
>>
>> Thanks a lot again !
>> Jan
>>
>>>
>>>    Thanks,
>>>    Jack
>>>
>>>
>>>
>>>
>>> On 07/07/09 08:42, Jan Damborsky wrote:
>>>> Hi,
>>>>
>>>> could I please ask for reviewing simple change for following bug ?
>>>>
>>>> 9026 vtoc partition size is not cylinder size adjusted on disk 
>>>> bigger than 1.96TB
>>>>
>>>> webrev:
>>>> http://cr.opensolaris.org/~dambi/bug-9026
>>>>
>>>> While I was there, I have also fixed following issues:
>>>>
>>>> * error message in ti_zfm.c (thanks Greg Onufer for catching this).
>>>>  It had ZFS volume name and pool name swapped around.
>>>>
>>>> * typo in idm_cyls_to_mbs macro definition in ti_dm.h
>>>>
>>>> Thank you very much,
>>>> Jan
>>>>
>>>>
>>>> modules affected:
>>>> -----------------
>>>> * libti
>>>>
>>>>
>>>> testing done
>>>> ------------
>>>>
>>>> [1] LiveCD - test of fix
>>>> ----------------------------
>>>> * HW: - Ultra 20 (1GB RWM)
>>>>      - target disk: external USB 4TB
>>>>
>>>> * SW: - created LiveCD ISO based on build 117 (contains fix for 
>>>> 6843138 GRUB issue)
>>>>        with modified libti
>>>>
>>>> * result:
>>>>  - installation succeeded (no adjustments done by TI to created VTOC)
>>>>  - system booted to desktop
>>>>  - format correctly recognized created VTOC
>>>>
>>>> [2] SPARC AI - regression test
>>>> ------------------------------
>>>> * HW: - sun4v T1000 (2GB RWM - LDOM control domain)
>>>>      - target disk: 73GB
>>>>
>>>> * SW: AI image based on build 117 with modified libti
>>>>
>>>> * result:
>>>>  - installation succeeded (no adjustments done by TI to created VTOC)
>>>>  - system booted
>>>>  - format correctly recognized created VTOC
>>>>
>>>> _______________________________________________
>>>> caiman-discuss mailing list
>>>> caiman-discuss at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>
>>
>