Hi Joe, It looks good to me now.
Thanks, --Karen Joseph J VLcek wrote: > Karen, > > I've made the suggested changes from you and Jan, reran the tests and > posted an updated webrev available at: > > http://cr.opensolaris.org/~joev/bug4871_B/ > > Let me know if you feel I can push. > > Thanks, Joe > > > Joseph J VLcek wrote: > >> Thank you Karen, >> >> My comments are inline below. >> >> Joe >> >> >> Karen Tung wrote: >> >>> Hi Joe, >>> >>> install-finish: >>> >>> I think there's no need to keep an extra variable and update it with >>> the same value of "1" in the for loop for >>> every failure. Before you call sys.exit(), you can just check >>> "errcount", if it is not zero, you return 1. >>> >>> >> Good idea. I will do this. >> >> >> >>> perform_slim_install.c: >>> >>> - line 1418: why check for OM_FAILURE, instead of != ICT_SUCCESS, so >>> that it is consistent with all the other ICT calls? >>> - The run_install_finish_script() function, why not return OM_SUCCESS >>> instead of ICT_SUCCESS since this is all >>> related to ICT. >>> >> OM_xyz is used by run_install_finish_script() because, for ICT Phase I, >> it is defined in liborchestrator which uses the OM prefix. >> >> In an effort to minimize changes I dont think moving >> run_install_finish_script() from liborchestrator to libict, at this >> phase in the project, is a good idea. For ICT Phase II this >> functionality will be reworked. >> >> >> Thanks again! >> >> Joe >> >> >> >> >>> --Karen >>> >>> >>> Joseph J VLcek wrote: >>> >>>> Hello, >>>> >>>> Can two people please do a code review for a fix for bug: >>>> >>>> 4871 ICT failures are not reported as a failed installation >>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4871 >>>> >>>> >>>> The webrev is available at: >>>> http://cr.opensolaris.org/~joev/bug4871/ >>>> >>>> >>>> * The modules affected and tested: >>>> >>>> liborchestrator >>>> install-finish >>>> >>>> * Testing done for GUI Install >>>> >>>> I booted a 101 live Image and applied the updated library using >>>> LD_LIBRARY_PATH, I copied the updated install-finish script onto /sbin >>>> >>>> I fabricated a failure in ict.py with a hard coded error to have one >>>> of the ICT return a failure. >>>> >>>> I used mount -F lofs so the ict.py containing the error would be used. >>>> >>>> I then repeated the test using the version of ict.py we deliver to >>>> ensure the installation succeeded. >>>> >>>> * Results: >>>> >>>> When using the version of ict.py with a hard coded error the >>>> installation reported errors. >>>> >>>> When using the version of ict.py we deliver, without a hard coded >>>> error, the installation succeeded. >>>> >>>> >>>> * Testing done for AI >>>> >>>> No AI testing was performed. >>>> >>>> >>>> Thank you, >>>> Joe >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>>> >> > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
