Hi Alok,
Alok Aggarwal wrote: > Hi Jan, > > On Wed, 11 Nov 2009, Jan Damborsky wrote: > >> Hi Jean, Alok, >> >> could I please ask you to review fix for following bugs ? >> >> <http://defect.opensolaris.org/bz/show_bug.cgi?id=7966>7966 setting >> 'livemode=text' is no-op for AI, should be removed from installadm(1M) >> 8140 Fix for 6440 applied to x86 AI images would reduce x86 AI >> memory consumption >> >> >> Thank you very much, >> Jan >> >> webrev: >> http://cr.opensolaris.org/~dambi/bug-8140/ > > installadm-common.sh: lines 463-464: Sorry if I'm dense but > I don't see why this check is needed. If x86.microroot exists, > it also means that split 32/64 bit archives don't exist; so > we should pick up the boot archive from /${BootLofs}/x86.microroot. > > Otherwise, pick it up from the appropriate platform directory. > Which cases would this miss? You are right. Too much 'ifs' as Jean also pointed out :-) I have removed one of them and added comment. The webrev has been updated accordingly. > > installadm-common.sh: line 464: Is the "\" before $ISADIR really > needed? Yes, since otherwise $ISADIR would be evaluated and we would end up with module$ /${BootLofs}/platform/i86pc//boot_archive in GRUB instead of desired module$ /${BootLofs}/platform/i86pc/$ISADIR/boot_archive > > ai_plat_setup.py: I wonder why we didn't catch this while testing > 8347. Creating that symlink was intentional - it was stop gap solution before fix for 8140 is integrated in order not to break 64bit XEN PV, as it scans following locations for 64 bit boot archive: /boot/x86.microroot /boot/amd64/x86.microroot /platform/i86pc/amd64/boot_archive More details about XEN dependencies on install can be seen at: http://hub.opensolaris.org/bin/view/Community+Group+xen/install-depedencies Thank you very much for review ! Jan
