[#2909] Addressed review comments

src/bin/admin/tests/mysql_tests.sh.in
    Added test to verify lease6 address and ipv6 reservation address
    column data conversion

src/lib/dhcpsrv/mysql_host_data_source.cc
    Use isc::asiolink::V6ADDRESS_LEN;

src/share/database/scripts/mysql/dhcpdb_create.mysql
src/share/database/scripts/mysql/upgrade_018_to_019.sh.in
    Add data conversion logic for lease6 and ipv6_reservation columns

src/bin/admin/tests/mysql_tests.sh.in

@@ -2812,6 +2812,59 @@ mysql_update_empty_duid_test() {
     test_finish 0
 }
 
+# Verifies that converting from lease6.address to binary column works
+# while preserving data.
+mysql_update_v6_addresses_to_binary() {
+    test_start "mysql.update_lease6_address_to_binary"
+
+    # Let's wipe the whole database
+    mysql_wipe
+
+    # We need to create an older database with lease data so we can
+    # verify the upgrade mechanisms which convert empty duid values
+    #
+    # Initialize database to schema 1.0.
+    mysql -u"${db_user}" -p"${db_password}" "${db_name}" < "/home/tmark/labs/build/keadev/sandbox/git.mysql-2909/kea/src/bin/admin/tests/dhcpdb_create_1.0.mysql"
+
+    # Now upgrade to schema 18.0
+    mysql_upgrade_schema_to_version 18.0
+
+    sql=\
+"insert into lease6 (address, lease_type, subnet_id) values('2601:19e:8100:1e10:b1b:51a8:f616:cf14', 1, 1);
+insert into lease6 (address, lease_type, subnet_id) values('2601:19e:8100:1e10:b1b:51a8:f616:cf15', 1, 1);"
+
+    run_statement "insert v6 leases" "$sql"
+
+    # Insert ipv6_reservations address is binary.
+    sql=\
+"insert into hosts(host_id, dhcp_identifier, dhcp_identifier_type) values (18219, '18219', 1); \
+insert into ipv6_reservations (address, prefix_len, type, dhcp6_iaid, host_id) \
+        values ('2601:19e:8100:1e10:b1b:51a8:f616:cf16', 128, 1, 123, 18219);"
+
+    run_statement "insert an ipv6 reservation" "$sql"
+
+    # Let's upgrade it to the latest version.
+    run_command \
+        "${kea_admin}" db-upgrade mysql -u "${db_user}" -p "${db_password}" -n "${db_name}" -d "${db_scripts_dir}"
+
+    # leases count for declined state should be 1 with DUID updated (0x000000)
+    qry="select count(*) from lease6 where address = inet6_aton('2601:19e:8100:1e10:b1b:51a8:f616:cf14');"
+    run_statement "#2" "$qry" 1
+
+    # leases count for non declined state should be 1 with DUID unchanged (0x323033)
+    qry="select count(*) from lease6 where address = inet6_aton('2601:19e:8100:1e10:b1b:51a8:f616:cf15');"
+    run_statement "#3" "$qry" 1
+
+    # verify the reservation is intact
+    qry="select inet6_ntoa(address) from ipv6_reservations where host_id = 18219;"
+    run_statement "ipv6_reservations_insert" "$qry" "2601:19e:8100:1e10:b1b:51a8:f616:cf16"
+
+    # Let's wipe the whole database
+    mysql_wipe
+
+    test_finish 0
+}
+
 # Run tests.
 mysql_db_init_test
 mysql_host_reservation_init_test
@@ -2836,3 +2889,4 @@ mysql_client_class_test dhcp4
 mysql_client_class_test dhcp6
 mysql_shrink_server_tag_test
 mysql_update_empty_duid_test
+mysql_update_v6_addresses_to_binary

src/lib/dhcpsrv/mysql_host_data_source.cc

@@ -1535,7 +1535,7 @@ public:
         bind_[reservation_id_index_].is_unsigned = MLM_TRUE;
 
         // IPv6 address/prefix BINARY(16)
-        ipv6_address_buffer_len_ = 16;
+        ipv6_address_buffer_len_ = isc::asiolink::V6ADDRESS_LEN;
         bind_[address_index_].buffer_type = MYSQL_TYPE_BLOB;
         bind_[address_index_].buffer = reinterpret_cast<char*>(ipv6_address_buffer_);
         bind_[address_index_].buffer_length = ipv6_address_buffer_len_;
@@ -1579,7 +1579,7 @@ private:
     my_bool reserv_type_null_;
 
     /// @brief Buffer holding IPv6 address/prefix in textual format.
-    uint8_t ipv6_address_buffer_[16];
+    uint8_t ipv6_address_buffer_[isc::asiolink::V6ADDRESS_LEN];
 
     /// @brief Length of the textual address representation.
     unsigned long ipv6_address_buffer_len_;
@@ -1635,7 +1635,7 @@ public:
     ///
     /// Initialize class members representing a single IPv6 reservation.
     MySqlIPv6ReservationExchange()
-        : host_id_(0), address_("::"), address_len_(0), prefix_len_(0), type_(0),
+        : host_id_(0), prefix_len_(0), type_(0),
           iaid_(0), resv_(IPv6Resrv::TYPE_NA, asiolink::IOAddress("::"), 128) {
 
         // Reset error table.
@@ -1684,14 +1684,15 @@ public:
 
         try {
             addr6_ = resv.getPrefix().toBytes();
-            if (addr6_.size() != 16) {
+            if (addr6_.size() != isc::asiolink::V6ADDRESS_LEN) {
-                isc_throw(DbOperationError, "createBindForSend() - prefix is not 16 bytes long");
+                isc_throw(DbOperationError, "createBindForSend() - prefix is not "
+                          << isc::asiolink::V6ADDRESS_LEN << " bytes long");
             }
 
-            addr6_length_ = 16;
+            addr6_length_ = isc::asiolink::V6ADDRESS_LEN;
             bind_[0].buffer_type = MYSQL_TYPE_BLOB;
             bind_[0].buffer = reinterpret_cast<char*>(&addr6_[0]);
-            bind_[0].buffer_length = 16;
+            bind_[0].buffer_length = isc::asiolink::V6ADDRESS_LEN;
             bind_[0].length = &addr6_length_;
 
             // prefix_len tinyint
@@ -1746,12 +1747,6 @@ private:
     /// @brief Host unique identifier.
     uint64_t host_id_;
 
-    /// @brief Address (or prefix).
-    std::string address_;
-
-    /// @brief Length of the textual address representation.
-    unsigned long address_len_;
-
     /// @brief Length of the prefix (128 for addresses).
     uint8_t prefix_len_;
 
@@ -1778,7 +1773,7 @@ private:
     std::vector<uint8_t> addr6_;
 
     /// @brief Binary address buffer.
-    uint8_t addr6_buffer_[16];
+    uint8_t addr6_buffer_[isc::asiolink::V6ADDRESS_LEN];
 
     /// @brief Binary address length.
     unsigned long addr6_length_;
@@ -3323,14 +3318,15 @@ MySqlHostDataSource::del(const SubnetID& subnet_id,
 
     // v6
     std::vector<uint8_t>addr6 = addr.toBytes();
-    if (addr6.size() != 16) {
+    if (addr6.size() != isc::asiolink::V6ADDRESS_LEN) {
-        isc_throw(DbOperationError, "del() - address is not 16 bytes long");
+        isc_throw(DbOperationError, "del() - address is not "
+                  << isc::asiolink::V6ADDRESS_LEN << " bytes long");
     }
 
-    unsigned long addr6_length = 16;
+    unsigned long addr6_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[1].buffer_type = MYSQL_TYPE_BLOB;
     inbind[1].buffer = reinterpret_cast<char*>(&addr6[0]);
-    inbind[1].buffer_length = 16;
+    inbind[1].buffer_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[1].length = &addr6_length;
 
     return (impl_->delStatement(ctx, MySqlHostDataSourceImpl::DEL_HOST_ADDR6, inbind));
@@ -3860,14 +3856,15 @@ MySqlHostDataSource::get6(const asiolink::IOAddress& prefix,
     memset(inbind, 0, sizeof(inbind));
 
     std::vector<uint8_t>addr6 = prefix.toBytes();
-    if (addr6.size() != 16) {
+    if (addr6.size() != isc::asiolink::V6ADDRESS_LEN) {
-        isc_throw(DbOperationError, "get6() - prefix is not 16 bytes long");
+        isc_throw(DbOperationError, "get6() - prefix is not "
+                  << isc::asiolink::V6ADDRESS_LEN << " bytes long");
     }
 
-    unsigned long addr6_length = 16;
+    unsigned long addr6_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[0].buffer_type = MYSQL_TYPE_BLOB;
     inbind[0].buffer = reinterpret_cast<char*>(&addr6[0]);
-    inbind[0].buffer_length = 16;
+    inbind[0].buffer_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[0].length = &addr6_length;
 
     uint8_t tmp = prefix_len;
@@ -3910,14 +3907,15 @@ MySqlHostDataSource::get6(const SubnetID& subnet_id,
     inbind[0].is_unsigned = MLM_TRUE;
 
     std::vector<uint8_t>addr6 = address.toBytes();
-    if (addr6.size() != 16) {
+    if (addr6.size() != isc::asiolink::V6ADDRESS_LEN) {
-        isc_throw(DbOperationError, "get6() - address is not 16 bytes long");
+        isc_throw(DbOperationError, "get6() - address is not "
+                  << isc::asiolink::V6ADDRESS_LEN << " bytes long");
     }
 
-    unsigned long addr6_length = 16;
+    unsigned long addr6_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[1].buffer_type = MYSQL_TYPE_BLOB;
     inbind[1].buffer = reinterpret_cast<char*>(&addr6[0]);
-    inbind[1].buffer_length = 16;
+    inbind[1].buffer_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[1].length = &addr6_length;
 
     ConstHostCollection collection;
@@ -3955,14 +3953,15 @@ MySqlHostDataSource::getAll6(const SubnetID& subnet_id,
     inbind[0].is_unsigned = MLM_TRUE;
 
     std::vector<uint8_t>addr6 = address.toBytes();
-    if (addr6.size() != 16) {
+    if (addr6.size() != isc::asiolink::V6ADDRESS_LEN) {
-        isc_throw(DbOperationError, "getAll6() - address is not 16 bytes long");
+        isc_throw(DbOperationError, "getAll6() - address is not "
+                  << isc::asiolink::V6ADDRESS_LEN << " bytes long");
     }
 
-    unsigned long addr6_length = 16;
+    unsigned long addr6_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[1].buffer_type = MYSQL_TYPE_BLOB;
     inbind[1].buffer = reinterpret_cast<char*>(&addr6[0]);
-    inbind[1].buffer_length = 16;
+    inbind[1].buffer_length = isc::asiolink::V6ADDRESS_LEN;
     inbind[1].length = &addr6_length;
 
     ConstHostCollection collection;

src/lib/dhcpsrv/mysql_lease_mgr.cc

@@ -2260,7 +2260,6 @@ MySqlLeaseMgr::createContext() const {
         }
     }
 
-
     // Prepare all statements likely to be used.
     ctx->conn_.prepareStatements(tagged_statements.begin(),
                                  tagged_statements.end());

src/share/database/scripts/mysql/dhcpdb_create.mysql

@@ -5678,15 +5678,24 @@ UPDATE schema_version
 -- This line concludes the schema upgrade to version 18.0.
 
 -- This line starts the schema upgrade to version 19.0.
--- Drop binaddr column and index.
+-- We have to play some games to make lease address
+-- binary, primary key and retain its place as first
+-- column.
+-- Move data and primary key to binaddr
 DROP INDEX lease6_by_binaddr ON lease6;
-ALTER TABLE lease6
+UPDATE lease6 set binaddr = inet6_aton(address);
-    DROP COLUMN binaddr;
+ALTER TABLE lease6 DROP PRIMARY KEY, ADD PRIMARY KEY (binaddr);
-
+-- Copy binary data back to address column
--- Change data type of lease6.address column.
+UPDATE lease6 set address = binaddr;
+-- Make address column binary and primary key again
 ALTER TABLE lease6 MODIFY COLUMN address BINARY(16);
+ALTER TABLE lease6 DROP PRIMARY KEY, ADD PRIMARY KEY (address);
+-- Drop binaddr column
+ALTER TABLE lease6 DROP COLUMN binaddr;
 
 -- Change data type of ipv6_reservations.address column.
+-- First we replace it's contents with network bytes value.
+UPDATE ipv6_reservations set address = inet6_aton(address);
 ALTER TABLE ipv6_reservations MODIFY COLUMN address BINARY(16);
 
 -- Convert binary lease6 address to text

src/share/database/scripts/mysql/upgrade_018_to_019.sh.in

@@ -53,15 +53,24 @@ fi
 
 mysql "$@" <<EOF
 -- This line starts the schema upgrade to version 19.0.
--- Drop binaddr column and index.
+-- We have to play some games to make lease address
+-- binary, primary key and retain its place as first
+-- column.
+-- Move data and primary key to binaddr
 DROP INDEX lease6_by_binaddr ON lease6;
-ALTER TABLE lease6
+UPDATE lease6 set binaddr = inet6_aton(address);
-    DROP COLUMN binaddr;
+ALTER TABLE lease6 DROP PRIMARY KEY, ADD PRIMARY KEY (binaddr);
-
+-- Copy binary data back to address column
--- Change data type of lease6.address column.
+UPDATE lease6 set address = binaddr;
+-- Make address column binary and primary key again
 ALTER TABLE lease6 MODIFY COLUMN address BINARY(16);
+ALTER TABLE lease6 DROP PRIMARY KEY, ADD PRIMARY KEY (address);
+-- Drop binaddr column
+ALTER TABLE lease6 DROP COLUMN binaddr;
 
 -- Change data type of ipv6_reservations.address column.
+-- First we replace it's contents with network bytes value.
+UPDATE ipv6_reservations set address = inet6_aton(address);
 ALTER TABLE ipv6_reservations MODIFY COLUMN address BINARY(16);
 
 -- Convert binary lease6 address to text