Hi Jean.
Here are my comments:
live-fs-root:
165: "differently" is misspelled
209: My guess is that this is not correct. Line 216 says if no usb
stick with a UFS file system on it was found (./liveusb is not a file --
see lines 192-207) then check for a CD. Lines 209-214 basically disable
lines 216-223.
IMO, there is no harm in checking for a CD; right now, AI does this for
X86 to no harm, even though the AI image isn't a live CD. Also,
eventually we may support a live CD for SPARC, and when we do, no
changes will be needed here to enable it.
create_iso: OK
bootroot_initialize.py: Thanks for cleaning up after me...
ai_sparc_image.xml:
36, 39: Please add a comment or XXX that this repo needs to be changed
before release. Better yet, file a bug to track it so it doesn't get
dropped.
I suggest basing ai_sparc_image.xml on the existing ai_x86_image.xml
since it has all of the comments present and current, and then changing
the XML contents to be what you want. Doing this will also bring the
comment at line 45 current, etc. If you don't do this now, you'll just
end up having to do it later anyway...
Package list is probably OK.
Regarding bootroot file list:
Doing a diff between ai_x86_image.xml and ai_sparc_image.xml...
- 398-9: Get rid of var/sadm/README and var/adm/spellhist
- Add var/sadm/install/contents
Not sure about the following:
- 385: I'm wondering if you can delete usr/bin/sparcv7/ksh93. You may
want to check...
- 422: ai_x86_image.xml doesn't include the opt dir. Do you need it?
- add excluding of etc/notices directory
Not sure about adding ai_post_bootroot_archive finalizer script: Karen
asked why the bootroot didn't have the permissions set the same way;
the reason is that this script wasn't in the SPARC manifest; however,
things still seemed to work... So I'm wondering why it was needed in
the first place, or if maybe it's needed for X86 but not for SPARC...
DC_defs.py: OK
bootroot_archive.py:
Please honor 80-character line maximums.
62: compress() needs a header
Nit: 67, 79, 88: if (status != 0) seems less circuitious than if not
status == 0
87: just use stat_out of the previous line: mode = stat.out.st_mode
Error handling in compress() can be optimized. replace 79-83 and 92-96
with:
if (status != 0):
break
and then at 103, raise an exception if status != 0.
Will files in the uncompress_flist be copied and compressed in the first
loop, and then get copied again in the loop at 114? This may be worth
optimizing of these files are large. Otherwise, it's OK.
263: Changes to files should be done in bootroot_configure, not
bootroot_archive.
282, 296, other places?: an optimization would be to do
is_sparc = (platform.platform().find('sparc') >= 0)
and then use is_sparc instead of repetatively doing platform.platform.
Thanks,
Jack
On 12/10/08 12:56, Jean McCormack wrote:
> Karen and anyone else please review:
>
> http://cr.opensolaris.org/~jeanm/slim_sparc/
>
> I believe the appropriate defect is:
> http://defect.opensolaris.org/bz/show_bug.cgi?id=4230
>
> Yes, I'll need to commit with this for the comments.
>
> Jean
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>
