Jan,
Thanks for the review...
jan damborsky wrote:
> Hi Ethan,
>
> please see my comments below.
>
> Thank you,
> Jan
>
>
> I have general comment for DC changes. Since installadm(1M)
> tools will take care of populating auto_install directory if it
> doesn't exist, does DC need to take care of it as well ?
The original intent was to do it only in DC so that installadm
has to do less work, but then I realized to support older build
images, installadm should also do it as a back up, since older
images won't have the directory there.
So the fact that installadm does it was supposed to be
secondary. In the future, if we change our scripting to just
loopback mount the AI iso instead of cpio'ing it, (an idea we've
thrown around before), it'd be better that these images have
them there since the imagepath won't be writable, and we'd
still want this support for those images.
The alternative would be to just copy off the needed files
away from the imagepath altogether, into some per-service
configuration location on the server. But if we every wanted
to change to do this, it wouldn't be incompatible with either
change that we do now.
>
> publish-manifest.py
> -------------------
> 70 - just a nit - could you please add short description for
> new introduced parameter (image path) ?
yes, will add.
>
> setup-image.sh
> --------------
>
> generic comment - stderr of CLI used is redirected to /dev/null
> (e.g. 310, 327, ...).
> I am thinking if it might be better it goes to the console,
> since if something goes wrong when those commands are invoked
> it might be easier to debug potential issue.
It's kinda moot. These scripts are all exec'ed from installadm.c
so none of these are currently seen anyway. But you're right,
it probably makes sense for some places to allow stderr to go
to the console, but not all. I'll update the calls at lines 317, 327,
334, 345. That sounds reasonable?
One thing I'm failing to capture is the return code at line 340.
I'll add this. Also, I'm going to redirect 340 to /dev/null since
we don't really want to see that ugly "records in/records out"
output. (If we ever call these scripts directly)
>
> 319 - caid_zlib is not defined, maybe ${target}/solaris.zlib
> could be used instead
yes.
thanks,
-ethan
>
>
>
>
> On 04/14/09 08:46, 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
>
