Forgot the webrev url... The updated webrev is available at: http://cr.opensolaris.org/~joev/bug6744_4407_7570/
Joseph J VLcek wrote: > 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 > >
