Dave,
On 09/15/11 06:54 PM, Dave Miner wrote:
Two things:
- Looks like we have new i18n objects at 678. That seems like a
problem at this stage? Not sure I can approve this way.
As noted elsewhere in the review thread, those
are exact copies of localized strings from elsewhere
in the file, so will not cause i18n issues. I could
improve this by assigning the string to a class variable
(using N_() delayed translation) so it's only referenced
once, but that would involve changing more code
and might be considered riskier? Let me know which
you prefer.
- Seems like we should file an RFE to add a convenience routine to the
partition object to empty itself out and create a single full-size
slice as that's an operation that seems somewhat common. The
implementation here works, but seems less than obvious to a consumer.
Agreed. 7091425 filed.
- Dermot
Dave
On 09/15/11 13:33, 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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
