diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc index 30de103d53..b12bdd5e19 100644 --- a/src/lib/dhcpsrv/mysql_connection.cc +++ b/src/lib/dhcpsrv/mysql_connection.cc @@ -213,22 +213,27 @@ MySqlConnection::prepareStatement(uint32_t index, const char* text) { } void -MySqlConnection::prepareStatements(const TaggedStatement tagged_statements[], +MySqlConnection::prepareStatements(const TaggedStatement* start_statement, + const TaggedStatement* end_statement, size_t num_statements) { // Allocate space for all statements - statements_.clear(); statements_.resize(num_statements, NULL); - text_statements_.clear(); text_statements_.resize(num_statements, std::string("")); // Created the MySQL prepared statements for each DML statement. - for (int i = 0; tagged_statements[i].text != NULL; ++i) { - prepareStatement(tagged_statements[i].index, - tagged_statements[i].text); + for (const TaggedStatement* tagged_statement = start_statement; + tagged_statement != end_statement; ++tagged_statement) { + prepareStatement(tagged_statement->index, + tagged_statement->text); } } +void MySqlConnection::clearStatements() { + statements_.clear(); + text_statements_.clear(); +} + /// @brief Destructor MySqlConnection::~MySqlConnection() { // Free up the prepared statements, ignoring errors. (What would we do diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h index 8acb894577..06fb34d466 100644 --- a/src/lib/dhcpsrv/mysql_connection.h +++ b/src/lib/dhcpsrv/mysql_connection.h @@ -247,9 +247,13 @@ public: /// failed. /// @throw isc::InvalidParameter 'index' is not valid for the vector. This /// represents an internal error within the code. - void prepareStatements(const TaggedStatement tagged_statements[], + void prepareStatements(const TaggedStatement* start_statement, + const TaggedStatement* end_statement, size_t num_statements); + /// @brief Clears prepared statements and text statements. + void clearStatements(); + /// @brief Open Database /// /// Opens the database using the information supplied in the parameters diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 150a10391e..70a9aa7a43 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -1633,10 +1634,6 @@ public: /// /// The contents of the enum are indexes into the list of SQL statements enum StatementIndex { - INSERT_HOST, // Insert new host to collection - INSERT_V6_RESRV, // Insert v6 reservation - INSERT_V4_OPTION, // Insert DHCPv4 option - INSERT_V6_OPTION, // Insert DHCPv6 option GET_HOST_DHCPID, // Gets hosts by host identifier GET_HOST_ADDR, // Gets hosts by IPv4 address GET_HOST_SUBID4_DHCPID, // Gets host by IPv4 SubnetID, HW address/DUID @@ -1644,9 +1641,20 @@ public: GET_HOST_SUBID_ADDR, // Gets host by IPv4 SubnetID and IPv4 address GET_HOST_PREFIX, // Gets host by IPv6 prefix GET_VERSION, // Obtain version number + INSERT_HOST, // Insert new host to collection + INSERT_V6_RESRV, // Insert v6 reservation + INSERT_V4_OPTION, // Insert DHCPv4 option + INSERT_V6_OPTION, // Insert DHCPv6 option NUM_STATEMENTS // Number of statements }; + /// @brief Index of first statement performing write to the database. + /// + /// This value is used to mark border line between queries and other + /// statements and statements performing write operation on the database, + /// such as INSERT, DELETE, UPDATE. + static const StatementIndex WRITE_STMTS_BEGIN = INSERT_HOST; + /// @brief Constructor. /// /// This constructor opens database connection and initializes prepared @@ -1777,39 +1785,20 @@ public: /// @brief MySQL connection MySqlConnection conn_; + /// @brief Indicates if the database is opened in read only mode. + bool is_readonly_; +namespace { + }; -namespace { + +/// @brief Array of tagged statements. +typedef boost::array +TaggedStatementArray; + /// @brief Prepared MySQL statements used by the backend to insert and /// retrieve hosts from the database. -TaggedStatement tagged_statements[] = { - // Inserts a host into the 'hosts' table. - {MySqlHostDataSourceImpl::INSERT_HOST, - "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, " - "dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, " - "dhcp4_client_classes, dhcp6_client_classes) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"}, - - // Inserts a single IPv6 reservation into 'reservations' table. - {MySqlHostDataSourceImpl::INSERT_V6_RESRV, - "INSERT INTO ipv6_reservations(address, prefix_len, type, " - "dhcp6_iaid, host_id) " - "VALUES (?,?,?,?,?)"}, - - // Inserts a single DHCPv4 option into 'dhcp4_options' table. - // Using fixed scope_id = 3, which associates an option with host. - {MySqlHostDataSourceImpl::INSERT_V4_OPTION, - "INSERT INTO dhcp4_options(option_id, code, value, formatted_value, space, " - "persistent, dhcp_client_class, dhcp4_subnet_id, host_id, scope_id) " - " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}, - - // Inserts a single DHCPv6 option into 'dhcp6_options' table. - // Using fixed scope_id = 3, which associates an option with host. - {MySqlHostDataSourceImpl::INSERT_V6_OPTION, - "INSERT INTO dhcp6_options(option_id, code, value, formatted_value, space, " - "persistent, dhcp_client_class, dhcp6_subnet_id, host_id, scope_id) " - " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}, - +TaggedStatementArray tagged_statements = { { // Retrieves host information, IPv6 reservations and both DHCPv4 and // DHCPv6 options associated with the host. The LEFT JOIN clause is used // to retrieve information from 4 different tables using a single query. @@ -1931,8 +1920,32 @@ TaggedStatement tagged_statements[] = { {MySqlHostDataSourceImpl::GET_VERSION, "SELECT version, minor FROM schema_version"}, - // Marks the end of the statements table. - {MySqlHostDataSourceImpl::NUM_STATEMENTS, NULL} + // Inserts a host into the 'hosts' table. + {MySqlHostDataSourceImpl::INSERT_HOST, + "INSERT INTO hosts(host_id, dhcp_identifier, dhcp_identifier_type, " + "dhcp4_subnet_id, dhcp6_subnet_id, ipv4_address, hostname, " + "dhcp4_client_classes, dhcp6_client_classes) " + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"}, + + // Inserts a single IPv6 reservation into 'reservations' table. + {MySqlHostDataSourceImpl::INSERT_V6_RESRV, + "INSERT INTO ipv6_reservations(address, prefix_len, type, " + "dhcp6_iaid, host_id) " + "VALUES (?,?,?,?,?)"}, + + // Inserts a single DHCPv4 option into 'dhcp4_options' table. + // Using fixed scope_id = 3, which associates an option with host. + {MySqlHostDataSourceImpl::INSERT_V4_OPTION, + "INSERT INTO dhcp4_options(option_id, code, value, formatted_value, space, " + "persistent, dhcp_client_class, dhcp4_subnet_id, host_id, scope_id) " + " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}, + + // Inserts a single DHCPv6 option into 'dhcp6_options' table. + // Using fixed scope_id = 3, which associates an option with host. + {MySqlHostDataSourceImpl::INSERT_V6_OPTION, + "INSERT INTO dhcp6_options(option_id, code, value, formatted_value, space, " + "persistent, dhcp_client_class, dhcp6_subnet_id, host_id, scope_id) " + " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, 3)"}} }; }; // end anonymouse namespace @@ -1945,23 +1958,56 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) DHCP4_AND_DHCP6)), host_ipv6_reservation_exchange_(new MySqlIPv6ReservationExchange()), host_option_exchange_(new MySqlOptionExchange()), - conn_(parameters) { + conn_(parameters), + is_readonly_(false) { // Open the database. conn_.openDatabase(); - // Disable autocommit. To avoid a flush to disk on every commit, the global - // parameter innodb_flush_log_at_trx_commit should be set to 2. This will - // cause the changes to be written to the log, but flushed to disk in the - // background every second. Setting the parameter to that value will speed - // up the system, but at the risk of losing data if the system crashes. - my_bool result = mysql_autocommit(conn_.mysql_, 0); + // Enable autocommit. In case transaction is explicitly used, this + // setting will be overwritten for the transaction. However, there are + // cases when lack of autocommit could cause transactions to hang + // until commit or rollback is explicitly called. This already + // caused issues for some unit tests which were unable to cleanup + // the database after the test because of pending transactions. + // Use of autocommit will eliminate this problem. + my_bool result = mysql_autocommit(conn_.mysql_, 1); if (result != 0) { isc_throw(DbOperationError, mysql_error(conn_.mysql_)); } - // Prepare all statements likely to be used. - conn_.prepareStatements(tagged_statements, NUM_STATEMENTS); + // Prepare query statements. Those are will be only used to retrieve + // information from the database, so they can be used even if the + // database is read only for the current user. + conn_.prepareStatements(tagged_statements.begin(), + tagged_statements.begin() + WRITE_STMTS_BEGIN, + WRITE_STMTS_BEGIN); + + std::string readonly_value = "false"; + try { + readonly_value = conn_.getParameter("readonly"); + boost::algorithm::to_lower(readonly_value); + } catch (...) { + // Parameter "readonly" hasn't been specified so we simply use + // the default value of "false". + } + + if (readonly_value == "true" || readonly_value == "1") { + is_readonly_ = true; + + } else if ((readonly_value != "false") && (readonly_value != "0")) { + isc_throw(BadValue, "invalid value '" << readonly_value + << "' specified for boolean parameter 'readonly'"); + } + + // If we are using read-write mode for the database we also prepare + // statements for INSERTS etc. + if (!is_readonly_) { + // Prepare statements for writing to the database, e.g. INSERT. + conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN, + tagged_statements.end(), + tagged_statements.size()); + } } MySqlHostDataSourceImpl::~MySqlHostDataSourceImpl() { @@ -2163,11 +2209,14 @@ getHost(const SubnetID& subnet_id, MySqlHostDataSource:: MySqlHostDataSource(const MySqlConnection::ParameterMap& parameters) - : impl_(new MySqlHostDataSourceImpl(parameters)) { + : impl_(new MySqlHostDataSourceImpl(parameters), + "MySQL host database backend is configured to" + " operate in read only mode") { + impl_.allowConstOnly(impl_->is_readonly_); } MySqlHostDataSource::~MySqlHostDataSource() { - delete impl_; + delete impl_.getPtr(); } void diff --git a/src/lib/dhcpsrv/mysql_host_data_source.h b/src/lib/dhcpsrv/mysql_host_data_source.h index 9ce64f2256..6ee563d83b 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.h +++ b/src/lib/dhcpsrv/mysql_host_data_source.h @@ -8,7 +8,9 @@ #define MYSQL_HOST_DATA_SOURCE_H #include +#include #include +#include namespace isc { namespace dhcp { @@ -255,7 +257,7 @@ public: private: /// @brief Pointer to the implementation of the @ref MySqlHostDataSource. - MySqlHostDataSourceImpl* impl_; + util::RestrictedConstPtr impl_; }; } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 410880af36..7e448428f2 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -82,7 +83,8 @@ const size_t HOSTNAME_MAX_LEN = 255; /// colon separators. const size_t ADDRESS6_TEXT_MAX_LEN = 39; -TaggedStatement tagged_statements[] = { +boost::array +tagged_statements = { { {MySqlLeaseMgr::DELETE_LEASE4, "DELETE FROM lease4 WHERE address = ?"}, {MySqlLeaseMgr::DELETE_LEASE4_STATE_EXPIRED, @@ -204,9 +206,8 @@ TaggedStatement tagged_statements[] = { "prefix_len = ?, fqdn_fwd = ?, fqdn_rev = ?, " "hostname = ?, hwaddr = ?, hwtype = ?, hwaddr_source = ?, " "state = ? " - "WHERE address = ?"}, - // End of list sentinel - {MySqlLeaseMgr::NUM_STATEMENTS, NULL} + "WHERE address = ?"} + } }; }; @@ -1236,7 +1237,8 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters) } // Prepare all statements likely to be used. - conn_.prepareStatements(tagged_statements, MySqlLeaseMgr::NUM_STATEMENTS); + conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end(), + tagged_statements.size()); // Create the exchange objects for use in exchanging data between the // program and the database. diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index c12ea64b08..fe4560a40b 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -514,6 +514,7 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { // The database is initially opened in "read-write" mode. We can // insert some data to the databse. HostPtr host = initializeHost6("2001:db8::1", Host::IDENT_DUID, false); + ASSERT_TRUE(host); ASSERT_NO_THROW(hdsptr_->add(host)); // Subnet id will be used in queries to the database. @@ -524,6 +525,7 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { ConstHostPtr host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(), &host->getIdentifier()[0], host->getIdentifier().size()); + ASSERT_TRUE(host_by_id); ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id)); // Close the database connection and reopen in "read-only" mode as @@ -536,15 +538,21 @@ GenericHostDataSourceTest::testReadOnlyDatabase(const char* valid_db_type) { VALID_PASSWORD, VALID_READONLY_DB)); + hdsptr_ = HostDataSourceFactory::getHostDataSourcePtr(); + // Check that an attempt to insert new host would result in // exception. - host = initializeHost6("2001:db8::2", Host::IDENT_DUID, false); - ASSERT_THROW(hdsptr_->add(host), ReadOnlyDb); + HostPtr host2 = initializeHost6("2001:db8::2", Host::IDENT_DUID, false); + ASSERT_TRUE(host2); + ASSERT_THROW(hdsptr_->add(host2), ReadOnlyDb); + ASSERT_THROW(hdsptr_->commit(), ReadOnlyDb); + ASSERT_THROW(hdsptr_->rollback(), ReadOnlyDb); // Reading from the database should still be possible, though. host_by_id = hdsptr_->get6(subnet_id, host->getIdentifierType(), &host->getIdentifier()[0], host->getIdentifier().size()); + ASSERT_TRUE(host_by_id); ASSERT_NO_FATAL_FAILURE(compareHosts(host, host_by_id)); } diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index 849e31b40b..058b2e358f 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -63,8 +63,13 @@ public: /// Rolls back all pending transactions. The deletion of myhdsptr_ will close /// the database. Then reopen it and delete everything created by the test. virtual ~MySqlHostDataSourceTest() { - hdsptr_->rollback(); + try { + hdsptr_->rollback(); + } catch (...) { + // Rollback may fail if backend is in read only mode. That's ok. + } HostDataSourceFactory::destroy(); + hdsptr_.reset(); destroyMySQLSchema(); } diff --git a/src/lib/util/pointer_util.h b/src/lib/util/pointer_util.h index a775584573..ed8374722f 100644 --- a/src/lib/util/pointer_util.h +++ b/src/lib/util/pointer_util.h @@ -1,4 +1,4 @@ -// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -7,9 +7,49 @@ #ifndef POINTER_UTIL_H #define POINTER_UTIL_H +#include +#include + namespace isc { namespace util { +template +class RestrictedConstPtr { +public: + + RestrictedConstPtr(T* ptr, const std::string& error_text) + : ptr_(ptr), const_only_(false), error_text_(error_text) { + } + + void allowConstOnly(const bool const_only) { + const_only_ = const_only; + } + + T* operator->() const { + return (ptr_); + } + + T* operator->() { + if (const_only_) { + isc_throw(E, error_text_); + } + return (ptr_); + } + + T* getPtr() const { + return (ptr_); + } + +private: + + T* ptr_; + + bool const_only_; + + std::string error_text_; +}; + + /// @brief This function checks if two pointers are non-null and values /// are equal. ///