Hi Jean,
On 08/ 3/10 12:34 PM, Sarah Jelinek wrote:
Hi Jean, Dave and Sanjay,
Below are my comments. I actually did not look at others comments so
I may have repeated some. I wanted to look at this without being
influenced by others comments.
thanks,
sarah
******
Page 4:
This sentence:
Solaris is installed in a partionsmarked "Solaris 2".,
should be: in a partition marked...
Will be fixed.
Page 5:
"The figure above is an example of an FDISK label with an
extended partition. A logical partition has been designated as a
Solaris 2 partition.
A VTOC label is used to further subdivide the logical partitions into
slices.
Note: This should be the ¨figure below...¨
Sarah, I think you're looking at the wrong version of the doc because
the version we released has this section on page 4 and the wording was
changed to below. We'll continue looking further at your comments. If
the current version has already been changed we'll state this.
Page 7:
"Once a device or set of devices have been discovered, the next phase
is to
determine the logical components."
Note: Seems to me that a better wording would be something like:
Once a set of physical devices have been discovered, the next phase
is to discover
the logical devices that are comprised with the physical devices.
The existing wording makes it seem as if the logical devices are
components of the
physical devices, which they are not.
Will change the wording. Good comment.
Section 5,Page 7:
The notion of a ZFS dataset residing on top of a pool seems strange
to me. ZFS
datasets live within a pool, not on top of a pool. I get what you are
trying
to describe here, but the description of this doesn't seem to fit the
design
of the zpool/zfs dataset relationship.
We'll change it. Thanks for the catch.
Section 5.1, Page 8:
The decomposition diagrams seem not quite correct. The diagrams
should show
zpool at the top, and the decomposition moving downward, since we are
trying
to reduce the zpool to its original components, right? These diagrams
look
like composition to me.
The current version shows it this way, zpool on top with datasets
underneath.
So, I think I figured out the problem.. I have been looking at the .pdf
version and it appears it has not been updated along with the .odt
version. And the version number is still the same on both so when I
brought them up I didn't see a version change. Sigh.. anyway, I can see
that the decomposition looks ok now.
The simple way you show the zfs decomposition doesn't really work for
me. It
doesn't affect the overall design but if we have the diagram we have
for the SVM decomposition we should have something similar for ZFS.
We actually think the SVM decomposition diagram is incorrect and needs
to be more
similar to the current zfs decomposition. It will be replaced.
I can see in the new doc the zfs decomp is much better and reflects more
of what I was expecting.
Page 11, Section 6:
General: This section could use a lot more descriptions for the
classes, relationships
and how these are used. I can't really get how this fits, except at a
basic level
of the physical and logical devices, together.
Specific:
"On Solaris, this results in the user selecting a slice or a
partition on a disk to
install the release."
Actually, the user can select a disk, a slice, a partition or a zpool
to install
the release.
Correct. We need to change that.
Page 12, Section 6.2:
What are HolyPartition and HolySlice objects and what are they used for?
They are to hold the information about the holes between partitions
and slices.
That way all the applications don't need to calculate these spaces.
Why do we have a PartitionChildren object?
Because EXTDOS can hold logical partitions.
What is the ShadowList? How is this kept in the DOC and why do the
objects
in in the UML diagram on page 13 have to_xml() methods as well.
Basically, I don't
understand where the DiskSliceChildren, etc... come in and how this ends
up as a valid xml document at the end. Can you provide more details
on this?
The ShadowList isn't in the DOC per se, but rather replaces a
DataObject's children.
Basically it's replacing the normal "list" functionality with our
"list" functionality.
The ShadowList shouldn't have to_xml. We'll fix it.
Page 12, Section 6.3, Logical Target Inheritance:
The architecture document indicates that we should have a
LegacyFilesystem class. The
diagram you show on this page has a UFS class which inherits from
PhyAggregate. I
don't know what PhyAggregate is supposed to represent. Can you describe?
You're looking at the old document. Current doc doesn't have
PhyAggregate anymore.
I see that it is removed. However, the document does not outline all the
classes, like LegacyFilesystem class. It should.
If we had other filesystem types, such as PCFS, how would they fit
into this model?
We can easily add on another subclass of LogicalTarget (like we have
UFS) that would accommodate PCFS.
This will make more sense if you reference the current doc.
Sure.. I can see how this would work, but I think the design document
needs to include the data classes that are being provided.
What is a LimitedSliceList class and how is it used?
Again, wrong doc.
What about ZVOL's?
ZVOLs would be under PhysicalSpecification.BlockFile like a lofi device.
Then this should be noted.
We have a SnapshotList class but no Snapshot class.
Yes. We're missing that. Nice catch.
Page 16, Section 7.2: DOC requirements:
-how/where does the DOC specify the starting point? As a parameter to
the 'execute()'
method or is this something the TD execute() method looks up in the DOC?
start_point is in the DOC so TD execute() expects to find it there.
Karen made the suggestion to make this an argument to execute which we
are considering.
Who specifies the start_point? The engine?
What is the discovered_target child node? I don't see mention of it
in the object
models you have presented prior to this section. How does it fit in
to the object
model?
It's the result of the Target Discovery. It's the root node of the
tree that target discovery created.
The object models represent the class hierarchy which constrains the
makeup of the tree.
We should provide an example tree here.
I think you actually need to provide more details of the actual classes
you will be implementing. Even if the discovered_target_child node is a
result of the target discovery, it should be discussed in this document.
It isn't obvious where things like this come from.
Page 17, Section 8.0, Target instantiation:
Why would we need to have a 'destroy' function? The data specified
for TI should
be complete, in that we either create, use, preserve or remove
targets, but it
isn't a destruction process really, it is a process of creating the
target
the way the user/application has specified it.
Dave will think about the concept of providing
create/use/preserve/remove flags to each target instead of the destroy
function. Seems like it could be a better way of solving the problem
but needs some thought.
I don't think having a 'destroy' is correct for TI. It really has to be
a create of some kind. Even when the user says delete say a partition,
it really is the act of creating the new partition table that removes
the old partitions.
TI must be able to create pools as well.
I believe we allow that but it wasn't stated in the list in section
8.1.1. Will add it.
Section 8.2:
"The TI module will require the DOC to contain a s"
What is this supposed to say?
See real doc.
What is the discovered_target object in this section?
The discovered target from TD.
I assume you mean something
like a 'created_target' object. How does this fit in to the object
model for
target devices?
We're a little lost on what you're referencing so it would be a good
idea to check out the correct version of the doc and see if there are
still issues.
General object model comments:
I don't see the following logical target classes:
Zone
VMEnv
LegacyLogicalVolume
They need to be added.
Also, many of the methods defined in the Caiman architecture document
are
not shown in your design doc. It may be that they are there, or that
you have
made changes from the architecture but either way we should account
for these. Perhaps
in an appendix or something. If these are private interfaces then
fine, but
it isn't clear to me what has been explicitly changed, what is
private and what
is just not shown.
These need to be updated/added. The doc was put out without them to
get the review process started. The pictures aren't perfect but we'll
work on them.
It's not just the pictures, it is the expression of these in text that
need to be added.
I don't see how the BootEnv class could be used for other versions of
Solaris that
are not Solaris 11. We need the ability to discover these as part of
the s10->s11
coexistence. It may just work, but I note this because it isn't
obvious it will work.
BootEnv is no longer in the doc it's just an attribute of a dataset.
So, it would be good if you could update the version when you rev the
document and update the .pdf version when you update the .odt version.
I will re-review the latest version so I can give you a more up to date
set of comments.
thanks,
sarah
*****
Jean & Dave.
-----------------------------------------------------------------------------------------------------------
On 07/27/10 01: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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
