Hi Drew & Dermot,
Here are my comments for the GUI changes. I'll try to squeeze in time to
look at the target libraries also.
usr/src/cmd/gui-install/aux/INSTALL_DISK_DISCOVERY_PANEL.txt:
-------------------------------------------------------------
15: Ultra Nit: s/iSCSI standard/iSCSI protocol
19: Nit: 'Local disks' is the quotation referenced, however the GUI
contains "Local Disks" d->D
usr/src/cmd/gui-install/src/disk_screen.py:
-------------------------------------------
604: Will "disk.disk_prop.dev_type != iSCSI" guarantee that the disk is
local? How would this cope with a fibre channel disk?
601 & 606: It might be useful to state what disks are being filtered
*out* also. Users are likely to look for answers to why they can't see
the disk they want to install to.
eg.
599 disk.disk_prop.dev_type == "iSCSI":
600 filtered_disks.append(disk)
601 LOGGER.debug("Filter *in* disk: %s", disk.ctd)
continue
602 if profile.show_local and \
603 (disk.disk_prop is None or \
604 disk.disk_prop.dev_type != "iSCSI"):
605 filtered_disks.append(disk)
606 LOGGER.debug("Filter *in* disk: %s", disk.ctd)
continue
Logger.debug("Filter *out* disk: %s, disk.ctd)
848 - 857: Why is it now hardcoding icons into the app instead of
looking them up from the icon theme as before?
This breaks both theming and accesibility. Icon changes or new icons
should go back into the Nimbus theme.
usr/src/cmd/gui-install/src/gui_install_common.py:
--------------------------------------------------
292 & 293 Nit: s/Iscsi/iSCSI
47 & 296: cache_update() It looks wrong to me for the GUI (or other
client) to be trying to manipulate libdiskmgt, which is just an
implementation detail of target_discovery. It sorta violates that whole
MVC thing :) Why can't target discovery take care of this itself when
doing iSCSI discovery?
356 & 400: Why does the checkpoint error no longer raise an exception?
It used to raise RunTimeError
Cheers!
Niall
On 16/02/2012 05:30, Drew Fisher wrote:
Good morning!
Dermot and I would like to get a first-round code review for the
following PSARC/CRs:
PSARC/2012/023
<http://psarc.us.oracle.com/Archives/CaseLog/arc/PSARC/2012/023> Interactive
Installation to iSCSI
6974246 <http://monaco.us.oracle.com/detail.jsf?cr=6974246> Automated Install
should provide mechanism for setting iSCSI initiator-id-node
7004719 <http://monaco.us.oracle.com/detail.jsf?cr=7004719> Opensolaris
LiveCD installation should give a GUI for installing Solaris onto iSCSI LUN
7004720 <http://monaco.us.oracle.com/detail.jsf?cr=7004720> Opensolaris text
installer should give a screen for installing solaris onto iscsi lun
7114789 <http://monaco.us.oracle.com/detail.jsf?cr=7114789> unlabelled iSCSI
drives and setting an iSCSI boot-device require special handling on SPARC
7121245 <http://monaco.us.oracle.com/detail.jsf?cr=7121245> iscsi paths don't
translate to bootable OBP paths/strings
7132111 <http://monaco.us.oracle.com/detail.jsf?cr=7132111> sample ai
manifest file does not mention setting whole_disk in disk target section
7132457 <http://monaco.us.oracle.com/detail.jsf?cr=7132457> Race condition in
AI involving the target discovery and the check for the new OS Device Name
7145512 <http://monaco.us.oracle.com/detail.jsf?cr=7145512> pulling iSCSI
information from the DHCP server does not support iSCSI boot
https://cr.opensolaris.org/action/browse/caiman/drewfish/iSCSI/
Karen: Could I get you to look specifically at the text-installer
changes?
Niall/John: Could I get you both to look specifically at the GUI changes?
Jesse: Can you look at code in $SRC/lib/install_target as that's
where all the calls to iscsiadm and libima.so exist
Outstanding issues:
- QE has not yet started a test cycle. I've been working with Angela
Li on this. I'm hoping to start a test cycle soon
- ai_manifest.4 is still being edited. I've emailed proposed changes
to Alta who will translate them into actual English
- The fix for 7132457 is slated to be tested by PIT soon for confirmation.
ISO / USB access can be found at Hudson:
http://indiana-build.us.oracle.com/view/iSCSI/ Choose the job for the
installer you want to look at and you'll see links along the job's
page pointing to the latest ISO / USB image
Screenshots of the GUI and Text-Installer (for those not interested in
firing up an entire ISO) can be found here:
http://mox.us.oracle.com/code/screenshots/gui/
http://mox.us.oracle.com/code/screenshots/ti/
If people want to try out the ISOs, let me know and I can provide
target IP/LUN information to use.
Thanks!
-Drew and Dermot
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
