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
