BIND 10 #2995: Add selected few (3-4) hoots into DHCPv6 server

[BIND 10 Development]

#2995: Add selected few (3-4) hoots into DHCPv6 server
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp6         |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130717
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  0             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by tomek):

 Replying to [comment:4 stephen]:
 > Reviewed commit commit c6b3ce6d16f92b80db77487900fb57a7a67b3203

 Continuing after [comment:5]

 > '''src/bin/dhcp6/dhcp6_srv.cc'''[[BR]]
 > With the changes listed at the end of this comment, the include of
 server_hooks.h can be removed.
 Changes applied, include removed.

 > Dhcpv6Srv constructor: unless Dhcpv6Srv is a singleton created before
 the configuration file is read and hook libraries loaded, hook
 registration should not be done here.  For a suggested way of registering
 hooks when the program is loaded, see the "Guide to Hooks for the BIND 10
 Component Developer" (in the .dox file) in #2980.
 Dhcpv6Srv is not a singleton, but there is never more than once instance
 at any given time. It is created before configuration file is read and
 libraries are loaded. Anyway, the registration now follows the Guide
 approach.

 > Dhcpv6Srv constructor: the changes in #2980 should mean that there is no
 need to call HooksManager::loadLibraries().  If no libraries are loaded
 when something is called, the framework will set itself up with an empty
 set of libraries (as you have done here).
 Hmm, I've commented it out and now 3 tests fail:
 [  FAILED  ] HooksDhcpv6SrvTest.simple_pkt6_send
 [  FAILED  ] HooksDhcpv6SrvTest.valueChange_pkt6_send
 [  FAILED  ] HooksDhcpv6SrvTest.deleteServerId_pkt6_send
 I do not understand why that is happening. I left it as it is now.

 > Dhcpv6Srv destructor: "todo" comment about deregistering a hook. I've
 been thinking this through and "deregister()" is not needed.  In this case
 there is only one Dhcpv6Srv object which is in existence all the time the
 server is running, so there is no need to deregister them.
 Ok, todo removed.

 >
 > ----
 > In our Jabber conversation a couple of days ago, we discussed hooks in
 an object such as the allocation engine, where we might have multiple
 allocation engines using different algorithms, and choose one based on a
 configuration option.  However, even then deregisterHooks is not needed.
 There are several reasons for this:
 >
 > * Although the allocation engine object is create and destroyed when the
 configuration is changed, the file is loaded when the program is started.
 Therefore the hooks can be registered then.
 >
 > For the the case where we have an alternate allocation engine available,
 then one of the following applies:
 > ...
 Thanks for the thorough explanation. Consider myself convinced.

 > In summary, hook names should be registered on start-up.  They are
 either unique to a piece of code - in which case there is no problem. Or
 they are common to bits of code derived from a common ancestor - in which
 the code should call the hook through a method in the common base class.
 There is no need to deregister the names.
 That makes sense. Hook registration in AllocEngine now follows the Guide.

 Changes checked in (commit-it 682cac0da7b318a81746d93424a638faa867ee7c)

 The review response will resume tomorrow.

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