Ethan Quach wrote:
> 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. 

Scratch that. They do come out ... I thought I wasn't seeing
them before.

-ethan

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

Reply via email to