Joseph J. VLcek wrote:
> 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}.

Clarification: prior to returning after an error is detected.

>
> 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
>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss


Reply via email to