Darren,
Thanks for the comments ...
On 02/16/11 04:24, Darren Kenny wrote:
Hi Ethan,
I took a look at your document, in general it looks pretty good...
Naturally though, there are some comments:
5.1.1<configuration> element
You mention here that there are two URI schemas supported, whilefile://
doesn't present a large problem, I'm wondering abouthttp:// URIs, since
right now the AI client doesn't do any fetching from remote URLs - it's
the manifest/locator service that does that.
Is there the expectation that this will be handled by the AI client
itself, or will the manifest/locator service be enhanced to to the
fetching of the URIs by extracting them from the manifest?
The AI client will (via a checkpoint that the AI client runs.) Do you
foresee an issue with that?
5.1.2 Structure of zone configuration files
ai_manifest.xml
Based on the comments in 5.2.1, and given that there is a difference
it the AI manifest tags when applying them to zones, should we
consider having a separate DTD such as ai_zones.dtd?
I don't think so because we actually want to be able to accept a
manifest that is used to install a global, to be usable to install a
non-global zone. The transparency makes managing AI manifests a much
easier on the user. In the design spec, this was eluded to as a future
enhancement, but I've gotten comments from Dave that this might be
desirable now.
So I think we want to keep the definition one and the same for this
purpose instead of separating them.
sysidcfg
The text here implies that sysidcfg isn't being used for S11, which I
believe is correct, but I think it might be worth mentioning
explicitly.
Based on comments from others, I'm just going to remove mention of
sysidcfg from this design spec now.
You mention that the "AI" will need to validate the required files, but
isn't it more correct to say that they will be validated by the
"Configuration Validation" checkpoint?
In this paragraph, I use "AI" to mean the higher level, overall process
of AI. The discussion of the checkpoints hadn't occurred at this point
in the document yet, so I didn' t get into specifics. To me, the "AI
client" is comprised of the checkpoints it will end up executing, so
saying the AI client will do X and Y (but which are really carried out
by some underlying checkpoint) is ok when being described at a higher level.
5.1.3 Processing the<configuration> elements
NIT: In the list of AI->CUD checkpoints, the "Transfer IPS" should really
be
one or more Transfer (not specifically IPS) checkpoints, depending on the
contents of the AI Manifest.
Ginnie document on ICTs is a more complete list of ICT checkpoints now,
obsoleting the list in the AI document. After "Transfer Logs" there is now
a "Create Snapshot" checkpoint (to create the @install checkpoint).
The ICT to Checkpoint document lists all ICTs, but it doesn't seem to
note which ones were applicable to AI and which ones were not. Other
than the Create Snapshot checkpoint, are there any others?
One question I actually had regarding the ICT conversion was that the
SetFlushIPSContentFlag checkpoint, which is in the AI list, is grouped
into the CleanupLiveCD ICT checkpoint. Does AI not need that ICT anymore?
5.1.3.1.1 Validating the zone 'config' files
The references to "AI Client" should most likely be "this checkpoint"
here.
I'll update this.
5.1.3.1.2 Validating the zone 'ai_manifest.xml' files
Based on the comments in 5.2.1, and similar to in 5.1.2 above, I think it
makes sense to have a separate ai_zones.dtd file to handle the syntactic
differences.
[...]
5.1.4 Structure of installed zone configuration files
Should "the installer" be "this checkpoint"?
I'll update this to say, "The Configuration Transfer checkpoint"
5.2 Using the AI client to install zones
In the list of checkpoints to be executed (and thus it's in the list of
checkpoints not to be executed) Target Discovery is missing.
I think that Target Discovery should still be run, but with the
restricting as it is in DC that it only discovers zpools, datasets and
BEs. This would be useful for validation purposes later.
This makes sense, I'll add it to the document.
The zone_rpool_dataset that is passed with the -Z option should be placed
into the Data Object Cache so that checkpoints can find it when required.
Yep, this is noted in 5.2.2
I notice that there isn't a TargetSelection checkpoint being used - it's
at this point that things are moved around between the Target.DISCOVERED
tree and the imported Manifest to the Target.DESIRED tree that TI
requires it's information to be in. This would be where you would add the
new structure of BE and datasets for TI to create for you.
I'll add TargetSelection to the zone checkpoint list. It doesn't sound
like its going differ from global zone usage.
5.2.1 Manifest Validation Zone
I think that this checkpoint wouldn't be necessarily different if the AI
manifest for a zones install was simply a different DTD.
Where there is overlap between the DTDs maybe we could break re-organise
things such that the AI and AI_Zones DTDs should share common elements.
[...]
5.2.2 Target Instantiation Zone
I'm not sure why this is different to standard the standard TI checkpoint.
I can't see good reason that things can't be common if the information
that is passed to TI doesn't tell it to create a new rpool (i.e. the
action != "create", but instead just tells it to add datasets.
Yes, it's possible that the TI code will need changes to do this
perfectly, but I'm not sure it warrants a new checkpoint but more of a
refinement of the existing one.
As mentioned in 5.2 above, you will likely need some checkpoint to create
the Target.DESIRED tree for TI to get it's instructions from.
5.2.2.1 libbe::be_init_nested_be()
I think that the standard TI above could handle this fairly simply if the
BE to be created is a zone BE as opposed to a boot BE. It could even
toggle it's use based on the locating of the zone_zpool_dataset in the
DOC.
What I was trying to avoid is littering our lower level checkpoint code
with the following in *random* places:
if (installing_zone_root())
...
..
else
...
..
Hence the subclassing (or separation of the class, as in the
ManifestValidation case) of certain classes to make it more apparent
and, in the long run, easier to maintain. But perhaps to reconcile
this, I could just augment the effected classes' initialization to
accept an 'installingZoneRoot' optional parameter. Code in the class
would still have the if/else clauses, but at least it's identified at
the class level. For example, ManifestValidation would look something
like the following:
def ManifestValidationCheckpoint(..., ..., installingZoneRoot=False)
Thoughts?
5.2.3 Initialise SMF Zone
Again, I'm not sure if there is a need for another checkpoint - the
difference is very small - and could hinge on the specification of the
zone_zpool_dataset in the DOC.
[...]
5.2.4 Transfer Logs Zone
I think this requirement could be mitigated by the standard CUD checkpoint
just doing the same thing and using /var/run/auto_install.<pid>/* for it's
logs during an install.
I would be interesting to know if people think this would be wrong for a
standard install to use...
Yes, my intention was to make this change as part of the standard
Transfer Logs checkpoint, not just for zone root install. This actually
needs to apply to *any* temporary work files that the AI client, and any
of its checkpoints, need to use. So the change actually spans beyond
just the Transfer Logs checkpoint.
Perhaps the main AI program code needs to set some WORKDIR variable in
the DOC, and all checkpoints need to base off that in creating temp files.
thanks,
-ethan
Thanks,
Darren.
On 14/02/2011 21:39, Ethan Quach wrote:
All,
I have uploaded the design document for AI Zones support into the
caiman-docs gate.
http://src.opensolaris.org/source/xref/caiman/caiman-docs/AI/AI_Zones_Design.odt
General review appreciated, and if so, please try to get me comments by
the end of the week; but I have also pinged individuals to review
particular parts of the document.
thanks,
-ethan
_______________________________________________
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
