Hi Dave, Looks ok now.
thanks, sarah ***** > Sarah Jelinek wrote: >> Hi Dave, >> >> Comments below... > > Thanks. An updated webrev which addresses both your comments and > Alok's is up at > > http://cr.opensolaris.org/~dminer/slim_6440_v2 > > See responses in-line. > >>> Caimaniacs, >>> >>> Please review the fixes for >>> >>> 6440 Build 106 installer fails to start in 512 MB environment >>> 7975 LiveCD still has old branding - desktop background - in B111 >>> >>> (I've dispensed with links to the above here as the recent webrev >>> updates now include d.o.o linkage in the webrev). >>> >>> webrev for which may be found at: >>> >>> http://cr.opensolaris.org/~dminer/slim_6440/ >>> >>> This has been tested extensively on x86, both in live CD and x86 AI >>> installations, including in domU's (thanks to John Levon). I'm >>> still in the process of testing SPARC, but since it's supposed to be >>> a no-op for SPARC, do not anticipate issues. >> DC_defs.py: >> -The base path for both sparc and x86 is /boot, so why not define a >> BR_BASEPATH, then go from there in terms of defining the >> BR_FILENAME_SPARC, etc... >> >> For example: >> BR_BASEPATH = "/boot" >> BR_FILENAME_SPARC = BR_BASEPATH + "boot_archive" >> BR_X86_FILENAME = "x86.microroot" >> BR_FILENAME_X86 = BR_BASEPATH + "/" + BR_X86_FILENAME >> ... > > Similar comment from Alok. Essentially accepted, see webrev. > >> >> ai_sparc_image.xml: >> -line 501 you pass in "all" to bootroot_archive.py, but in >> bootroot_archive.py you check for is_sparc. Why do you pass in "all" >> then check for is_sparc, if it is_sparc why not pass in 'sparc'? >> > > Also suggested by Alok and accepted. > >> I can see that you pass in 'all' in ai_x86_image.xml as well which >> passes through to these lines: >>> 267 else: >>> 268 BR_ARCHFILE = PKG_IMG_MNT_PT + BR_FILENAME_ALL >>> 269 BR_BUILD = BR_MASTER >>> 270 strip_archive = False >> I assume you did this just to be consistent in the xml templates? >> > > I decided not to bite off doing this for AI at this time. > Consistency, and it simplifies the argument handling a little bit. > Mostly wanted this to be an explicit behavior in the manifest, rather > than some hidden side-effect. > >> bootroot_archive.py: >> -Why do we add 20% to smaller archives for padding? >> > > I ran into ramdisk exhaustion with the lesser padding on these > archives; SMF makes a copy of its repository at boot and that > typically failed on the 32-bit archive without this change. > >> slim_cd_generic_live.xml: >> -line 137, why the change to sendmail from ndp? >> > > See response to Alok. > >> transfer_mod.py: >> -line 446, why did you move the list of files to cpio to /tmp rather >> than keeping it in self.dst_mntpt? >> > > The initial version of this fix used the root_archive command to do > the unpacking of the alternate archive after the file lists were > generated (since it's exactly what we have to do), and one of the > things "root_archive unpack" does is wipe the destination directory, > so the file lists were getting destroyed, with obviously catastrophic > results for the install. I had to change to the reviewed > implementation with transfer_mod doing the gunzip/mount and using the > existing cpio logic because "root_archive unpack" doesn't work right > in domU's (see 6828454). I could have moved these back since > root_archive is out of the picture now, but /tmp seemed like the right > answer anyway so we don't have to worry about cleaning them up > (actually, probably should be /var/run, now that I think about it, so > changed). We already have swap allocated by the time this runs, so > the space usage in tmpfs isn't an issue. > >> -General question.. what is the memory requirement with your changes? >> < 512MB but do you have a sense of how close this is to the 512MB limit? >> > > No, didn't bother since it's not a requirement; probably can go a > little below that since the responsiveness isn't bad, but likely not > very far. This works fine in 512 MB VM's, which is the primary > platform we care about for 512 MB. Mary is testing some of our 512 MB > hardware for me, results seem to be similar as for 2008.11. > > Dave
