Hi Dave,

Dave Miner wrote:
> Jan Damborsky wrote:
>> Hi,
>>
>> could I please ask for reviewing fix for following issues ?
>>
>> 1633 backout ZFS part of fix for 770 due to the failing "zpool import"
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=1633
>>
>> 1567 orchestrator should unmount ZFS dataset before resetting the 
>> mountpoint property
>> http://defect.opensolaris.org/bz/show_bug.cgi?id=1567
>>
>> Fix for 1633 removes problematic "zpool import" command from 
>> orchestrator
>> code - this one which is triggering 1350.
>>
>> The fix doesn't address
>>
>> 1610 installer should not lie
>>
>> This one would require substantial changes in install-finish & mkmenu
>> commands as far as error detection and handling is concerned, since that
>> was being a little bit neglected (to be modest) during design & 
>> implementation
>> phases.
>>
>> * Webrev:
>> http://cr.opensolaris.org/~dambi/bug-1567-1/
>>
>> * Testing done:
>> [1] Booted LiveCD RC2
>> [2] liborchestrator replaced
>> [3] Installation into existing Solaris instance in vmware was done
>>
>> Results:
>> * New Solaris instance booted successfully
>>
>
> Code is OK, couople of nits in perform_slim_install.c
>
> 891: comment is no longer accurate

I removed the comment.

> (really, it seems like we should run install-finish before resetting 
> the mount properties, but that's for another day)

I agree - the current way it is being done is cumbersome.
It would be addressed as part of fix for 1040.

>
> 1818: this comment is inaccurate on the list of file systems being reset

You are right - I modified the list of file systems appropriately.

Thank you very much for the code review,
Jan


Reply via email to