Hi Alok, Thank you for the review. See below.
Alok Aggarwal wrote: > Hi Sue, > > On Mon, 1 Dec 2008, Sue Sohn wrote: > >> Please review the changes for: >> >> 4111 having multiple install services all said "Opensolaris" in grub menu >> http://defect.opensolaris.org/bz/show_bug.cgi?id=4111 >> >> 4553 create-client returning non zero exit code. >> http://defect.opensolaris.org/bz/show_bug.cgi?id=4553 >> >> 4559 create-client: succeeded when invalid mac addr was given. >> http://defect.opensolaris.org/bz/show_bug.cgi?id=4559 >> >> 4819 add/remove/delete-client: getting full usage message >> http://defect.opensolaris.org/bz/show_bug.cgi?id=4819 >> >> 4534 create-service: target directory is created upon failure with >> non-existent >> or invalid image. >> http://defect.opensolaris.org/bz/show_bug.cgi?id=4534 >> >> >> which are posted at: >> >> http://cr.opensolaris.org/~sohn/4111_4553_4559_4819_4534 > > ai_bootroot_configure: lines 43-51: need to update the comment > to account for the changes you're making Thanks, I've updated them. > installadm-common.sh: line 118-119: should not use $dir here, > depending upon the context this could be wrong (if another > function were to start calling get_relinfo). It would be > better to set say releasepath=$1/.release and use that instead. Fixed. > installadm-common.sh: lines 119: do you really want to display > a version such as - > > OpenSolaris2008.11snv_101X86 > > ? It will not look that way - double spaces are removed, not single spaces. So it essentially strips the leading spaces. > That doesn't look too pleasing. What is $VERSION if the .release > file doesn't exist? VERSION will be "OpenSolaris" in that case (defined at the top of the file). > create-client.sh: lines 246-248: This will change when defect 5053 > is addressed but in the interim, maybe the following error message > would be more appropriate? > > echo "${myname}: ${IMAGE_PATH}/solaris.zlib does not exist. " > "The image provided is not an OpenSolaris image" Fixed, with this wording for the second line: "The specified image is not an OpenSolaris image." Please see the updated webrev, same location. Sue
