On Mon, 2009-10-05 at 12:48 -0700, Karen Tung wrote:
> Alexander Eremin wrote:
> > Hi caiman-team,
> > please review my initial changes in DC bootroot_archive.py after
> > investigation of 6361 bug (more details here - I found also that for
> > sparc 'cpio' runs twice:
> > http://www.opensolaris.org/jive/thread.jspa?threadID=114297&tstart=0)
> >
> > Test:
> > # distro_const build -r br-arch ai_sparc_image.xml
> > ...
> > Before changes:
> >
> > # df -h /mnt
> > Filesystem             size   used  avail capacity  Mounted on
> > /opensolaris/dc/build_data/pkg_image/boot/boot_archive
> >                        116M    79M    36M    69%    /mnt
> >
> > After:
> >
> > # df -h /mnt
> > Filesystem             size   used  avail capacity  Mounted on
> > /opensolaris/dc/build_data/pkg_image/boot/boot_archive
> >                         87M    79M   7.6M    92%    /mnt
> >
> > webrev:
> > http://cr.opensolaris.org/~alhazred/6361/
> >
> > Also tested with bootroot_size > 150000
> >
> >   
> Hi Alexandar,
> 
> Thanks for making the changes.  You are right about the fact that the
> first CPIO that's done in the existing bootroot_archive.py script is a bug.
> 
> Here are my comments the changes:
> 
> - line 103: shouldn't the check be done for dst + "/" + cpio_file?
> - Please add more detail to the comment on lines 327-329 explaining
> why different overhead multipliers are used for sparc.
> - I think it would make the code easier to read if you move the code
> between lines 370-415 around so there's only 1 if-then-else block for
> if (sparc)... else...
> - You didn't mention it in your message above.  Did you also built
> and tested x86 LiveCD images to make sure the changes didn't affect
> those?
> - I noticed that the formatting of the file doesn't correspond to the new
> PEP8 formating requirement that Clay sent email about last week.
> Can you fix that?   The link announcing the requirement is at:
> http://www.opensolaris.org/jive/thread.jspa?threadID=114047&tstart=15
> 
> Thank you for your work!
> 
> --Karen
Thanks Karen!
Next days will work for code and tests on x86.
-- 

::alhazred

Reply via email to