Hi Jack, Thank you for looking over the changes. I have made the change to validate_crypt_id().
Please take a look at the updated webrev at: http://cr.opensolaris.org/~ktung/12726-updated2/ Thanks, --Karen On 01/20/10 11:08 AM, Jack Schwartz wrote: > Hi Karen. > > Thanks for addressing my comments. Only one thing: > > install_utils.py, line 772: > > the call to validate_crypt_id() is missing the alt_root arg. > > The rest looks fine. > > Thanks, > Jack > > On 01/19/10 15:52, Karen Tung wrote: >> 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 >>> >> >
