BIND 10 #2320: Allocation engine v4: Address assignment

[BIND 10 Development]

#2320: Allocation engine v4: Address assignment
-------------------------------------+-------------------------------------
            Reporter:  tomek         |                        Owner:  tomek
                Type:  task          |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp4         |                    Milestone:
            Keywords:                |  Sprint-DHCP-20130122
           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
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => tomek


Comment:

 Reviewed commit 21794c5df650ecf28f55ed95cb12d278945ab3a5

 '''doc/guide/bind10-guide.xml'''[[BR]]
 I've made a tiny change here (s/NACK/NAK/) and pushed the result.

 '''src/bin/dhcp4/dhcp4_messages.mes'''[[BR]]
 Again a textual change here in DHCP4_RELEASE_EXCEPTION: I've replaced the
 work "exception" in the explanation with "error", on the grounds that for
 this usage, "exception" is really a programming term but the audience may
 not have programming experience.

 '''src/bin/dhcp4/dhcp4_srv.cc'''[[BR]]
 >> Line 439: Strictly speaking, the return of "false" from the call to
 deleteLease() just means that the lease was not in the database: it does
 not mean that the deletion operation failed. Such a condition merits a
 warning message, not a debug message.
 > It is currently an ERROR message as it indicates database inconsistency,
 some race conditions once we have multi-process capability or someone
 tampering with the database while we are processing this release.
 The severity guidelines in src/lib/log/README state:
 {{{
 ERROR
 -----
 Something has happened such that the program can continue but the
 results for the current (or future) operations cannot be guaranteed to
 be correct, or the results will be correct but the service is impaired.
 For example, the program started but attempts to open one or more network
 interfaces failed.

 WARN
 ----
 An unusual event  happened.  Although the program will continue working
 normally, the event was sufficiently out of the ordinary to warrant
 drawing attention to it.  For example, at program start-up a zone was
 loaded that contained no resource records,
 }}}
 In this case I would argue that the result for the current operation is
 correct and the program will continue working normally, hence a WARN.
 True, there may be problems in the future but we don't know that - it may
 be just this record is affected.

 However, I won't push this - we can leave it as an error.


 '''src/lib/dhcpsrv/subnet.cc'''[[BR]]
 Fair point about the default values.  However, getting the default value
 through a virtual method would work:
 {{{
 class Subnet {
         virtual PoolPtr getPool(isc::asiolink::IOAddress hint);
         virtual PoolPtr getPool() {
                 return (getPool(default_pool()));
         }
         virtual isc::asiolink::IOAddress default_pool() = 0;
 };

 class Subnet4 : public Subnet {
         virtual isc::asiolink::IOAddress default_pool() {
                 return (isc::asiolink::IOAddress("0.0.0.0"));
         }
 };

 class Subnet6 : public Subnet {
         virtual isc::asiolink::IOAddress default_pool() {
                 return (isc::asiolink::IOAddress("::"));
         }
 };
 }}}
 Although a bit more complicated than the ugly hack, it does have the
 advantage of insulating the base Subnet class from any knowledge of
 subclasses derived from it.

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