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}. 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