Niall, Reviewed target_selection.py, In general looks pretty clean, nice work. Will try and review the rest of the targets code as well.
target_selection.py: 1772 : - You've removed the validation for determining of the disk is contained in the root pool, what's your reasoning behind this ? - For the children check e.g. VTOC sparc children must be Slices etc. Can you add the text for these tests to the comments at the start of the validate disk method. - Also add an other specific GPT validation checks to comment summary at the start of the method 1805: - check for disk has children in a whole disk scenarion is commented out, if this is intentional should just remove the code completely, but I can't think of a good reason to remove this, - Again the check for check_in_root_pool for the disk why are you removing this here ? 1985, 1988, 2000: - Check for root_slice_required and root_slice_found Both of these are set to false at start of function, the only place they are set then is at 1985 and 1988, where they are both always set to true, effectively meaning that they will always both be either False or both True, thus the check at 2000 can never actually happen. When would a root_slice_required not be needed ? surely a root slice is always required regardless of slice or gptslice ? 2851, 3375; - Why is disk_copy.whole_disk being set to True for GPT slice ? 3523: - Manual setting of in_zpool/in_vdev on Controller selected disks was not required before, how come it's required now ? cheers Matt On 12/19/11 09:19, 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
