Dermot, Still looks good. I do note that you have added a modal dialog with 2 localized messages both messages are already within the gui-installer space. So it looks good.
Thanks, John On Sep 15, 2011, at 10:33 AM, Dermot McCluskey wrote: > Darren, > > Thanks for reviewing. Both suggestions accepted > (and tested). > > New webrev: > https://cr.opensolaris.org/action/browse/caiman/dermot/7090232-2/ > > Incremental since round #1: > https://cr.opensolaris.org/action/browse/caiman/dermot/7090232-2-incremental/ > > - Dermot > > > > On 09/15/11 17:38, Darren Kenny wrote: >> Hi Dermot, >> >> Generally looks fine, but I do have some comments: >> >> 659 slices = partition.get_children(class_type=Slice) >> ... >> 661 + for slc in slices: >> 662 + partition.delete_slice(slc) >> >> >> could be rewritten as simply: >> >> partition.delete_children(class_type=Slice) >> >> I don't believe there is any reason that you must call delete_slice(), is >> there? >> >> 679 + LOGGER.error("Internal Error: Solaris2 partition " >> 680 + "should have exactly 1 gap.") >> 681 + LOGGER.error("Gaps found: %s" % gaps) >> 682 + LOGGER.error("Attempting to continue anyway.") >> >> Can you really continue here? I would think you could handle> 1 gap easily >> enough - but I don't think continuing here without creating the slice would >> really work. >> >> Thanks, >> >> Darren. >> >> >> On 15/09/2011 17:22, Dermot McCluskey wrote: >>> I'm seeking b175 putback approval and two code reviewers for: >>> >>> http://monaco.sfbay.sun.com/detail.jsf?cr=7090232 >>> GUI installer does not create rpool on whole partition >>> if previous slices exists in partition >>> >>> Webrev: >>> https://cr.opensolaris.org/action/browse/caiman/dermot/7090232/ >>> >>> Details: >>> This is an oversight in gui-install: in certain cases if there's >>> an existing Solaris partition, and it contains slices, gui-install/ >>> TargetController do not wipe those slices and lay down our >>> own preferred layout (TextInstall does this explicitly). >>> If the user makes any changes to the Solaris partition (type, >>> size) the child slice details are lost and gui-install notices this >>> and re-instates slice 0 at validation time. This fix takes >>> advantage of this by always wiping the slices and creating >>> slice 0 at validation time. This results in a consistent layout >>> after validation, regardless of whether: >>> - the partition has an existing slice layout >>> - TargetController created a slice layout >>> - the slice layout has previously been removed >>> >>> Testing done: >>> Used format to create an "unfavorable" slice layout on the >>> Solaris2 partition of the target disk. Ran gui-install. >>> Before fix: if no changes are made to the partition, the >>> old slice layout is retained. >>> After fix: regardless of whether changes are made, the >>> correct slice 0 layout is imposed. >>> >>> >>> - 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 _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
