Ethan Quach wrote:
> Please review..
>
> Defect:
> ----------
> http://defect.opensolaris.org/bz/show_bug.cgi?id=7986
>
> Webrev:
> ------------
> http://cr.opensolaris.org/~equach/webrev.7986
>
>
>
> This fix moves the default.xml file from [1] to [2]
>
> [1] SUNWinstalladm-tools -- 
> /var/installadm/ai-webserver/AI_data/default.xml
>
> [2] SUNWauto-install-common -- /usr/share/auto_install/default.xml
>
>
> The entire /usr/share/auto_install directory from the AI image will be
> copied into the top level of the image at build time, so that its readily
> available for the install service to use.
>
> create-service will look for the default.xml file in this directory
> in the image first before falling back to the one on the running system.
> add[-manifest] will look for the ai_manifest.rng file in this directory
> in the image first before falling back to the one on the running system.
>
> If/when this directory doesn't exist in the AI image, it will be 
> copied from
> the solaris.zlib file by create-service (to support older images that 
> wouldn't
> have had the directory already copied into the top level of the image at
> build time.)
>
>
> Scenarios tested:
> --------------------------
>
> New install server w/ new install image
>    - add manifest - manifest gets validated with schema from image.
>    - create-service - default.xml from image is used to set up service.
>
> New install server w/ old install image
>    - add manifest - (to an old image service that had already been set
>      up before new tools present) - no change - falls back to using the
>      ai_manifest.rng on the running system.
>        - add manifest - (to an old image service that was created with 
> new
>      tools present) - add manifest uses the ai_manifest.rng from the
>      image.
>
>    - create-service - default.xml didn't exist in the client image in
>      older builds, so create-service falls back to using the default.xml
>      from the system.
>      The ai_manifest.rng did exist in client images in older builds, so
>      adding manifests in this scenarios does use the ai_manifest.rng
>      from the image.
>
>
> Older install server w/ new install image
>    - add manifest - no change
>    - create-service - no change
>
>
>
> thanks,
> -ethan
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
usr/src/cmd/installadm/setup-image.sh
--------------------------------------------------------------

Issue 1:
--------
 310                 mkdir -p 755 ${img_ai_dir} > /dev/null 2>&1

This will create directory ./755 and ${img_ai_dir}

Issue 2:
--------

Prior to returning you don't clean up ${img_ai_dir}.

Issue 3:
--------

You pipe stderr and strout to /dev/null which could make debugging any issues 
difficult.
To aid in debugging it might be valuable to allow the user turn on a level of 
debug output
that would result in the output of these commands to be logged.

Issue 4:
---------
Instead of using echo for error logging I think you should use 
print_err.

Issue 5:
--------

This code should trap escape. For example in case a user attempted to interrupt 
it by
typing ctrl-C. The code should trap that and clean up, not leaving lofimounts 
and 
directory caid_mnt...




usr/src/cmd/ai-webserver/publish-manifest.py
-------------------------------------------------------------------------

Issue 1:
------------
Question.

 702                 if self._AIschema != None:

Please confirm: Is it really an error to re-set the AIschema? 

 732                         raise AssertionError('Imagepath is already set')

Same question with ImagePath.

Issue 2:
------------

Suggestion: Wouldn't it be more flexible to defines these strings as 
constants in case these file
names and or paths were ever to change?

 709                                     "/auto_install/ai_manifest.rng")):
 711                                             "/auto_install/ai_manifest.rng"
 714                                             
"/usr/share/auto_install/ai_manifest.rng"

usr/src/cmd/installadm/setup-service.sh
---------------------------------------------------------------

Define PATH or use full paths for command
e.g.

cp on lines 302 & 305


Reply via email to