Hi Alok and Drew,
Here are my comments for files in section C.
usr/src/lib/install_target/shadow/shadow_physical.py:
- insert_slice() function: question: for inserting a slice into a partition,
I think we can also check that the start_sector of the slice is bigger
than the start_sector of the partition.
- lines 215-216: can these 2 lines be moved to the very top of this
function?
There's no point to do all the other computations if this is true.
- line 234: s/continer/container/
- insert_partition() function, the key 15 is used many times in this
function
to access the PARTITION_ID_MAP to map to the extended partition part_type.
It would be good to define this as a constant for better readability.
- insert_partition() function, in varies places in this function, you
determine whether the partition to be inserted is a primary or logical
partition by comparing to hard coded value of 1-4 for primary partitions
and 5-32 for extended partitions. Since there are predefined constants
FD_NUMPART and MAX_EXT_PARTS, I think it is better to use those.
- insert_partition() function, there's no check to make sure that there
are at most 4 primary partitions.
- line 293: partition.name need to be casted to an int?
- line 308: value.name need to be cast to an int?
- line 311: partition.name cast to an int?
- lines 337-338: Question: is it ever possible that the disk has no size?
- line 399: value.name cast to an int?
- line 261: define 11 as a constant?
usr/src/lib/install_target/test/test_shadow_list.py:
TestZpool() tests:
- For delete zpool, perhaps also add tests for deleting all the added
pools, and
double deleting a pool?
- For delete vdev, add tests for deleting all vdev, and double deleting?
TestZFSFilesystem tests:
- Similar to above, delete all ZFS filesystem, and double deleting?
TestZvol: some additional test cases?
- Add a zvol that's bigger than the pool?
- Add 2 zvols whose total size is bigger than the pool?
- add zvol with size 0?
- add zvol with negative size value?
TestPartition tests:
- line 549, 580-581: remove comment?
- Some suggested additional partition tests that comes to mind:
* Add more than 4 primary partition
* Invalid partition id
TestLogicalPartition: additional suggested test cases:
* Add a logical partition with a size that's bigger than the extended
partition
* invalid logical partition ID, like "50"
TestFinalValidation tests:
- I remembered we decided that final validation would check for
a whole bunch of additional things, such as having 1 partition of
type Solaris, and having a root pool defined...etc... If those
things are checked, test_simple, test_simple_disk_with_one_partition,
and test_simple_disk_and_zpool should all fail. Furthermore, we
need to add more tests to make sure that those are indeed checked.
Thanks,
--Karen
On 02/24/11 12:33, Alok Aggarwal wrote:
We would like volunteers to review the TI/TD project. The
review is being broken down into the following chunks to
aid the review process.
a) Target discovery
b) Target Instantiation
c) Target controller API and target validation
d) ctypes interfaces
e) Other
Webrev location:
http://cr.opensolaris.org/~aalok/cud_ti/
Please let us know which of the above sections you'd like to
sign up for, an approximate list of files corresponding to each of the
review sections is included below for your convenience.
We would like your comments back by March 10.
Thanks,
Drew, Alok (and Jean)
(a) usr/src/lib/install_target/td.py
usr/src/lib/install_target/test_td.py
usr/src/lib/install_target/vdevs.py
usr/src/lib/install_target/size.py
usr/src/lib/install_target/test/test_zpool_vdevs.py
b) usr/src/lib/install_target/ti.py
usr/src/lib/install_target/physical.py
usr/src/lib/install_target/logical.py
usr/src/lib/install_target/test/test_target_instantiation.py
usr/src/lib/install_target/test/ti_full.py
c) usr/src/lib/install_target/shadow/*
usr/src/lib/install_target/test/test_shadow_list.py
d) usr/src/lib/install_target/libadm/*
usr/src/lib/install_target/libbe/*
usr/src/lib/install_target/libdevinfo/*
usr/src/lib/install_target/libdiskmgt/*
usr/src/lib/install_target/libnvpair/*
e) Makefiles, DC changes, MP test changes, engine and
errorsvc changes, packaging
_______________________________________________
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
