Ethan, Looks good now.
- Sundar Ethan Quach wrote: > > > Sundar Yamunachari wrote: >> Ethan Quach wrote: >>> Sundar, >>> >>> Thanks for the comments .... >>> >>> >>> Sundar Yamunachari wrote: >>>> Ethan, >>>> >>>> *usr/src/cmd/installadm/create-client.sh: >>>> >>>> *199: Just a nit... The comment doesn't sound right. >>> >>> # If IMAGE_SERVER is not empty, this means it was passed in by the >>> # user. So if its not empty, check that its equal to the local system >>> # since we don't yet support a remote system being the image server. >> I was thinking more like the wording "If IMAGE_SERVER was passed in, >> check that its the local machine". The "its" here means "it is"? > > Ah okay :-) > > Webrev is updated, and merged with the 7218 push. > > Also updated setup-dhcp.sh:309-327 to only create the network > table in the DHCP server with a router if the router found is for > the network we're configuring. If not, its not used, and a message > is displayed. > > > Fyi, retested everything after doing a post 7218 convert, > (which worked quite well, though it took enabling one service > for all of them to get kicked online.) Also filed 7982 which > seems to have been broken by 7218. > > > thanks, > -ethan > >>> >>>> >>>> *usr/src/cmd/installadm/installadm-common.sh:* >>>> >>>> 102: Check whether ipaddr is non-null after line 98. If it is null, >>>> this will result in syntax error. >>> >>> Will do. >>> >>>> 122: Validate ipaddr and netmask >>> >>> Will do. >>> >>>> 123: Can you add some comment to indicate what bitwise_and does and >>>> why you are doing it? >>> >>> bitwise_and() ands together the lower four bits of the two >>> decimal values passed in, and returns the result as a decimal >>> value. I'll add this comment. >>> >>> Fyi, this whole get_network() routine was taken from an existing >>> script that we deliver with jumpstart servers, "chkprobe". >>> >>>> >>>> *usr/src/cmd/installadm/installadm.c* >>>> >>>> Are these changes part of your bug fix? It looks like saving >>>> service data is moved up. >>> >>> Yes, they are part of this bugfix. I've moved registering the service >>> up above dhcp-server configuration so that upon any dhcp-server >>> configuration failure, one can cleanly do a "delete-service". >>> >>> It makes sense to move it up there anyway because at that point, >>> we've created the target image and have set up on the service >>> on the server already. >> okay. I wanted to make sure that it is deliberate and not webrev >> problem. >> >> - Sundar >>> >>>> >>>> *usr/src/cmd/installadm/setup-dhcp.sh* >>>> >>>> 63: Make sure that IP address is non-null >>> >>> Will do. >>> >>> >>> thanks, >>> -ethan >>> >>>> >>>> - Sundar >>>> >>>> >>>> Ethan Quach wrote: >>>>> Can I get a review for these blockers. >>>>> >>>>> >>>>> Webrev: >>>>> ------------ >>>>> http://cr.opensolaris.org/~equach/webrev.5589.7797.7226/ >>>>> >>>>> Defects: >>>>> ------------ >>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5589 >>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7797 >>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7226 >>>>> >>>>> >>>>> >>>>> thanks, >>>>> -ethan >>>>> >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> caiman-discuss at opensolaris.org >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>>> >>