Matt:
Here's my code review comments. Most of the changes I suggested for
varshared.py are just optimizations to make the code more efficient.
target_selection.py
-------------------
1359: change to 'if not vdev_devices:'
varshared.py
------------
28: doc string is referencing varshareddataset.py rather than varshared.py
91-104: can't this be two simple get_descendants calls?
def in_dataset_list(self):
for cl_type in [Filesystem, Zvol]:
dslist = self.target.get_descendants(cl_type=class_type,
name=dsname)
if dslist is not None:
return dslist[0]
return None
This would also allow you to get rid of self.zvol_list and
self.filesystem_list, although you might have to return the class_type
it was
found under to help with your error messages.
116-120: Since there can only be one root_pool in the DESIRED tree, it
would
make sense to make a class property which finds the root pool. This would
prevent multiple lookups of the pool:
@property
def root_poot(self):
if self._root_pool is not None:
return self._root_pool
for root_pool in [z for z in self.zpool_list if z.is_root]:
self._root_pool = root_pool
else:
self._root_pool = None
Then, line 122 becomes:
if self._root_pool is None:
178-223: This is essentially a complete copy of process_var() but with
SHARED_DATASET_NAME and SHARED_DATASET_MOUNPOINT. Can you make a generic
funtion that handles both cases?
for name, mountpoint in [(VAR_DATASET_NAME, VAR_DATASET_MOUNTPOINT),
(SHARED_DATASET_NAME, SHARED_DATASET_MOUNTPOINT)]:
<all of the code>
229: You never do anything with dry_run so don't bother with this line
-Drew
On 7/7/11 8:31 AM, Matt Keenan wrote:
Hi,
Can I get a code review for bugs :
7048015 Automated Installer should create a separate /var and shared
area
7049157 Text installer should create a separate /var and shared area
7049160 GUI installer should create a separate /var and shared area
http://monaco.sfbay.sun.com/detail.jsf?cr=7048015
Webrev :
https://cr.opensolaris.org/action/browse/caiman/mattman/7048015.7049157.7049160
All three installers need to create /var within the installed BE and
/var/shared globally available. This is achieved by adding two
Filesystem objects to the DESIRED root Zpool object before Target
Instantiation is called. TI will then simply create them.
A new checkpoint "VarSharedDataset" is being created to handle the
additions, and will be called by all three installers.
cheers
Matt
_______________________________________________
caiman-discuss mailing list
caiman-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
caiman-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
