BIND 10 trac2342, updated. b0ed08d21421ca321502cda1e9811a4bfe560275 [2342] Address review changes (in comment:5 of the ticket)
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Oct 24 13:14:34 UTC 2012
The branch, trac2342 has been updated
via b0ed08d21421ca321502cda1e9811a4bfe560275 (commit)
from 15bc004f2076bd04df69df0efa59728b795ee7c5 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit b0ed08d21421ca321502cda1e9811a4bfe560275
Author: Stephen Morris <stephen at isc.org>
Date: Wed Oct 24 14:12:06 2012 +0100
[2342] Address review changes (in comment:5 of the ticket)
-----------------------------------------------------------------------
Summary of changes:
configure.ac | 15 ++----
src/lib/dhcp/Makefile.am | 2 +-
src/lib/dhcp/dhcpdb_create.mysql | 68 ++++++++++++++++--------
src/lib/dhcp/lease_mgr.h | 9 ++--
src/lib/dhcp/lease_mgr_factory.cc | 29 ++++++++--
src/lib/dhcp/lease_mgr_factory.h | 53 ++++++++++++++++--
src/lib/dhcp/mysql_lease_mgr.cc | 26 +++++++--
src/lib/dhcp/mysql_lease_mgr.h | 1 -
src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc | 31 ++++++-----
9 files changed, 171 insertions(+), 63 deletions(-)
-----------------------------------------------------------------------
diff --git a/configure.ac b/configure.ac
index ec41ca1..0c76536 100644
--- a/configure.ac
+++ b/configure.ac
@@ -735,9 +735,9 @@ LIBS=$LIBS_SAVED
# the --with-mysql-config (default to /usr/bin/mysql-config). By default,
# the software is not built with MySQL support enabled.
mysql_config="no"
-AC_ARG_WITH([mysql-config],
- AC_HELP_STRING([--with-mysql-config=PATH],
- [specify the path to the mysql_config script]),
+AC_ARG_WITH([dhcp-mysql],
+ AC_HELP_STRING([--with-dhcp-mysql=PATH],
+ [path to the MySQL 'mysql_config' script (MySQL is used for the DHCP database)]),
[mysql_config="$withval"])
if test "${mysql_config}" = "yes" ; then
@@ -747,11 +747,8 @@ elif test "${mysql_config}" != "no" ; then
fi
if test "$MYSQL_CONFIG" != "" ; then
- if test -d "$MYSQL_CONFIG" ; then
- AC_MSG_ERROR([Specified mysql_config program ${MYSQL_CONFIG} is a directory])
- fi
- if ! test -x "$MYSQL_CONFIG" ; then
- AC_MSG_ERROR([--with-mysql-config should point to a mysql_config program])
+ if test -d "$MYSQL_CONFIG" -o ! -x "$MYSQL_CONFIG" ; then
+ AC_MSG_ERROR([--with-dhcp-mysql should point to a mysql_config program])
fi
MYSQL_CPPFLAGS=`$MYSQL_CONFIG --cflags`
@@ -1466,8 +1463,6 @@ dnl includes too
${LOG4CPLUS_LIBS}
SQLite: $SQLITE_CFLAGS
$SQLITE_LIBS
- MySQL: $MYSQL_CPPFLAGS
- $MYSQL_LIBS
Features:
$enable_features
diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am
index 74c4491..945eaee 100644
--- a/src/lib/dhcp/Makefile.am
+++ b/src/lib/dhcp/Makefile.am
@@ -20,8 +20,8 @@ lib_LTLIBRARIES = libb10-dhcp++.la libb10-dhcpsrv.la
libb10_dhcp___la_SOURCES =
libb10_dhcp___la_SOURCES += dhcp6.h dhcp4.h
libb10_dhcp___la_SOURCES += duid.cc duid.h
-libb10_dhcp___la_SOURCES += iface_mgr_bsd.cc
libb10_dhcp___la_SOURCES += iface_mgr.cc iface_mgr.h
+libb10_dhcp___la_SOURCES += iface_mgr_bsd.cc
libb10_dhcp___la_SOURCES += iface_mgr_linux.cc
libb10_dhcp___la_SOURCES += iface_mgr_sun.cc
libb10_dhcp___la_SOURCES += lease_mgr.cc lease_mgr.h
diff --git a/src/lib/dhcp/dhcpdb_create.mysql b/src/lib/dhcp/dhcpdb_create.mysql
index e4b7771..41bea24 100644
--- a/src/lib/dhcp/dhcpdb_create.mysql
+++ b/src/lib/dhcp/dhcpdb_create.mysql
@@ -28,53 +28,63 @@
#
# source dhcpdb_create.mysql
+
# Holds the IPv4 leases.
CREATE TABLE lease4 (
- address INT UNSIGNED UNIQUE NOT NULL, # IPv4 address
- hwaddr VARBINARY(20), # Hardware address
- client_id VARBINARY(128), # Client ID
- lease_time INT UNSIGNED, # Length of the lease (seconds)
- expire TIMESTAMP, # Expiration time of the lease
- subnet_id INT UNSIGNED # Subnet identification
+ address INT UNSIGNED PRIMARY KEY NOT NULL, # IPv4 address
+ hwaddr VARBINARY(20), # Hardware address
+ client_id VARBINARY(128), # Client ID
+ lease_time INT UNSIGNED, # Length of the lease (seconds)
+ expire TIMESTAMP, # Expiration time of the lease
+ subnet_id INT UNSIGNED # Subnet identification
) ENGINE = INNODB;
-# Holds the IPv6 leases
+# Holds the IPv6 leases.
+# N.B. The use of a VARCHAR for the address is temporary for development:
+# it will eventually be replaced by BINARY(16).
CREATE TABLE lease6 (
- address VARCHAR(40) UNIQUE NOT NULL, # IPv6 address (actually 39 is max)
- hwaddr VARBINARY(20), # Hardware address
- client_id VARBINARY(128), # Client ID
- lease_time INT UNSIGNED, # Length of the lease (seconds)
- expire TIMESTAMP, # Expiration time of the lease
- subnet_id INT UNSIGNED, # Subnet identification
- pref_lifetime INT UNSIGNED, # Preferred lifetime
- lease_type TINYINT, # Lease type
- iaid INT UNSIGNED, # IA ID
- prefix_len TINYINT UNSIGNED # For IA_PD only
+ address VARCHAR(40) PRIMARY KEY NOT NULL, # IPv6 address
+ hwaddr VARBINARY(20), # Hardware address
+ client_id VARBINARY(128), # Client ID
+ lease_time INT UNSIGNED, # Length of the lease (seconds)
+ expire TIMESTAMP, # Expiration time of the lease
+ subnet_id INT UNSIGNED, # Subnet identification
+ pref_lifetime INT UNSIGNED, # Preferred lifetime
+ lease_type TINYINT, # Lease type (see lease6_types
+ # table for possible values)
+ iaid INT UNSIGNED, # See Section 10 of RFC 3315
+ prefix_len TINYINT UNSIGNED # For IA_PD only
) ENGINE = INNODB;
# ... and a definition of lease6 types. This table is a convenience for
# users of the database - if they want to view the lease table and use the
# type names, they can join this table with the lease6 table
CREATE TABLE lease6_types (
- lease_type TINYINT UNIQUE NOT NULL, # Lease type code
- name VARCHAR(5) # Name of the lease type
+ lease_type TINYINT PRIMARY KEY NOT NULL, # Lease type code.
+ name VARCHAR(5) # Name of the lease type
);
+START TRANSACTION;
INSERT INTO lease6_types VALUES (0, "IA_NA"); # Non-temporary v6 addresses
INSERT INTO lease6_types VALUES (1, "IA_TA"); # Temporary v6 addresses
INSERT INTO lease6_types VALUES (2, "IA_PD"); # Prefix delegations
+COMMIT;
# Finally, the version of the schema. We start at 0.1 during development.
# This table is only modified during schema upgrades. For historical reasons
# (related to the names of the columns in the BIND 10 DNS database file), the
# first column is called "version" and not "major".
CREATE TABLE schema_version (
- version INT NOT NULL, # Major version number
- minor INT NOT NULL # Minor version number
+ version INT PRIMARY KEY NOT NULL, # Major version number
+ minor INT # Minor version number
);
+START TRANSACTION;
INSERT INTO schema_version VALUES (0, 1);
-
COMMIT;
+# Notes:
+#
+# Indexes
+# =======
# It is likely that additional indexes will be needed. However, the
# increase in lookup performance from these will come at the expense
# of a decrease in performance during insert operations due to the need
@@ -90,3 +100,17 @@ COMMIT;
# For lease stability: if a client requests a new lease, try to find an
# existing or recently expired lease for it so that it can keep using the
# same IP address.
+#
+# Field Sizes
+# ===========
+# If any of the VARxxx field sizes are altered, the lengths in the MySQL
+# backend source file (mysql_lease_mgr.cc) must be correspondingly changed.
+#
+# Portability
+# ===========
+# The "ENGINE = INNODB" on some tables is not portablea to another database
+# and will need to be removed.
+#
+# Some columns contain binary data so are stored as VARBINARY instead of
+# VARCHAR. This may be non-portable between databases: in this case, the
+# definition should be changed to VARCHAR.
diff --git a/src/lib/dhcp/lease_mgr.h b/src/lib/dhcp/lease_mgr.h
index 7315551..6d74e5e 100644
--- a/src/lib/dhcp/lease_mgr.h
+++ b/src/lib/dhcp/lease_mgr.h
@@ -164,6 +164,7 @@ struct Lease4 {
/// @brief Constructor
///
/// Initialize fields that don't have a default constructor.
+ /// @TODO Remove this
Lease4() : addr_(0) {}
};
@@ -280,7 +281,7 @@ struct Lease6 {
/// @brief Constructor
///
/// Initialize fields that don't have a default constructor.
- Lease6() : addr_(0) {}
+ Lease6() : addr_("::") {}
};
/// @brief Pointer to a Lease6 structure.
@@ -336,7 +337,7 @@ public:
/// @exception DbOperationError Database function failed
virtual bool addLease(const Lease6Ptr& lease) = 0;
- /// @brief Returns existing IPv4 lease for specified IPv4 address and subnet_id
+ /// @brief Returns IPv4 lease for specified IPv4 address and subnet_id
///
/// This method is used to get a lease for specific subnet_id. There can be
/// at most one lease for any given subnet, so this method returns a single
@@ -522,7 +523,6 @@ public:
/// As host reservation is outside of scope for 2012, support for hosts
/// is currently postponed.
-
protected:
/// @brief returns value of the parameter
std::string getParameter(const std::string& name) const;
@@ -535,9 +535,6 @@ protected:
ParameterMap parameters_;
};
-/// @brief Pointer to a Lease Manager
-typedef boost::shared_ptr<LeaseMgr> LeaseMgrPtr;
-
}; // end of isc::dhcp namespace
}; // end of isc namespace
diff --git a/src/lib/dhcp/lease_mgr_factory.cc b/src/lib/dhcp/lease_mgr_factory.cc
index d69f8af..adc8ce0 100644
--- a/src/lib/dhcp/lease_mgr_factory.cc
+++ b/src/lib/dhcp/lease_mgr_factory.cc
@@ -23,6 +23,7 @@
#include <utility>
#include <boost/foreach.hpp>
+#include <boost/scoped_ptr.hpp>
#include <boost/algorithm/string.hpp>
#include <exceptions/exceptions.h>
#include <dhcp/lease_mgr_factory.h>
@@ -36,6 +37,12 @@ using namespace std;
namespace isc {
namespace dhcp {
+boost::scoped_ptr<LeaseMgr>&
+LeaseMgrFactory::getLeaseMgrPtr() {
+ static boost::scoped_ptr<LeaseMgr> leaseMgrPtr;
+ return (leaseMgrPtr);
+}
+
LeaseMgr::ParameterMap
LeaseMgrFactory::parse(const std::string& dbconfig) {
LeaseMgr::ParameterMap mapped_tokens;
@@ -64,7 +71,7 @@ LeaseMgrFactory::parse(const std::string& dbconfig) {
return (mapped_tokens);
}
-LeaseMgrPtr
+void
LeaseMgrFactory::create(const std::string& dbconfig) {
const std::string type = "type";
@@ -78,14 +85,30 @@ LeaseMgrFactory::create(const std::string& dbconfig) {
// Yes, check what it is.
#ifdef HAVE_MYSQL
if (parameters[type] == string("mysql")) {
- return LeaseMgrPtr(new MySqlLeaseMgr(parameters));
+ getLeaseMgrPtr().reset(new MySqlLeaseMgr(parameters));
+ return;
}
#endif
// Get here on no match
- isc_throw(InvalidParameter, "Database configuration parameter 'type' does "
+ isc_throw(InvalidType, "Database configuration parameter 'type' does "
"not specify a supported database backend");
}
+void
+LeaseMgrFactory::destroy() {
+ getLeaseMgrPtr().reset();
+}
+
+LeaseMgr&
+LeaseMgrFactory::instance() {
+ LeaseMgr* lmptr = getLeaseMgrPtr().get();
+ if (lmptr == NULL) {
+ isc_throw(NoLeaseManager, "no current lease manager is available");
+ }
+ return (*lmptr);
+}
+
+
}; // namespace dhcp
}; // namespace isc
diff --git a/src/lib/dhcp/lease_mgr_factory.h b/src/lib/dhcp/lease_mgr_factory.h
index 11057d0..164d2f9 100644
--- a/src/lib/dhcp/lease_mgr_factory.h
+++ b/src/lib/dhcp/lease_mgr_factory.h
@@ -22,14 +22,25 @@
namespace isc {
namespace dhcp {
-/// @brief Invalid Type Exception
+/// @brief Invalid type exception
///
/// Thrown when the factory doesn't recognise the type of the backend.
class InvalidType : public Exception {
+public:
InvalidType(const char* file, size_t line, const char* what) :
isc::Exception(file, line, what) {}
};
+/// @brief No lease manager exception
+///
+/// Thrown if an attempt is made to get a reference to the current lease
+/// manager and none is currently available.
+class NoLeaseManager : public Exception {
+public:
+ NoLeaseManager(const char* file, size_t line, const char* what) :
+ isc::Exception(file, line, what) {}
+};
+
/// @brief Lease Manager Factory
///
/// This class comprises nothing but static methods used to create a lease
@@ -44,8 +55,12 @@ public:
/// @brief Create an instance of a lease manager.
///
/// Each database backend has its own lease manager type. This static
- /// method returns a lease manager of the appropriate type, based on the
- /// the data in the input argument.
+ /// method sets the "current" lease manager to be a manager of the
+ /// appropriate type. The actual lease manager is returned by the
+ /// "instance" method.
+ ///
+ /// Note: when called, the current lease manager is *always* destroyed
+ /// and a new one created - even if the parameters are the same.
///
/// dbconfig is a generic way of passing parameters. Parameters are passed
/// in the "name=value" format, separated by spaces. The data MUST include
@@ -57,8 +72,27 @@ public:
/// are back-end specific, although must include the "type" keyword
/// which gives the backend in use.
///
- /// @return Implementation of lease manager for the specified database.
- static LeaseMgrPtr create(const std::string& dbconfig);
+ /// @exception InvalidParameter dbconfig string does not contain the
+ /// "type" keyword.
+ /// @exception InvalidType The "type" keyword in dbconfig does not identify
+ /// a supported backend.
+ static void create(const std::string& dbconfig);
+
+ /// @brief Destroy lease manager
+ ///
+ /// Destroys the current lease manager object. This should have the effect
+ /// of closing the database connection. The method is a no-op if no
+ /// lease manager is available.
+ static void destroy();
+
+ /// @brief Return Current Lease Manager
+ ///
+ /// Returns an instance of the "current" lease manager. An exception
+ /// will be thrown if none is available.
+ ///
+ /// @exception NoLeaseManager No lease manager is available: use create()
+ /// to create one before calling this method.
+ static LeaseMgr& instance();
/// @brief Parse Database Parameters
///
@@ -69,6 +103,15 @@ public:
///
/// @return std::map<>std::string, std::string> Map of keyword/value pairs.
static LeaseMgr::ParameterMap parse(const std::string& dbconfig);
+
+private:
+ /// @brief Hold pointer to lease manager
+ ///
+ /// Holds a pointer to the singleton lease manager. The singleton
+ /// is encapsulated in this method to avoid a "static initialization
+ /// fiasco" if defined in an external static variable.
+ static boost::scoped_ptr<LeaseMgr>& getLeaseMgrPtr();
+
};
}; // end of isc::dhcp namespace
diff --git a/src/lib/dhcp/mysql_lease_mgr.cc b/src/lib/dhcp/mysql_lease_mgr.cc
index aac0682..1f56ec9 100644
--- a/src/lib/dhcp/mysql_lease_mgr.cc
+++ b/src/lib/dhcp/mysql_lease_mgr.cc
@@ -24,6 +24,25 @@
using namespace std;
+namespace {
+///@{
+/// @brief Maximum Size of Database Fields
+///
+/// The following constants define buffer sizes for variable length database
+/// fields. The values should be greater than or equal to the length set in
+/// the schema definition.
+///
+/// The exception is the length of any VARCHAR fields: these should be set
+/// greater than or equal to the length of the field plus 2: this allows for
+/// the insertion of a trailing null regardless of whether the data returned
+/// contains a trailing null (the documentation is not clear on this point).
+
+const size_t ADDRESS6_TEXT_MAX_LEN = 42; // Max size of a IPv6 text buffer
+const size_t DUID_MAX_LEN = 128; // Max size of a DUID
+const size_t HWADDR_MAX_LEN = 20; // Max size of a hardware address
+///@}
+};
+
namespace isc {
namespace dhcp {
@@ -290,16 +309,17 @@ private:
// Note: All array legths are equal to the corresponding variable in the
// schema.
std::string addr6_; ///< String form of address
- char addr6_buffer_[42]; ///< Character array form of V6 address
+ char addr6_buffer_[ADDRESS6_TEXT_MAX_LEN]; ///< Character
+ ///< array form of V6 address
unsigned long addr6_length_; ///< Length of the address
MYSQL_BIND bind_[10]; ///< Static array for speed of access
std::vector<uint8_t> clientid_; ///< Client identification
- uint8_t clientid_buffer_[128]; ///< Buffer form of client ID
+ uint8_t clientid_buffer_[DUID_MAX_LEN]; ///< Buffer form of DUID
unsigned long clientid_length_; ///< Length of client ID
my_bool error_[10]; ///< For error reporting
MYSQL_TIME expire_; ///< Lease expiry time
const my_bool false_; ///< "false" for MySql
- uint8_t hwaddr_buffer_[20]; ///< Hardware address buffer
+ uint8_t hwaddr_buffer_[HWADDR_MAX_LEN]; ///< Hardware address buffer
unsigned long hwaddr_length_; ///< Length of hardware address
uint32_t iaid_; ///< Identity association ID
Lease6Ptr lease_; ///< Pointer to lease object
diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h
index 50ccb72..4a7ba74 100644
--- a/src/lib/dhcp/mysql_lease_mgr.h
+++ b/src/lib/dhcp/mysql_lease_mgr.h
@@ -244,7 +244,6 @@ public:
/// @exception DbOperationError if the commit failed.
virtual void commit();
-
/// @brief Rollback Transactions
///
/// Rolls back all pending database operations. On databases that don't
diff --git a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
index 03e6a29..2739a90 100644
--- a/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/mysql_lease_mgr_unittest.cc
@@ -68,7 +68,8 @@ public:
/// Open the database.
MySqlLeaseMgrTest() {
- lmptr_ = LeaseMgrFactory::create(validConnectionString());
+ LeaseMgrFactory::create(validConnectionString());
+ lmptr_ = &(LeaseMgrFactory::instance());
}
/// @brief Destructor
@@ -78,9 +79,10 @@ public:
virtual ~MySqlLeaseMgrTest() {
lmptr_->rollback();
+ LeaseMgrFactory::destroy();
}
- LeaseMgrPtr lmptr_; // Pointer to the lease manager
+ LeaseMgr* lmptr_; // Pointer to the lease manager
};
@@ -92,32 +94,37 @@ public:
/// opened: the fixtures assume that and check basic operations.
TEST(MySqlOpenTest, OpenDatabase) {
- LeaseMgrPtr lmptr;
+ // Check that attempting to get an instance of the lease manager when
+ // none is set throws an exception.
+ EXPECT_THROW(LeaseMgrFactory::instance(), NoLeaseManager);
// Check that wrong specification of backend throws an exception.
// (This is really a check on LeaseMgrFactory, but is convenient to
// perform here.)
- EXPECT_THROW(lmptr = LeaseMgrFactory::create(connectionString(
+ EXPECT_THROW(LeaseMgrFactory::create(connectionString(
INVALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)),
- InvalidParameter);
+ InvalidType);
// Check that invalid login data causes an exception.
- EXPECT_THROW(lmptr = LeaseMgrFactory::create(connectionString(
+ EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, INVALID_NAME, VALID_HOST, VALID_USER, VALID_PASSWORD)),
DbOpenError);
- EXPECT_THROW(lmptr = LeaseMgrFactory::create(connectionString(
+ EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD)),
DbOpenError);
- EXPECT_THROW(lmptr = LeaseMgrFactory::create(connectionString(
+ EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, VALID_NAME, VALID_HOST, INVALID_USER, VALID_PASSWORD)),
DbOpenError);
- EXPECT_THROW(lmptr = LeaseMgrFactory::create(connectionString(
+ EXPECT_THROW(LeaseMgrFactory::create(connectionString(
VALID_TYPE, VALID_NAME, VALID_HOST, VALID_USER, INVALID_PASSWORD)),
DbOpenError);
- // Check that database opens correctly and that version is as expected
- ASSERT_NO_THROW(lmptr = LeaseMgrFactory::create(validConnectionString()));
- ASSERT_TRUE(lmptr);
+ // Check that database opens correctly.
+ ASSERT_NO_THROW(LeaseMgrFactory::create(validConnectionString()));
+ EXPECT_NO_THROW((void) LeaseMgrFactory::instance());
+
+ // ... and tidy up.
+ LeaseMgrFactory::destroy();
}
/// @brief Check conversion functions
More information about the bind10-changes
mailing list