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 >>> >> >
