Dave,

Dave Miner wrote:
> jan damborsky wrote:
>> Hi Dave,
>>
>> based on the yesterday's discussion during Caiman meeting,
>> I am assuming that following bug is to be qualified as
>> 2008.11 stopper. This fix addresses scenario Rafal
>> reported in bug 4872.
>>
>> Could you please let me know, if my understanding is correct ?
>>
>
> Yes, I've approved it.

Thank you

>
>> If this is the case, could I please ask two people to review
>> those changes ?
>>
>> 4980 Installer should convert legacy Solaris partition (0x82) to 
>> Solaris2 one (0xbf)
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4980
>>
>> The webrev is available at:
>> http://cr.opensolaris.org/~dambi/bug-4980
>>
>
> 131: s/SUNW/SUN/

Done.

>
>
> 136: s/one// 

Done.

>
>
> 153: it's not at all obvious to me why "0" is the right value to use 
> here for the handle; perhaps a comment, at least. 

'handle' parameter is currently unused
in this function - I have put appropriate
comment there explaining this.

>   I also would have expected to check the return value on this call, 
> not just whether committed_disk_target == NULL; that's not explicitly 
> guaranteed to be its state in error cases, though I agree that it is 
> in the current implementation.

This is a good suggestion - I have modified
the code, so that it checks for return value.
Since om_set_disk_partition_info() returns
OM_FAILURE in all cases when committed_disk_target
is set to NULL, 'committed_disk_target == NULL'
check became redundant and was removed.

Updated webrev with all changes has been published
at the same location.

Thank you very much for review,
Jan

>
> Dave
>> Thank you,
>> Jan
>>
>> Modules affected:
>> -----------------
>> * liborchestrator
>>
>> Testing done:
>> -------------
>> configuration:
>> * HW: vmware guest (1GB RWM) on Linux host
>> * SW: LiveCD installation based on osol-0811-101a-rc1b.iso
>> * original partition configuration (before installation):
>> * Id    Act  Bhead  Bsect  Bcyl    Ehead  Esect  Ecyl    Rsect      
>> Numsect
>>   192   0    0      1      1       254    63     1023    16065      
>> 22491000    130   0    254    63     1023    254    63     1023    
>> 22507065   29988864    130   0    254    63     1023    254    63     
>> 1023    52500420   4192965
>>
>> * Disk screen identified partitions as:
>>   - please see attached screenshot
>>
>> [1] Without fix
>> ---------------
>> * installer didn't change legacy Solaris partition to new one
>> * both 0x82 partitions were marked as 'active':
>> * Id    Act  Bhead  Bsect  Bcyl    Ehead  Esect  Ecyl    Rsect      
>> Numsect
>>   192   0    0      1      1       254    63     1023    16065      
>> 22491000    130   128  254    63     1023    254    63     1023    
>> 22507065   29988864    130   128  254    63     1023    254    63     
>> 1023    52500420   4192965
>>   * installer hung during "bootadm update-menu -R /a -Z -O 
>> /dev/rdsk/c3d0s0"
>>
>> [2] With fix (full install_log attached)
>> ----------------------------------------
>> * installer changed legacy Solaris partition to new one (0xbf)
>> * only 0xbf partition was marked as 'active':
>> * Id    Act  Bhead  Bsect  Bcyl    Ehead  Esect  Ecyl    Rsect      
>> Numsect
>>   192   0    0      1      1       254    63     1023    16065      
>> 22491000    191   128  254    63     1023    254    63     1023    
>> 22507065   29988864    130   0    254    63     1023    254    63     
>> 1023    52500420   4192965
>>
>> * installer didn't hang and installed Solaris was successfully booted
>>
>>
>>
>>
>> ------------------------------------------------------------------------
>>
>


Reply via email to