Ethan,

thanks for the review :


On 06/10/10 03:46 PM, Ethan Quach wrote:
Hi Matt,

Overall looks good.  I much appreciate you noting the behavior of
the how some of the existing definitions work as well.  I have a couple
comments below, on the schema.


page 10 - "zpool_properties" and "zpool_file_system_properties"

   * Can the "zpool_" portions of their element names be removed
      since they are already children of <zpool>?

   * Same comment for "zpool_vdev", "zpool_dataset", and
     "zpool_dataset_properties"



Yep, not a problem, and makes sense.

page 10 - Target definitions

Not mentioned here, but appearing in the actual schema later on,
is the <zpool_target> element.  Initially, it seemed like an unnecessary
additional layer underneath <zpool_vdev>, but looking at it more
it seems we need it so that we can correlate sets of <ai_target_device>,
<ai_device_partitioning>, and <ai_device_vtoc_slices> together.
Was that the reason, or is there something else?


An omission on my part, should be included in this section. Usage is exactly as you state allowing for the grouping of target/partition/slice specifications.

I wonder if perhaps now is the time to simply merge with the
element tag names that Sarah is defining to replace these existing
target tags in the new AI/DC schema design, and just use those anew
here in your work instead of trying to reuse <ai_target_device>,
<ai_device_partitioning>, and <ai_device_vtoc_slices>.  It would
be great if the resultant instance files which support multi disk/pools
look the same.

(It was known that those existing element definitions didn't quite
fit well with being used to support multiple disks.  Namely, that
partitioning and vtoc_slice information should be children of the
target_device, not peers)


I will look into this more when I return from vacation.


page 10 - It would seem more natural that "type" and "name" simply
be attributes of the <zpool_dataset> element.  Was there a reason why
you made two additional subelements levels to define them?


dataset_type is used to encapsulate the choice of whether a filesystem or a volume is being created. The reason for the encapsulation is if it's a volume there are two sub elements which are required to be grouped together, volname and volsize.

In thinking about it more, this could be simplified and remove the dataset_type completely, and just have "name" as an attribute. The name would pertain to both dataset or volume name.

Then make volsize and optional sub element, and dictate it's usage to be, if specified then it's a zfs volume being created. If not specified it's a filesystem.

Schema would look something like this :

<zeroOrMore>
<element name="dataset">
<oneOrMore>
<interleave>
<attribute name="name">
<text/>
</attribute>

<!--
Optional element volsize, if specified creating a ZFS volume.
                     If not specified, then assume creating a filesystem.
                  -->
<optional>
<element name="volume_size">
<unsignedLong/>
</element>
</optional>


<optional>
<element name="properties">
<text/>
</element>
</optional>

</interleave>
</oneOrMore>
</element>
</zeroOrMore>


This would simplify the dataset specification somewhat. What do you think ?

cheers

Matt


thanks,
-ethan


On 06/08/10 05:10, Matt Keenan wrote:
Apologies,

Forgot to add external link :

http://hub.opensolaris.org/bin/download/Project+caiman/auto_install/ai-multi-design-0.1.pdf

cheers

Matt

On 06/ 4/10 08:53 PM, Peter Tribble wrote:
On Fri, Jun 4, 2010 at 4:50 PM, Matt Keenan<[email protected]> wrote:
Hi,

First draft of the Automated Installer Multi Disk/Pool Support Design v0.1
is now available for review at :


https://securewiki.sun.com/download/attachments/78512557/ai-multi-design-0.1.pdf

That would be available for review by Employees only, I take it?


_______________________________________________
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

Reply via email to