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


Reply via email to