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