On 04/21/09 14:52, Joseph J VLcek wrote: > jan damborsky wrote: >> Hey Joe, >> >> thanks a lot for reviewing those changes. >> Please see my response in line. >> >> I have updated webrev with all changes incorporated. >> >> Jan >> >> >> >> On 04/20/09 18:15, Joseph J VLcek wrote: >>> jan damborsky wrote: >>>> Hi, >>>> >>>> could I please ask two people for reviewing changes for following >>>> blocker ? >>>> >>>> 8130 mismatch between boot_archive and solaris*.zlib if more than >>>> one service is created for Sparc >>>> >>>> The fix also addresses >>>> >>>> 8115 SPARC AI shouldn't use root-path(Option 17) for locating >>>> solaris*.zlib archives >>>> >>>> Sundar, could I please ask you, if you might confirm/deny >>>> if it is fine to get rid of using 'Rootpath' option by >>>> Sparc AI client for locating solaris*.zlib archives ? >>>> Thank you. Just in case you check emails - Monday is fine ;-) >>>> >>>> webrev: >>>> http://cr.opensolaris.org/~dambi/bug-8130 >>>> >>>> Thank you very much, >>>> Jan >>>> >>>> >>>> modules affected: >>>> ----------------- >>>> * DC (Sparc AI manifest) >>>> * installadm tools >>>> * AI image (live-fs-root) >>>> >>>> testing done (full test procedures attached): >>>> --------------------------------------------- >>>> [1] AI image containing fix built using modified DC >>>> - verified that microroot contains /usr/bin/mkdir >>>> >>>> [2] combinations of 'create-service' & 'create-client' >>>> tested using installadm tools with fix (please see >>>> attached test results for more details). >>>> >>>> [3] new AI image booted successfully without 'Rootpath' >>>> option provided by DHCP server. >>>> >>>> known issues: >>>> ------------- >>>> * fix will introduce incompatible change in Sparc AI image >>>> and installadm tools - old Sparc AI images will no longer >>>> work with new installadm tools, since they rely on Rootpath >>>> DHCP option. x86 is not affected. >>>> >>>> solution: >>>> Flag day will be sent out. >>>> >>>> >>>> ------------------------------------------------------------------------ >>>> >>>> >>>> _______________________________________________ >>>> caiman-discuss mailing list >>>> caiman-discuss at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> >>> Hey Jan, >>> >>> Looks OK. A couple miner nits/issues. >>> >>> >>> >>> usr/src/cmd/slim-install/svc/live-fs-root >>> >>> Issue 1: >>> -------- >>> >>> If a failure is detected /etc/netboot is not removed. >> >> you are right - but I don't think we need to clean up this, >> since it is temporary working environment and changes >> will disappear after reboot. >> >> Also, in general I think if anything fails during the installation >> process we shouldn't clean up things if not necessary, but instead >> leave the system untouched, so that user can investigate at which >> point things went wrong. >> >>> >>> Issue 2: >>> ------- >>> >>> Suggestion: Use a variable for /etc/netboot >>> >>> >>> From: >>> 287 $MKDIR /etc/netboot >>> ... >>> 297 $MOUNT -F hsfs -o ro "$BOOTFS_DISK" /etc/netboot >>> > \ >>> >>> To: >>> >>> NETBOOT="/etc/netboot" >>> ... >>> >>> 287 $MKDIR ${NETBOOT} >>> ... >>> 297 $MOUNT -F hsfs -o ro "$BOOTFS_DISK" ${NETBOOT} > \ >> >> Changed. >> >>> >>> >>> Issue 3: >>> -------- >>> >>> Not an issue that needs to be resolved for this change but in >>> general many of our scripts should do a better job of doing things >>> like trapping interrupts and meeting shell coding guidelines using >>> built in true/false instead of strings "true"/"false"... >>> >>> For future script work consider the suggestions here: >>> http://installzone-wiki.central.sun.com/wiki/index.php/Ksh93_Tips >> >> I think I could at least promise that in future any script I will >> put together from scratch will follow those recommendations ;-) >> > > Thanks Jan. > > I think you make a good point about not cleaning up things on failure > to aid diagnosing what went wrong.
Joe, thanks a lot for review ! Jan
