Hi Sarah.
Here are my comments on the "New Solaris Install Architecture
Specification". Seems like you aptly summarized the impossible here,
with a good balance of one-size-fits-all generality to suit the engine
combined with enough details to describe it.
General: Driver Update is not mentioned anywhere, but it is ancillary to
getting the system to an installable state. Does it need to be?
Section 1:
second to last bullet: "bootable media formats" instead of "media formats"?
second to last PP: semicolon after "installing a set of packages"
second to last PP: 1st sentence was confusing: Could the first type of
components be described as "... a set of application components that are
used to implement higher-level functionality..."? (I assume that
installing a set of packages is an example here because this
functionality is layered on top of the pkg(5) command itself.)
Section 2:
I would restructure the first PP since "various components provided by
this project" leaves a hole which is filled in later, which made it
confusing to me. I would say something like "The core engine classes
provide functionality needed by an application's specific components
(provided by various subprojects) as well as the common application
classes. Not sure how to integrate the installation flow concept into
this if you want to keep it though.
I've noted next to the three bullets:
- first bullet is "pieces of an installation"
- second bullet is "communication between the pieces"
- third bullet is "external communication from pieces to the outside"
Including this may be useful, to help simplify what is being explained.
Section 2.2:
second bullet: sounds too specific to AI. Leave AI out of it, and just
say "manifest".
retrieve_data(): This methods provides -> This method provides
Below last clump of three bullets: This method -> These methods
Section 2.3:
Anal nit: just above the bullets: A Logger -> A Logger object
Section 3:
Second PP: Why limit yourself: second sentence: "AI and DC currently
utilize XML manifests."
Third PP: clarify what a target is: "Target Discovery provides the
initial system discovery of disks and partitions available as potential
installation targets."
Fourth PP: I don't think common application class needs to be
capitalized anywhere. If you decided to capitalize anyhow, "class" is
inconsistently capitalized.
Section 3.2:
First PP: I would delete the word "logical", as it hasn't been defined
yet. (It is defined in the next PP.)
Second PP: Add "boot environment" to the list of logical objects listed
in the first sentence.
Last PP before 3.2.1: First sentence seems to detailed to me. I think
you are saying that the objects need to be initialized to represent a
certain type of target. This is done with a seed value which is a
DeviceTarget or a LogicalTarget specification.
The last sentence is "See section 3.2" but we're already in section 3.2.
Section 3.2.1:
Second sentence:
"These targets are represented by two class types: those..."
Section 3.2.1.1:
General:
Do we want to assume people know the connection between disks,
partitions and slices?
Disk class: It is unclear what set/get_assoc_parts() and
set/get_assoc_slices() do. Other functions are clearer only because of
my background. Others may not know about them either. Probably a good
idea to have a quick description of what all methods do (though not
necessarily an individual description for each one).
OK... now I see it. May be less confusing to drop the "assoc" and just
call set/get_parts() and set/get_slices().
What is the difference between get_assoc_parts() and get_part_map()?
set_assoc_parts() and set_part_map()?
set_part_map() and create_part_map()?
Slice class:
Does get/set_assoc_part() make more sense than get/set_assoc_parts()?
3.2.1.2:
The listed logical device base classes are out of order from the rest of
the document, and Dataset class is missing.
Zpool
LogicalVolume
BootEnv
Filesystem
Dataset
VMEnv
Zone
LogicalTarget interface methods:
get_assoc_container() PP: last sentence: "This method allows us..."
Zpool class:
PP below set_props: It says "Thus one method to manage these is
sufficient." Yet, there are two methods listed :)
Dataset class:
First PP:
First sentence: get rid of the "s" in datasets.
Third sentence: "interfaces" -> "methods" to be consistent.
VMEnv class:
The content of this PP seems tilted toward xVM. What about Virtual Box?
LDoms?
The VMEnv class has no methods. Is the point of the VMEnv class to
"mark" an environment as a virtual machine environment? Is it an
interface spec which can be combined with other interface specs in this
section (e.g. LogicalVolume that defines a lofi-mounted file enveloping
the VM environment)?
If VMEnv can be combined with other interfaces, it seems unique in this
respect. Other interfaces, even logical ones, define "physical"
attributes (e.g. snapshots of boot environments and zfs datasets).
VMEnv would rely on the "physical" attributes of what it is combined
with to define its location characteristics, and VMEnv would serve only
to mark an environment as a Virtual Machine.
If this is the case, you may want to call this out in the document. If
not, please clarify in the document how it is used.
4.1:
This is a public document, and the link to securewiki is a sun-private link.
4.2.1:
Drawing 4.2:
The sequence diagram appears to have several mismatched or unmatched
parentheses after method names. Some methods have [], some have (],
some have only (.
I don't understand the store_data() arrow going from Target
Instantiation (TI) to Data Collection coming before the
get_data_collection() arrow going from TI to the Execution Engine?
Ditto for Transfer, Instance Configurator. (Looks like Target Discovery
got it right, though.)
Looks like Instance Configurator block got pasted twice.
4.2.2:
Drawing 4.3:
The "P" of "public" is missing or partly missing in many places.
All methods have no closing ).
I'm a newbie at UML. That said, I interpret the rightmost :disk,
:partition and :slice columns to be loop parameters (i.e. :disk means
"loop for each disk found"). If this is incorrect, please let me know
and disregard the following:
I think that when you look at the columns, the partition loop envelopes
the slice loop. (That is, the right most :partition column represents
the looping through all partitions, and following that column down, we
enter the slice loop for that partition.) This seems correct to me.
But the box enveloping the slice loop is outside the box enveloping the
partition loop. Shouldn't the slice loop box be inside the partition
loop box, much like the partition loop box is inside the disk loop box?
4.2.3:
Drawing 4.4:
Is the reason why there is no instantiate() under the :disk loop because
the disk is a physical medium? To me, instantiate() creates an object
representation of something (e.g. an object representing a disk), as
opposed to the thing itself (e.g. the disk). If this is the correct
interpretation, there is a missing instantiate() in the :disk loop.
5.1:
Item 4: It may be that this level of detail is needed, but to say
"reverse any media-specific customizations" sounds very specific (e.g.
undoing what slim-utils does to set up "jack"). Would it be better to
say "customize the installation to the application's requirements"?
Drawing 5.1: I think the XML Manifest shown in the picture represents a
saved file. If this is the case, remove "DOM". DOM pertains to the
representation of the data in memory, not in a file.
5.2:
Nit: extra space between "via" and "a" in first sentence.
Item 1: First sentence: "...Manifest Retrieval, Manifest Parsing and
Target Discovery." (Target Discovery needs to be added.)
Second sentence: Do you mean the manifest name is fetched from a service
property, and the service having that property is specified in the grub
boot properties? If yes, can that be made more clear?
Drawing 5.2: Same comment about "DOM" as for drawing 5.1
5.3:
Item 1: May wish to clarify that the "target" being instantiated here
refers to where the image will be built.
Drawing 5.3: Target discovery not needed in the diagram.
5.4:
Seems target discovery is needed to find where to plop down the boot
image. It is missing from the text. (It is in drawing 5.4 though.)
Thanks,
Jack
On 12/01/09 14:18, Sarah Jelinek wrote:
> Hi Caimaniacs,
>
> I know you have been waiting for this bestseller to hit the shelves! :-).
>
> The Caiman team has been working on an updated architecture and we
> have the architecture document ready for review. The
> opensolaris-caiman project architecture page is located here(formerly
> known as Caiman Unified Design):
>
> http://hub.opensolaris.org/bin/view/Project+caiman/CUD
>
> The Caiman architecture document open for review is located here:
>
> http://hub.opensolaris.org/bin/download/Project+caiman/CUD/caimanarchitecture.pdf
>
>
>
> Please send comments/questions to caiman-discuss by 12/18/09. If you
> need more time please let us know, with the holidays coming up we may
> have to extend the review period.
>
> Thank you for your time.
>
> Regards,
> sarah
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
