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.

- 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.

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

Reply via email to