Hi Jack,

Thanks for the code review.  Please see my response below.

On 01/18/10 15:38, Jack Schwartz wrote:
> HI Karen.
>
> Here are my comments:
>
> install_utils.py:
> 677: typo  coorsponds -> corresponds
Will fix.
>
> 695: As Keith stated, the close() in the finally clause will execute 
> before the return on 696, so 695 is not needed.
Yes, you are right.  I also took Keith's suggestion and used the "with" 
clause.
>
> Question: Is there any problem with using the build-host's /etc crypt 
> files instead of the crypt files of the build area?  Seems it would be 
> safer to build using the build area's files whenever possible.  This 
> is also an easy change since you have the pkg_image area passed into  
> pre_boot_archive_pkg_image_mod.py automatically already.

I think it is better to use the build area's /etc.  I updated the 
password generation functions to pass in an optional
alternate root.  If it is specified, the alternate root's files will be 
used.  Otherwise, the system's files will be used.

Please see my updated webrev addressing everybody's comment here:

http://cr.opensolaris.org/~ktung/12726-updated2/

Thanks,

--Karen

>
>    Thanks,
>    Jack
>
>
> Karen Tung wrote:
>> Please review my changes to fix the following bugs:
>>
>> 12726 <http://defect.opensolaris.org/bz/show_bug.cgi?id=12726> DC and 
>> installers shouldn't require a root password in the packaged /etc/shadow
>> 12669 <http://defect.opensolaris.org/bz/show_bug.cgi?id=12669> DC 
>> errors about /var/pkg/catalog
>> 12186 <http://defect.opensolaris.org/bz/show_bug.cgi?id=12186> 
>> Deleting catalog cache needs to be revisited with with catalog v1 
>> <http://defect.opensolaris.org/bz/show_bug.cgi?id=1>
>>
>> http://cr.opensolaris.org/~ktung/12726/
>>
>> The following testing is done:
>>
>> - Build images using all modified manifests, name make sure that
>> the install is successful for each image, and the installed system 
>> boots cleanly.
>> Also make sure that I can use the default
>> root password of opensolaris to login as root or assume the root role.
>>
>> - Built an image where I specify my own root password and make sure
>> the installation is successful, and I can assume the root role
>> using the password I provided.
>>
>> Thanks,
>>
>> --Karen
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>


Reply via email to