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