Thank you Jack. All suggestions made built and I ran a test of the individual ICT, ICT set_Solaris_partition_active, on SPARC.
Joe Jack Schwartz wrote: > Hi Joe. > > Looks really good. A few small comments. > > 1563: where did 1024 come from? My guess is that's MAXPATHLEN, but you > may want to say so in a comment, and add this to bug 5559 for extracting > #defines from C header files. > > 1567: Why not just use status, and get rid of return_status? > > 1571: This should be '...status = ' + str(return_status) > > These are easy, low-risk fixes to make. As such, they don't require > retesting except for sanity checking (i.e. that the module compiles) and > I won't ask for another code review once these changes are made. > > Thanks, > Jack > > > > On 01/26/09 12:56, Joseph J VLcek wrote: >> * Please review the changes for Bug 6080: >> >> http://defect.opensolaris.org/bz/show_bug.cgi?id=6080 >> >> * webrev >> >> http://cr.opensolaris.org/~joev/bug6080/ >> >> * The modules affected and tested: >> >> ICT - Python code >> >> --- Testing --- >> >> * Testing done on SPARC: >> ------------------------ >> >> On a SPARC test system an alternate root was manually populated using >> pkg image-create and pkg install of SUNWcsd, SUNWcs and slim_install. >> >> Using the eeprom command the default boot-device was set to an >> invalid value. >> >> Then the ict.py updated Class member: set_Solaris_partition_active was >> exercised using the ICT Class test member: exec_ict() >> >> Than a reboot was performed >> >> * SPARC Results: >> ICT set_Solaris_partition_active completed successfully and system >> booted to the correct disk. >> >> * Testing done on x86 (to confirm no regressions:) >> -------------------------------------------------- >> >> [1] Booted LiveCD image on live hardware HP Pavilion dv5000 >> [2] mount -F lofs to use the updated ict.py module >> [3] Installer run >> >> * Results: >> All ICT completed successfully and system booted >> >> * Testing done on SPARC: (To be completed prior to push of fix) >> --------------------------------------------------------------- >> >> On SPARC, with help from Jan a full AI install was performed. >> >> * SPARC Results: >> All ICT completed successfully and system booted >> >> * Description: >> >> This code change is to set the default boot-device on SPARC. >> >> * Outstanding issue: >> >> The Python ICT code needs to be updated to share definitions between C >> system headers and the ICT Python code. >> >> The following bug has been filed and updated with ICT specific >> information to track this issue. >> >> Bug 5559 - Need better scheme for sharing definitions between C & >> Python code. >> >> >> >> >> >> >> Huge thanks for help to Tycho Nightingale, Jan Damborsky and Jack >> Schwartz >> >> Thank you. >> Joe >
