BIND 10 #3281: Subnet reconfiguration improvements

BIND 10 Development do-not-reply at isc.org
Fri Mar 28 17:36:18 UTC 2014


#3281: Subnet reconfiguration improvements
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:
                Type:  defect        |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcpconf      |  reviewing
            Keywords:                |                    Milestone:  DHCP-
           Sensitive:  0             |  Kea0.9-alpha
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  24            |                 CVSS Scoring:
         Total Hours:  7             |              Defect Severity:
                                     |  Medium
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  2
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * hours:  5 => 2
 * owner:  tmark => marcin
 * totalhours:  0 => 7


Comment:

 Really nothing but minor nits:

 -------------------------------------------------------
 subnet_unittest.cc

 {{{
 +    // we use us the one of second subnet. That way we ensure that the
 }}}

 Change "we use us the" to "we use the"

 -------------------------------------------------------

 cfgmgr.cc

 {{{
 +                  << subnet->getID() << "' is the same as ID of an
 existing"
 }}}

 Change "same as ID" to "same as the ID", or even  "is already in use."

 -------------------------------------------------------

 dhcp4/tests/config_parser_unittest.cc

 multipleSubnetsExplicitIDs - not sure why you do this 10 times,
 and I should think twice would be enough.  Why you do the confugre
 once before the loop?   Also you don't need to convert to json each
 time. That you only need to do once.

 -------------------------------------------------------

 dhcp6/tests/config_parser_unittest.cc

 You can remove this todo:
 {{{
      /// @todo: Uncomment subnet removal test as part of #3281.
 }}}

 ----------------------------------------------------------

 I would recommend maybe something more like this for the ChangeLog entry:

 {{{
     b10-dhcp4, b10-dhcp6: added a new parameter, "id", to subnet
 configuration.
     This parameter allows subnet ids to be set to arbitrary values or
     automatically generated values.  Generated subnet ids are renumbered
     each time one or more subnets are removed.  Setting the ids to to
 specific
     values with this parameter prevents this renumbering.
 }}}

 --------------------------------------------------------------------------

 The units all pass under OS-X, I did not run valgrind.
 I did some manual system tests using bindctl and was able to add
 v6 subnets with arbitrary ids, duplicates were not allowed.

 cppcheck for this code was clean.

 Tidy up the nits are you see fit and merge in your changes please.

-- 
Ticket URL: <http://bind10.isc.org/ticket/3281#comment:8>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list