Hi Jean,
I mostly have some clarifying questions, with a few comments/fixed typos
thrown in for good measure. Sorry for the delay - there's been quite a
bit to review lately!
Thanks,
Keith
4.1:
1st paragraph: The statement that the "Solaris" partition id is no
longer used because it collides with Solaris2 is only partially correct,
isn't it? What I mean is, even though the new OS our installer puts down
will definitely create a Solaris2 partition, for compatibility reasons
the data structures need to understand that an existing LinuxSwap
partition could actually have Solaris slices on it, which means it can't
easily coexist with a Solaris2 partition. The data classes need to be
aware of this on some level.
Diagram: The text says the "VTOC label is not shown," but slices are
listed under the Solaris partition.
6.1: Text implies that only __setitem__ and insert() are overridden, but
the diagram seems to indicate that several other functions are overridden.
Can you expand on what the ShadowList will be doing? From prior
discussions, I'm guessing it will be asserting that all items in a list
are of the same type, but it's not clear from the usage here if that's
the case.
6.2:
Diagram 1: HolySlice vs. HoleyPartition - 'e' or no 'e' in Holy? What do
those objects represent?
Nit: I don't think disks have an "offset," do they?
Nit: Partition.part_type/part_id -> could these just be "type" and "id"?
As they are attributes of a class, they shouldn't end up clashing with
the Python namespace builtins of "type()" and "id()".
Diagram 2:
Can you explain the "Mixins"? I know I've read about this Python concept
before, but it's still not concrete, and there's no explanation of the
Partition/SliceChildrenMixin classes here.
I'm not certain all the subclasses will be required explicitly. For
example, where the only thing changing is basic values like the "limit,"
once could simply instantiate different objects and have a limit
parameter, rather than an entire class for it. I've not thought about it
deeply, but I almost think a single PhysicalChildList class could be
sufficient, with appropriate parameters for limit, class(es) accepted, etc.
Page 16:
The type() function has very limited value for making logical decisions
(mostly due to complications from subclasses). It'd be best not to
encourage its use in this fashion - isinstance() should work fine here.
Side note: The classes specified here will all include __str__ method
definitions, correct?
7.1.2:
Can you clarify what other factors, if any, would affect the value
returned by get_progress_estimate()? e.g., zpool searching is dependent
on the number of the disks (which may already be known, if disk
searching was already done).
7.2: Nit: Last paragraph would be more helpful if listed first.
7.3: I think it'd be better to have a separate error class for "received
libtd error" and "incorrect input" - suggest perhaps ValueError for the
latter. "DiscoveryError" would also be a more descriptive name for TDError.
Will libtd still be in C? If so, in the last sentence here, "exception
raised" is misleading (error/return code would be more accurate, I
think). If not, I strongly caution against re-masking a Python exception
unless specific additional context can be added.
8.1.1: Nit: If the dry_run flag is set to True, then *no* changes to the
target will be performed
8.1.3: I'm not certain how much value there may or may not be in this,
but would it be possible to send a summary of what was destroyed/created
to the logs, if the install was canceled? It could be convenient to have
an idea of the current disk state, though admittedly the amount of use
that would get is very small.
8.2: So 'destroy' objects will simply be names like "c0t0d0s0", and not
full Physical/LogicalTargets? Would it be possible to accept both?
For "create," what would happen if the disk originally had a Solaris2
partition and a Windows partition, and the object under "create" was
that disk with just the Solaris2 partition (unchanged) with modified
slices? Would the Windows partition be destroyed, left untouched, or
would this be an error?
8.3: Same comments as 7.3
On 07/27/10 12:34 PM, jean.mccormack wrote:
Please review the design doc for the Common Object classes/TI/TD
design located at:
http://hub.opensolaris.org/bin/view/Project+caiman/CUE_docs
We'd like to get comments by COB on Tuesday August 3rd.
Thanks,
Jean, Dave, & Sanjay
_______________________________________________
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
