Niall,
This looks good. Mostly just a nit or two...
disk_window.py
858: can just do: if not new_size_byte
864: s/artefacts/artifacts
-Drew
On 3/5/12 8:03 AM, Niall Power wrote:
Hi,
I'd like to ask for a couple of reviews for the following few bugs:
A. 7151031: Text Installer resizes Solaris partition AFTER final
validation
UEFI/GPT development specific (no CRs)
B. Text installer doesn't calculate the maximum available GPT
partition size correctly
C. ui_object is unitialised in validate() method.
7151031 is described in bugster. The proposed fix is to side step the
timing issue altogether by not resizing the partition in
on_exit_edit() but instead do it much earlier when validating the
numeric input into the edit field. This results in a slight
behavioural difference but one that is an improvement IMO. It has the
following benefits:
- solves the key problem of on_exit_edit() resizing the partition
after final validation. All size modifications will be completed prior
to final validation.
- Improves the interaction of the text UI as available size field for
other partitions/slices is adjusted and update instantaneously as user
edits the partition size. UI is more responsive now.
- Resizing errors are caught earlier.
Changes are to disk_window.py.
B. The problem is in ti_target_utils.py:UIGPTPartition.get_max_size()
at line 671+
The method tries to calculate free space by examining start and end
sectors of adjacent partitions as well as the end sector of the
enclosing disk. This is wrong because it fails to accound for the
sectors at the beginning and end of the disk occupied by the primary
and backup GPT tables. These sectors are not available for partitions
to consume and this causes resizing to raise an InsufficientSpaceError
exception.
Besides that, iterating through the UI objects to look for partitions
is a bad idea. The DOC should be used as the definitive reference when
searching for free space. From my own in use observations, this method
is unreliable and yields incorrect results for available size. It is
just plain broken.
The solution is to replace the implementation with one based on the
get_gap() methods provided by the target physical GPTPartition object.
These methods account for the start at end sectors reserved by the
primary and GPT table headers and take these calculations out of the
hands of text install.
I think the get_max_size() methods for the DOS partition and Slice
objects (UIPartition and UISlice) could do with some improvement too
by the way (I logged 7151025).
Changes are to ti_target_utils.py
C. The problem here is that a block of code was accidentally removed
from a previous commit. ui_object in PartEditScreen:validate() would
be unitialised before reference on SPARC at line 294.
Changes are to partition_edit_screen.py
Webrev at:
http://jurassic.us.oracle.com/~npower/webrevs/webrev-7151031-gptmaxsize/
Thanks!
Niall
_______________________________________________
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
