Hey Drew,
Thanks for reviewing - I wasn't expecting this since you are supposed to
be on vacation so thanks! :-)
Responses below...
On 12/23/11 11:58 PM, Drew Fisher wrote:
Niall,
DC:
create_iso.py: Does anything bad happen if the user specifies
something for UEFI with the args to mkisofs? I ask because the
command that gets called will always have -b self.bios_eltorito (from
bios_args) but with UEFI, it willl *ALSO* have -b self.uefi_eltorito
(from uefi_args).
I'm not sure what you mean so I will try a series of guesses :)
The -b self.uefi_eltorito argument is fine. I've been through the
mkisofs arguments with Seth and they are precisely as he instructed. He
was quite adamant about the ordering of the arguments which is reflected
in the comments. Notice the -b self.uefi_eltorito argument is preceeded
by "-eltorito-platform efi" so that should eliminate ambiguity or
confusion about the El Torito firmware image type.
The mkisofs command as DC invokes it will behave well if either:
- only a BIOS El Torito image is provided
- BIOS AND UEFI El Toritio images are provided
In either case the Bios El-Torito is required as a bare minimum (this
is all that we would get from Legacy GRUB)
There is currently no case where UEFI will be specified in its own,
absent BIOS since GRUB2 supports both and that's what the Boot
checkpoint requests.
otherwise this looks fine.
Boot:
boot.py:
217 - XXX comment
Can't do much about this until SPARC ODD is implemented in pybootmgt :-)
We don't need it to create SPARC ISOs in the meantime.
347-351 - XXX comment
As with above, these aren't implemented in pybootmgmt so the next best
thing i can do is make a note of it so we can add it in
easily when it's available.
677: NIT - fix indent (back up one space)
Will do.
783-784: XXX comment
Will chase that up with Seth. We could probably delete this block since
we won't be using Legacy Grub for new installations any more.
856: commented line
It refers to the XXX comment below at 861
861-864: XXX comment
Will follow up with Seth if it's still broken otherwise re-enable the
desired ident file name.
978-980: XXX comment
Again, needs SPARC ODD implementation in pybootmgmt so my hands are tied :-)
997: Damn right.
I'm so glad you review that ;)
Otherwise this looks ok to me.
tests.nose:
I have no idea what really changed here, but I'm assuming your
add/deleting some entries in the giant list which is fine by me.
I just added the install_boot/test subdir to the end of the list of unit
test dirs to look in. It was missing previously, so the boot unit tests
were not getting exercised during automated unit test runs
Thanks!!!
Niall
-Drew
On 12/23/11 4:44 AM, Niall Power wrote:
Hi,
Just a reminder that the timeout for round 1 review comments is this
Friday 23rd.
Thanks to everyone who has already provided comments or volunteered
to review.
I'll still check e-mails for review feedback over the holiday break,
but expect responses to be somewhat spotty :-)
I'd also really like to limit the scope of round 2 comments to final
tweaks and polish. So if there is anything you are deeply
uncomfortable with in the webrev but haven't mentioned yet, best to
get it out in the air now :)
I'm getting good feedback on Targets, AI and the GUI.
I haven't heard anything back regarding the following sections:
- Text Install
- Makefiles and Misc.
- DC and Boot
Karen has kindly offered to review the Text and Makefile & misc.
sections.
If anyone else feels like having a stab at any of these let me know.
Thanks,
Niall
On 12/19/11 08:19 PM, Niall Power wrote:
Hi!
Myself, Drew and Dave Marker would like to ask for code reviewers
for the Install UEFI, GRUB2 and large disk boot project.
I've broken down the webrev into component groupings and specified
people who we would particularly like to review particular
components (eg. leads for those compononents) but we welcome and
appreciate any and all review comments from others too of course.
Some things to be aware of:
This is about 95%, but not yet 100% complete.
The core is complete and functional but there are a few odds and
ends that need to be done still:
- usbgen and usbcopy need to be ported to be GRUB2 and UEFI
compatible. I am figuring out this with Seth. Will create a separate
webrev.
- installadm changes for GRUB2 and UEFI. Sue is working on this
and will be handled in a separate review
- Installation of UEFI system into an MBR partitioned disk. This
is a minor amount of work that doesn't impact the existing code
changes (it will essentially add a small amount more code). I'll get
this included in time for round 2, otherwise create a separate webrev.
I'd like to set an initial timeout for collection of round 1 review
comments on Friday Dec 23rd.
Following that I propose to ring in the new year with a second round
of review comments from Monday Jan 2nd until Friday Jan 6th.
Please let me know if you intend to review any components, or if you
are unavailable to review specific requested components.
*Webrev URL:
----------------*
https://cr.opensolaris.org/action/browse/caiman/niall/slim_uefi_version_01/webrev-uefi-ver-1/
Most of the files below have only a very small number of lines of
code changes so it's not as big a task as it looks.
*
Makefile and Misc.: Volunteers
----------------------------------------*
usr/src/Makefile.master
usr/src/Targetdirs*
*usr/src/lib/Makefile.targ
usr/src/lib/install_common/__init__.py.src
*
*
*Targets: Dermot, Darren, Matt
---------------------------------------*
usr/src/pkg/manifests/system-library-install.mf
usr/src/lib/install_manifest/dtd/target.dtd
usr/src/lib/install_target/Makefile
usr/src/lib/install_target/__init__.py
usr/src/lib/install_target/controller.py
usr/src/lib/install_target/cuuid.py
usr/src/lib/install_target/discovery.py
usr/src/lib/install_target/instantiation.py
usr/src/lib/install_target/libdiskmgt/diskmgt.py
usr/src/lib/install_target/libefi/Makefile
usr/src/lib/install_target/libefi/__init__.py
usr/src/lib/install_target/libefi/cfunc.py
usr/src/lib/install_target/libefi/const.py
usr/src/lib/install_target/libefi/cstruct.py
usr/src/lib/install_target/libefi/efi.py
usr/src/lib/install_target/logical.py
usr/src/lib/install_target/physical.py
usr/src/lib/install_target/shadow/__init__.py
usr/src/lib/install_target/shadow/physical.py
usr/src/lib/install_target/test/test_shadow_list.py
*AI: Darren, Matt
--------------------*
usr/src/cmd/auto-install/checkpoints/target_selection.py
usr/src/cmd/auto-install/test/test_target_selection_gpt.py
*DC and Boot: Ethan, Seth, Drew
-----------------------------------------*
usr/src/cmd/distro_const/checkpoints/create_iso.py
usr/src/lib/install_boot/boot.py
usr/src/lib/install_boot/test/test_boot.py
usr/src/tools/tests/tests.nose
*
GUI: Darren, Dermot
---------------------------*
*
*usr/src/cmd/gui-install/src/Makefile
usr/src/cmd/gui-install/src/confirm_screen.py
usr/src/cmd/gui-install/src/disk_panel.py
usr/src/cmd/gui-install/src/disk_screen.py
usr/src/cmd/gui-install/src/fdisk_panel.py
usr/src/cmd/gui-install/src/gpt_panel.py
usr/src/cmd/gui-install/src/screen_manager.py
usr/src/cmd/gui-install/xml/Makefile*
*usr/src/pkg/manifests/system-install-gui-install.mf *
*
*
I seriously wouldn't bother code reviewing these glade
auto-generated XML files but if you enjoy pulling your eyes out :)*
usr/src/cmd/gui-install/xml/fdisk-panel.xml
usr/src/cmd/gui-install/xml/gpt-panel.xml
usr/src/cmd/gui-install/xml/installationdisk.xml
*
**
Text Install: Karen + Volunteer
--------------------------------------*
usr/src/cmd/text-install/Makefile
usr/src/cmd/text-install/__init__.py
usr/src/cmd/text-install/disk_selection.py
usr/src/cmd/text-install/disk_window.py
usr/src/cmd/text-install/fdisk_partitions.py
usr/src/cmd/text-install/gpt_partitions.py
usr/src/cmd/text-install/helpfiles/Makefile
usr/src/cmd/text-install/helpfiles/gpt_partitions.txt
usr/src/cmd/text-install/partition_edit_screen.py
usr/src/cmd/text-install/progress.py
usr/src/cmd/text-install/summary.py
usr/src/cmd/text-install/ti_install.py
usr/src/cmd/text-install/ti_install_utils.py
usr/src/cmd/text-install/ti_target_utils.py
usr/src/cmd/text-install/welcome.py*
*usr/src/pkg/manifests/system-install-text-install.mf
*
*Thanks!!
Niall, Dave & Drew
*
**
*
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
