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