Thank you Ethan, See my comments in-line below.
I've tested the updates using the manual set up with the ICT test exercisers as described below. A full SPARC AI test install is in the works. I will not push until this test succeeds. The updated webrev is available at: Ethan Quach wrote: > Joe, > > > ict.c - nit - 743, the string in this line now fits with the string in > the previous line so it could be moved up. done > ict.c - return_status could be set at line 883, but then clobbered > at 900. This is correct the way it is because return_status is set to the same error code on both line 883 and 900. > ict.c - 886-903 - Instead of copying these files here, it would > seem to make more sense to augment ls_transfer() to take an array > of filenames to transfer along with the one known log file it > already copies. A summary of what we discussed on the phone: ls_transfer() should only transfer files created by logging service. Updating the logging service is outside of the scope of this bug. To track this I've filed enhancement bug: 7715 logging service should be more generic > > ict.c - Would you also need a call to ict_log_print() for failure > and success before lines 942 and 944 respectively as well? done > > > ict.py - 144 - For Sparc, its not GRUB, so can you rename this > to ICT_CREATE_SPARC_BOOT_MENU_FAILED instead. > > ict.py - 347,348 - Same comment as above, replace GRUB in the > variable name with BOOT > > ict.py - 1596 - same comment, change function name to > create_sparc_boot_menu() > > ict.py - 1597-1685 - I think you get the idea; anywhere that > uses the word grub/GRUB in the context of Sparc, rework it to > use boot/BOOT. I think I got them all. % grep -i sparc usr/src/lib/libict_pymod/ict.py | grep -ic grub 0 I also updated install-finish to use the new name. > > ict.py - 1617 - Don't hardcode in this dataset path. > Looks like theres a function called _get_root_dataset() > which is what you'd want to use. done > > > thanks, > -ethan > > > > > Joseph J VLcek wrote: >> Could I please get a code review for the fixes for bugs 6744, 4407 & >> 7570. >> >> Bugs: >> ----- >> >> 6744 beadm fails for sparc AI sun4v due to missing /rpool/boot/menu.lst >> http://defect.opensolaris.org/bz/show_bug.cgi?id=6744 >> >> 4407 AI should also tranfer the ai_sd_log and the revelant xml to the >> system after install >> http://defect.opensolaris.org/bz/show_bug.cgi?id=4407 >> >> 7570 boot -L does not work on sparc due to missing >> <rootpool>/platform/`uname -m`/bootlst >> http://defect.opensolaris.org/bz/show_bug.cgi?id=7570 >> >> Webrev: >> ------- >> http://cr.opensolaris.org/~joev/bug6744_4407_7570 >> >> Brief description: >> ------------------ >> >> The majority of the changes involve: >> >> Two new ICT added to ict.py: >> - create_sparc_grub_menu (fix for: bug6744) >> - copy_sparc_bootlst (fix for: bug7570) >> >> One updated ICT in ict.c >> - ict_transfer_logs (fix for bug 4407) >> >> I made some minor fixes to the ict_test.c test program. >> >> Some changes involve small cleanup and rearranging as needed for these >> changes. >> >> The modules affected and tested: >> -------------------------------- >> liborchestrator >> liblogsvc <- small change >> install-finish >> ICT, Library and Python >> >> Testing done on SPARC >> --------------------- >> >> 1. >> I manually set up an AI install environment on a SPARC system. I then >> used the ict_test program and the ict.py exerciser to invoke the >> relevant ICT. >> >> 2. >> I built a SPARC AI image to include my updated bits. Then a full AI >> install was performed. (Thank you Jan for the help with this.) >> >> >> 3. >> Testing done on x86 (to confirm no regressions:) >> >> - Booted LiveCD image in VirtualBox >> - using ld_preload to load updated libraries >> - mount -F lofs to mount updated ict.py >> - cp updated install-script to /sbin >> - performed install >> >> * Results: >> All ICT completed successfully and system booted >> >> >> Thank you! >> >> Joe >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
