Ethan,
Please find responses inline.  Took your suggestions unless otherwise noted.

current webrev: http://cr.opensolaris.org/~wmsch/profile4/
differences with last webrev http://cr.opensolaris.org/~wmsch/profile3-diff/

On 02/24/11 06:12 PM, Ethan Quach wrote:
William,

On 02/16/11 07:43, William Schumann wrote:

current webrev: http://cr.opensolaris.org/~wmsch/profile3/
differences with last webrev http://cr.opensolaris.org/~wmsch/profile2-diff/
last webrev: http://cr.opensolaris.org/~wmsch/profile2/

TODO: I'm still hanging onto code that extracts hostname and timezone from 
profiles until which time I'm sure that its removal
won't interfere with testing.

Curious.  Does this code being left in include the current support for the 
embedded SC profile?  I'm noticing that's still in
there and it's something that should be removed by this project.
Indeed, the embedded SC profile code should be removed from the AI client, 
since the AI client code is bound to the AI service image.


AI_database.py:
--------------------
260-264 - I don't get why choose to split up the functions like this:

   numManifests() / numNames()

Why not just create:

   numManifests() and numProfiles()
numManifests was used by existing code, so I kept it. There was no need for a profile-specific version - the generalized (parameterized) version was preferable.

so that the caller doesn't need to know about the defined table names?
to serve the code that is generalized with table name as a parameter.

Same comment goes for:

   getManNames() / getNames()
getManNames - same as for numManifests

It seems you're doing that for:

   getManifestCriteria() / getProfileCriteria()

so why not do it in other places?
because there was no need for a generalized entry point in this case.


275-276 - This comment needs updating.

655 - Should include the ":default" instance name here at least. but preceding 
it with the scheme is also preferable,
"svc:/system/install/server:default"


publish_manifest.py:
--------------------------
59 - Why did you change this line.  It looks correct the way it was.
missing open bracket for -C option.

ai_get_manifest.py
------------------------
995 - is msg.get_filename() the name of the file as it exists on the server?  
If so, I'm wondering if it'd be better to preserve
that naming on the installed system.


installadm.1m.txt:
-----------------------
439 - Shouldn't this line be {-m <manifest_name> | -p <profile_name> ...}

445 - "or or"

537-538 - With -P being provided to validate the given profile against the 
service_bundle associated with some install service,
shouldn't the usage be:

   installadm validate
           -n <svcname> {-p <profile_name> ... | -P <profile_name> ...}
After you and discussed this, I implemented your suggestion. Modified distro_const/checkpoints/pre_pkg_img_mod.py to copy service_bundle.dtd.1 into the AI service image. Modified installadm to require a service, then to look for the service_bundle in the AI service image. Warns if service_bundle is not there.

Thank you,
William

584 - could you leave off the "assigned by DHCP" portion.  Our general netboot 
case uses DHCP, but we have a supported usage
scenario where DHCP isn't used, i.e. the wanboot without DHCP case.  In that 
case, the IP is manually set at the OBP, and is still
what's processed for the sake of Criteria here.  So perhaps this can be changed 
to:

   ipv4 - IP version 4 network address


587 - Accordingly, can you change this to:

   network - IP version 4 network number


597 - "manifest" -> "manifest or profile"

927-935 - Don't we need to specify the service name with -P?  Otherwise, what 
service_bundle are you validating against?



thanks,
-ethan


On 01/28/11 12:23 AM, Dave Miner wrote:
... set_criteria.py
87: Perhaps add a "not both" to the end here to clarify slightly (though, I 
wonder, would it be possible and useful to be able
to do both at once?  seems convenient.)
Added support for both at once.

Thank you,
William
_______________________________________________
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