Hi Drew,
I'm looking through section B (instantiation). I haven't finished physical.py
yet so my comments are in relation to logical.py and instantiation.py:
logical.py
----------
197:
@property
def datasets(self):
The command invoked is "zfs list -H -t fileystem,,,"
This only returns a subset of the potential datasets as it doesn't
include zvols or snapshots which are also datasets. Would it be more
appropriate to rename this property method to "def filesystems(self)"?
It doesn't exactly do what it says on the tin.
219:
Zpool.get()
Method doesn't strip out the header line from the results nor split the
output so that the value can be isolated from the other columns
eg. Here's the output returned from Zpool.get("bootfs")
NAME PROPERTY VALUE SOURCE
rpool bootfs rpool/ROOT/solaris-161 local
class Filesystem: snapname(), set(), get(), create(), exists():
Instead of having to caclulate the full filesystem name in each of
these methods, would it be quicker and easier to just store full_name as
a
property of the Filesystem object?
class BE: 789 def mount() & class LOFI: 878
os.mkdir(mountpoint) assumes the parent directory of mountpoint exists.
Would os.makedirs() be more suitable to create intermediate directories?
shadow/Zpool.py
---------------
35:
""" ShadowZpool - class to hold and validate Solaris x86 Zpool objects.
"""
Is it really x86 specific?
instantiation.py
----------------
63:
self.target = self.doc.get_descendants(name=Target.DESIRED,
class_type=Target)[0]
if not self.target:
raise RuntimeError("No desired target nodes specified")
Is it ever possible for self.target to be None here?
Wouldn't you get an IndexError in the previous line where you lookup index [0]
of the results of self.doc.get_descendants()?
104: if partition_list:
Should this be encased within a conditional for architecture since MBR
partitions are not valid on Sparc? The manifest parser can't enforce this
restriction, is it enforced anywhere else (eg. in Target Controller?)
I'll finish going through physical.py tomorrow.
Thanks,
Niall
--
This message posted from opensolaris.org
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
