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 >>> >> > >
