On 06/26/12 02:02 PM, Nirmal Agarwal wrote:
Hi all
Can I please get code review for below dhcp related CR's :
7130777 installadm should allow Administrators to disable modification of the
DHCP configuration file
7173043 dhcp smf service goes into maintenance after delete-client
7151276 installadm should try to preserve existing dhcp entry data where
possible
Webrev Location :
https://cr.opensolaris.org/action/browse/caiman/nirmal27/dhcp-crs/webrev/
Testing :
Pep8 clean
Slim test : Pass
Manual Testing :
/net/indiana-build.us.oracle.com/export/home/na210770/ai/dhcp/Manual-Testing.txt
Thanks
Nirmal
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
Hi Nirmal,
General comment:
You are adding new strings beyond the G11n message freeze (b19).
client_control.py
174 To improve readability, can you change the name get_manage_dhcp()
to be something more intuitive so we'd have "if com.managing_dhcp()"
or "if com.dhcp_being_managed()" or something along those lines?
create_service.py
216 The error message says: "DHCP server configuration is disabled."
which made me think that DHCP was disabled. Perhaps change to more
explicitly say that installadm (or AI) updates to DHCP server
configuration are disabled.
216 suggested reword:
Value of all_services/manage_dhcp property of
install/server SMF should be True to manage DHCP server
->
Set the value of the install/server SMF property, %s,
to True to have AI manage the DHCP server changes.
216 Also, please use cli_wrap (cw) for this output
dhcp.py
1065 "we modify host stanza as per service"
Can you expand on this comment? Not sure what that means.
installadm_common.py
987 see name change suggestion under client_control above
989 Please modify comment so it is clear you are referring
to the AI SMF service and not a regular AI service.
993 What do you want the behavior to be if the property
is not part of the service?
server.xml
109-113 please indent to line up with the other all_services properties above
194-206 same comment, indent to line up with properties above
201-203 suggested reword:
Property which defines whether AI related DHCP server configuration is
managed by AI (true) or by administrator (false).
Questions on testing:
1) Have you tried create-client for a u1 service if that same client currently
existed for
an S11 service (and vice versa) with both manage_dhcp = True and False? Did you
ensure that
the contents of the dhcpd conf file (or the output) were correct for both
cases, switching from
supporting uefi to not as appropriate?
2) Did you test update-service?
Sue
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
