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
